Skip to content

Commit 78b1fc5

Browse files
committed
Add CodeRabbit configuration for sqlguard
1 parent 4038f0d commit 78b1fc5

1 file changed

Lines changed: 153 additions & 0 deletions

File tree

.coderabbit.yaml

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
2+
#
3+
# CodeRabbit configuration for sqlguard.
4+
# Docs: https://docs.coderabbit.ai/guides/configure-coderabbit
5+
#
6+
# The path_instructions below teach CodeRabbit this repo's non-obvious
7+
# invariants (from CLAUDE.md) so reviews are relevant and it does NOT flag
8+
# deliberate patterns (e.g. the //nolint deprecated database/sql delegations).
9+
10+
language: en-US
11+
12+
tone_instructions: >-
13+
Reviewing sqlguard, a near-zero-dependency, security-conscious SQL analyzer
14+
for Go. Be precise; prioritize correctness, hot-path allocations, concurrency
15+
safety, and PII redaction. Pre-release: prefer clean redesign over compat shims.
16+
17+
early_access: false
18+
19+
reviews:
20+
# assertive surfaces more issues — appropriate for a library aiming at a high
21+
# bar. Switch to "chill" if reviews feel noisy.
22+
profile: assertive
23+
request_changes_workflow: false
24+
high_level_summary: true
25+
poem: true
26+
review_status: true
27+
collapse_walkthrough: false
28+
sequence_diagrams: true
29+
changed_files_summary: true
30+
31+
auto_review:
32+
enabled: true
33+
drafts: false
34+
base_branches:
35+
- main
36+
37+
# Don't spend review budget on build output or vendored trees.
38+
path_filters:
39+
- "!bin/**"
40+
- "!dist/**"
41+
- "!**/testdata/**"
42+
43+
path_instructions:
44+
- path: "**/*.go"
45+
instructions: >-
46+
Go 1.26; modern idioms expected (range-over-int, `any`, compile-time
47+
interface asserts `var _ I = (*T)(nil)`). Keep the core dependency-light:
48+
analyzer, middleware and reporter must stay free of third-party deps and
49+
of YAML. Flag new external imports in those packages. Watch for
50+
allocations and unnecessary work on the per-query hot path
51+
(analyzer.Analyze, middleware.Guard.Check). Code must be safe for
52+
concurrent use where documented (Guard, QueryTracker, the caches).
53+
54+
- path: "analyzer/**"
55+
instructions: >-
56+
Redaction is the default and there is ONE canonical normalizer. Never
57+
let raw literal values reach a Result that leaves the process:
58+
Result.Query must be redacted unless rawQuery is explicitly set, and
59+
Result.Fingerprint must always be PII-free/low-cardinality. Do NOT
60+
introduce a second normalizer — everything funnels through Redact /
61+
Fingerprint. The `analyzer` package must NOT import `config` or any YAML
62+
library. Rules self-register via analyzer.Register in init(); a new rule
63+
should be a RuleSpec registration, not a hand-maintained list. Rules read
64+
the normalized Statement, never raw SQL.
65+
66+
- path: "middleware/**"
67+
instructions: >-
68+
middleware.Guard is the single analysis core (Check / CheckLatency /
69+
Observe / ResetN1 / Analyzer). All interception points and every
70+
integration must route through one Guard — flag any hand-rolled
71+
check/latency logic that bypasses it (it silently loses redaction,
72+
fingerprints, the parser seam, config, N+1, dedup and the analysis
73+
cache). The analysis cache is keyed on the EXACT query string (not the
74+
fingerprint) on purpose: a few rules read literal-derived facts the
75+
fingerprint folds away. Cached result slices are shared/read-only — do
76+
not mutate them (see Guard.report). Dedup and N+1 use bounded maps; keep
77+
them bounded.
78+
79+
- path: "middleware/driver.go"
80+
instructions: >-
81+
This hand-implements the database/sql driver-wrapping chain
82+
(Driver/DriverContext -> Connector -> Conn -> Stmt/Tx). The deprecated
83+
delegations (base.Begin, legacy Queryer/Execer, Stmt.Exec/Query) are
84+
DELIBERATE and carry //nolint:staticcheck — do NOT flag them or suggest
85+
removing them; driver.Conn and driver.Stmt still require those methods,
86+
and a faithful wrapper must delegate to whatever the base exposes. The
87+
optional-interface forwarding (return driver.ErrSkip / documented no-op
88+
when the base lacks an interface) preserves bare-driver behavior — do not
89+
"simplify" it away. Analyze only on the path that actually executes
90+
(never before an ErrSkip return) to avoid double-counting.
91+
92+
- path: "integrations/**"
93+
instructions: >-
94+
Each integration is a separate Go module so its heavy deps stay opt-in.
95+
Every integration must build on the exported middleware.Guard
96+
(pgxguard is the reference). Flag any integration that re-implements
97+
analysis, redaction or N+1 by hand. Each should expose ResetN1() for
98+
per-request scoping.
99+
100+
- path: "parsers/**"
101+
instructions: >-
102+
Optional real-grammar parsers, isolated in their own modules so heavy
103+
parser deps never enter a consumer's build. A parser must never break the
104+
caller's query path: on a parse failure return the FallbackParser's
105+
best-effort Statement (Exact=false), not an error. The dialect parsers
106+
derive structural facts from the AST but intentionally keep MaxInListLen,
107+
ImplicitCommaJoin and CartesianJoin as fallback heuristics — that carve-
108+
out is documented, not a bug.
109+
110+
- path: "config/**"
111+
instructions: >-
112+
`config` is the ONLY YAML-aware package; nothing else may import YAML and
113+
nothing should depend on `config`. Parsing is lenient by default (unknown
114+
keys/rules warn) unless strict:true — preserve that forward-compat
115+
behavior so a newer config still loads on an older binary.
116+
117+
- path: "explain/**"
118+
instructions: >-
119+
EXPLAIN must never execute the statement: keep the read-only
120+
BeginTx + always-Rollback, never use ANALYZE, and keep validate()'s
121+
comment/string-aware multi-statement rejection and SELECT/WITH-only (DML
122+
behind WithAllowDML) policy. EXPLAIN takes no bind params, so
123+
concatenation is by design — the defense is validate() + the rolled-back
124+
read-only tx; do not "fix" it with parameterization.
125+
126+
- path: "**/*_test.go"
127+
instructions: >-
128+
Use `-race` for anything touching middleware (driver chain, QueryTracker,
129+
caches are concurrent). Tests for new behavior should also prove the
130+
failure mode (e.g. a bug-reintroduction check) where practical. errcheck
131+
is intentionally relaxed in tests.
132+
133+
# CodeRabbit can also run linters/scanners and fold results into the review.
134+
tools:
135+
golangci-lint:
136+
enabled: true # uses the repo's .golangci.yml (v2 schema)
137+
gitleaks:
138+
enabled: true # secret scanning — important for a security-adjacent tool
139+
yamllint:
140+
enabled: true
141+
actionlint:
142+
enabled: true # lints .github/workflows/ci.yml
143+
markdownlint:
144+
enabled: true
145+
146+
chat:
147+
auto_reply: true
148+
149+
knowledge_base:
150+
learnings:
151+
scope: auto
152+
pull_requests:
153+
scope: auto

0 commit comments

Comments
 (0)