|
| 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