52 Rules, 5 Dialects, Zero Installers: The SQL Analyzer

nitsql — a multi-dialect SQL static analyzer: 52 rules across SQL Server, PostgreSQL, Oracle, MySQL and SQLite. Finds SQL injection, non-SARGable predicates, missing indexes, connection leaks and error-handling gaps — in .sql files and in SQL embedded in Python, Node.js and C#. Runs as a CLI, a CI gate, a pre-commit hook, or a Claude Code skill. Open source, MIT.

We maintain quite few SQL Server database repositories — thousands of stored procedures, functions and views, most of them older than anyone currently on the team. SQL review was done by humans, in pull requests, inconsistently. This is how we replaced that with an analyzer that runs before the commit and again in CI, what we got wrong on the first attempt, and the one design decision that made it actually stick. Everything below is from a real rollout — including the three things I found wrong in my own documentation while writing it up.

The problem isn't that SQL review is hard. It's that it doesn't scale.

Human SQL review fails in a specific, predictable way: the reviewer who knows about non-SARGable predicates spots them, and the reviewer who doesn't, doesn't. So the same class of bug ships on Tuesday and gets blocked on Thursday, depending on who was free.

And the review comments themselves are usually useless:

"Consider parameterising this." — a real review comment, on a 400-line legacy procedure, with no line number and no indication of which of the eleven dynamic-SQL concatenations was meant.

That comment is not actionable. The author has to re-derive the reviewer's reasoning from scratch. Multiply that by dozen repos and a backlog of legacy objects, and review becomes a formality people click through.

What we wanted instead was a finding a developer can act on without asking a question: rule ID, exact line, the offending snippet, and a concrete fix.

Two layers, one analyzer

The design is deliberately boring:

1. A pre-commit git hook — runs the analyzer on staged *.sql files. CRITICAL findings block the commit; everything else prints and allows. Fast, local, no network.

2. A GitHub Actions gate — the same analyzer, on every pull request and push, posting rule-keyed findings. This is the authoritative layer.

The hook is a courtesy that saves you a round-trip. The CI gate is what actually enforces. That distinction matters, because a local hook is trivially bypassable (git commit --no-verify) and you should assume it will be bypassed. Never make the local hook your only enforcement point — it is a fast-feedback device, not a control.

The decision that made it stick: vendor it, don't install it

Our first version shipped an install.sh. Developers were told to run it once. Predictably, coverage was partial and silently decayed — new laptops, new hires, fresh clones.

Then we found something worse. Some clones had core.hooksPath pointed at a directory that had never existed in the repo. Git does not warn about this. It just silently runs no hooks at all.

Check this on your own repo right now: git config core.hooksPath If it returns a path, confirm that path exists and contains your hooks. A stale or wrong value means every git hook in that clone is silently disabled — for everyone who cloned the same way.

The fix was to stop depending on a human running an installer:

• Vendor the analyzer into the repo — we drop it in .githooks/ in each database repo, alongside the hook script nitsql ships in its own hooks/. It then survives plugin upgrades, plugin removal, and developers who have never installed the tooling at all.

• Auto-enable the hook from a session-start bootstrap that runs when a developer opens the repo, instead of asking them to remember a command. The bootstrap also detects and replaces a stale core.hooksPath rather than failing on it.

• Refuse to clobber. If the repo already uses a real hook framework, the bootstrap backs off instead of overwriting it.

• Pin hook scripts and workflow YAML to LF in .gitattributes. A CRLF-ified shell script fails with an incomprehensible error on Linux CI runners.

If you take one thing from this post: the reliability of a lint gate is determined almost entirely by how it is distributed, not by how good its rules are. An excellent analyzer nobody has installed catches nothing.

What an actionable finding looks like

Here is a real finding from the gate (identifiers changed), on a legacy procedure that builds dynamic SQL:

Line 240: [SA0002] SQL injection risk -- String concatenation with variable in dynamic SQL
    | (CONVERT([varchar](8),@TenantId)+'-')+IIF(@SegmentId = -1,'ALL', CONV...
    -> Use parameterised queries (sp_executesql, $1, :bind, ?)
    FIX: Replace string concatenation with parameterised query using bind variables

Compare that to "consider parameterising this." Four things are present that a human comment almost never includes:

• A stable rule ID (SA0002) — searchable, suppressible, and countable over time.

• An exact line number — no hunting.

• The offending snippet — so the author sees which concatenation, not merely that one exists.

• A fix, not a principle. "Use sp_executesql with bind variables" is a next action. "Be careful with dynamic SQL" is not.

The analyzer auto-detects dialect from context, and the 52 rules split into a portable core plus dialect-specific families:

Family	Count	Scope
SA0001–SA0019	19	Portable — apply to every dialect
SA-MS001–SA-MS013	13	SQL Server
SA-PG* / SA-MY*	5 / 5	PostgreSQL / MySQL
SA-ORA* / SA-LITE*	5 / 5	Oracle / SQLite

Because every finding carries its family-scoped ID, a suppression names exactly one rule — you never disable the SQL Server checks to silence a MySQL one.

Severity: block almost nothing

This is where most lint rollouts die. If everything is an error, the team routes around the gate by day three.

Severity	Behaviour	Rationale
CRITICAL	Blocks the commit and fails CI	SQL injection, dangerous dynamic SQL — a security bug, not a style opinion
HIGH / MEDIUM / LOW	Reported, does not block	Performance and maintainability findings, where context legitimately wins sometimes

Only CRITICAL blocks. That single choice is what let us turn the gate on across twelve repos at once, instead of negotiating per-team exceptions for a quarter.

Usefully, this is the analyzer's own exit-code contract, so CI wiring is a one-liner — I verified it rather than assuming: a file with two HIGH findings exits 0, and a file with a CRITICAL finding exits 1. You do not need to parse output to decide whether to fail the build.

python scripts/analyze_sql.py --severity critical --json sql/   # exit 1 only on CRITICAL

The escape hatch is mandatory — and it must leave a trail

You are going to turn this on over a codebase written before the rules existed. Legacy procedures will trip CRITICAL rules, and some of them are genuinely fine — a concatenated identifier that can only ever be an internal constant, for example.

Without a sanctioned bypass, people invent unsanctioned ones (--no-verify, disabling the workflow, "just merge it"). So the analyzer supports scoped suppression pragmas:

-- Scope to one rule, as a TRAILING comment on the offending line:
SET @sql = @sql + @Filter;   -- nitsql:ignore=SA0002

-- Several rules on the same line:
SET @sql = @sql + @Filter;   -- nitsql:ignore=SA0002,SA-MS007

-- Whole file, with a reason:
-- nitsql:ignore-file=SA0002 (identifier is an internal constant; no user input reaches this path)

Two details that are easy to get wrong — I verified both by running the analyzer against its own regression corpus, not by reading the docs:

• The line pragma must be on the same line as the finding, as a trailing comment. A pragma sitting on its own line above the statement suppresses nothing — the finding still fires, and you get a false sense of having handled it.

• Use = before the rule ID. ignore=SA0002 is scoped. ignore SA0002 (with a space) silently parses as a wildcard and suppresses every rule on that line — the ID you wrote is ignored. On a line tripping two rules, the space form killed both; the = form killed exactly one. Grep your codebase for nitsql:ignore (trailing space) to find suppressions that are silently broader than their author intended.

Three rules we enforce in review about how these get used:

• Always scope to a rule ID, with =. A bare -- nitsql:ignore disables every rule on that line, including ones added next year.

• Prefer line scope over file scope. A file-level suppression on a 400-line procedure hides future violations in code nobody has written yet.

• The reason is the point. A suppression without a justification is just a disabled test. With one, it is a reviewable, greppable, auditable decision — and git blame tells you who accepted the risk and when.

We ask contributors to have their AI assistant draft the justification alongside the pragma, then review it like any other claim in the PR. The point isn't who writes the sentence; it's that a sentence exists.

Turning it on over legacy code without a revolt

The specific sequence that worked:

1. Run it report-only first and count findings per rule across the whole repo. You need the distribution before you choose what blocks.

2. Block on CRITICAL only (see above). Everything else stays informational until the numbers come down.

3. Do not require a clean sweep. The gate analyses changed files in a pull request, so untouched legacy debt doesn't block unrelated work. This is the difference between a gate people accept and one they disable.

4. Announce the bypass before they hit the wall. We published the pragma syntax in the same message that announced the gate. A developer who discovers a hard block with no documented escape at 6pm becomes a permanent opponent of the tool.

5. Treat suppressions as a backlog, not a resolution. Count them; they are your remediation queue.

Beyond regex: letting the model own the documentation rules

The static analyzer is regex-based, and that is the right call for it — injection patterns and dialect syntax are mechanical. But we also wanted to enforce documentation standards: author full name, ISO date, description, usage example, GRANT EXECUTE present, no bracket-quoted identifiers, correct batch separators.

Our first attempt encoded each of those as a Python regex. It ran to roughly 700 lines and was full of edge cases. The one that finally killed the approach: a rule requiring a human author name couldn't distinguish a developer named Claude from the literal AI tool name.

So we inverted it. The rules now live only in the repo's markdown documentation, and a thin (~400-line) validator passes those documents verbatim, plus the staged SQL, to a model with a strict tool-use schema and temperature=0.

• Single source of truth. Adding or relaxing a rule is a markdown edit — no code change, no test churn, and no drift between the documented rule and the enforced rule.

• Determinism where it matters. A forced JSON tool schema plus temperature=0 means identical input yields identical output. A flaky gate is worse than no gate.

• Graceful degradation. If the API key isn't configured, the validator warns loudly in CI rather than silently passing. "Couldn't check" must never render as "passed" — that is the same failure mode as a stale core.hooksPath.

Two smaller fixes worth stealing

Dependency-ordered migration builds

Our generator emitted repeatable (R__) migration scripts in alphabetical order. That works until a pull request adds a view and a stored procedure that references it: alphabetically the procedure is emitted first, the deploy fails on a missing object, and the developer gets a confusing error about code they wrote correctly.

Replacing the alphabetical sort with a topological sort over object dependencies fixed it — the view is emitted before the procedure that references it, regardless of name. If you generate migration scripts from a model, check which sort you are using. Alphabetical ordering is a latent deploy failure waiting for the right pair of object names.

Bot-branch sprawl

Our config-sync automation opened a new branch per run (feature/<job>-v1, -v2, -v3…). Within weeks the branch list was unusable. Reworking it to a single long-lived bot branch with auto-merge removed the sprawl entirely, and we added orphan detection so objects deleted from the database don't linger in config forever. If you have automation that opens pull requests on a schedule, give it one branch and let it force-update.

Does it actually work?

The public repo carries 103 analyzer tests (pytest test-app/tests/test_analyzer.py) and 13 bats tests for the hook — 8 covering auto-enable behaviour, including recovery from that stale core.hooksPath, and 5 covering pre-commit orchestration. All passing as of this writing. test-app/ doubles as the regression corpus: test_runner.py replays it per dialect and reports where detection drifted.

Our internal integration of the hook and gate adds its own tests on top, for severity translation and cross-platform line-ending handling.

That test count matters more than it sounds. A pre-commit hook is infrastructure that runs on every developer's machine on every commit; when it breaks, it breaks everyone at once, and it breaks them in the middle of doing something else. Test it like production code.

The documentation is tested too

The two suppression gotchas above were behaviour I had documented wrongly. Writing this up turned up a third, of a different kind: the README and the skill reference disagreed with each other, and with the analyzer, about how many rules there were. Prose drifts from code silently, because nothing compiles it.

So the repo now has a CI gate that derives the counts from source and fails the build if any claim disagrees. It counts two deliberately different things:

$ python scripts/check_rule_counts.py
Derived analyzer rule count (scripts/analyze_sql.py): 52
Derived advisory rule count (references/universal-rules.md): 71 (58 guidance markers + 13 SA cross-refs)

All rule-count claims agree with derived counts.

Conflating those two numbers is exactly how the original contradiction happened: 52 is what the analyzer mechanically enforces, 71 is the broader written guidance, of which 13 entries cross-reference an analyzer rule. If you publish a number about your own tool, derive it in CI. A README is a claim, and claims rot.

It's open source — take it, or add a rule

The analyzer is public and MIT-licensed: github.com/Bugzbaggy/nitsql. No signup, no service, nothing phones home — it is a Python script you can read in an afternoon and vendor into your own repo.

git clone https://github.com/Bugzbaggy/nitsql.git
cd nitsql
python scripts/analyze_sql.py path/to/your/sql/

# dialect is auto-detected; override when you need to
python scripts/analyze_sql.py --dialect postgresql migrations/
python scripts/analyze_sql.py --severity critical --json .   # CI-friendly
python scripts/analyze_sql.py --fix procedures/              # show suggested fixes

What ships with it:

• The CLI analyzer — 52 rules across SQL Server 2019+, PostgreSQL 13+, Oracle 19c+, MySQL 8.0+ and SQLite 3.x, including SQL embedded in Python, Node.js and C#. Rules that depend on a server version say so, rather than firing against a release that never had the feature.

• A GitHub Actions template (ci/nitsql.yml). The useful one-liner is --severity critical --json: fail the build on CRITICAL only, let everything else report.

• A pre-commit hook with its own bats test suite (bats hooks/tests/).

• A Claude Code skill carrying the rule reference, dialect-specific optimisation notes, connection patterns and least-privilege guidance.

• Operational SQL you can run directly — security audits and unused/duplicate/missing index checks for SQL Server, PostgreSQL, Oracle and MySQL, plus index recommendations from live workload and slow-query diagnostics.

• A deliberately-bad regression corpus in test-app/. Point the analyzer at it to watch every rule fire — it is also the fastest way to sanity-check a change you make, and it is how I verified the suppression behaviour described above.

Contributions are open, and new rules are the most useful kind. The bar is deliberately concrete — a rule needs four things:

1. a rule ID (portable SAxxxx, or dialect-scoped like SA-MS0xx);

2. a dialect scope;

3. a fixture in test-app/ that actually trips it; and

4. a suggested fix — the sentence that appears after FIX:.

That fourth requirement is the one I would keep if I could keep only one. A rule that reports a problem without naming the remedy just moves work from the author to the reader. If you can't write the fix line, the rule probably isn't ready.

What to copy

• Vendor the analyzer into the repo. Do not depend on developers running an installer.

• Audit git config core.hooksPath. A stale value silently disables every hook, team-wide, with no warning.

• Two layers: a local hook for fast feedback, a CI gate for real enforcement. Assume the local hook gets bypassed.

• Block on CRITICAL only. Blocking on style findings is how lint rollouts die.

• Analyse changed files, not the whole repo, so legacy debt doesn't block unrelated work.

• Ship a scoped, documented bypass — rule-specific, line-scoped where possible, with a written justification. Announce it with the gate, not after. Then test your own suppression syntax: ours silently degraded to a wildcard when the rule ID was space-separated instead of =-separated, and did nothing at all when placed on the line above.

• Emit rule ID + line + snippet + fix. If a finding doesn't tell the author what to do next, it isn't a finding, it's an opinion.

• Never let "couldn't check" report as "passed." Fail loudly, or fail closed.

• Topologically sort generated migrations. Alphabetical ordering is a deploy failure waiting to happen.

• Test your hooks. They run on every commit, on every machine.

• Derive published numbers in CI. If your README states a count, have the build compute it from source and fail on disagreement. Documentation drifts silently; nothing compiles prose.

None of this is clever. It is a regex analyzer, a git hook, and a YAML workflow. What made it work was accepting three things: developers will not install tools, legacy code will violate your rules, and a gate without a sanctioned escape hatch gets routed around rather than respected.