feat(cli): offline schema source (--sql-file / --snapshot) for lint/fix/generate (#225)

[ozimakov]

Summary

Promotes the offline schema-analysis engine the MCP server already ships up to the CLI: pgrls lint, fix, and generate can now analyze a schema with no live Postgres and no Docker via two new mutually-exclusive sources:

• --sql-file PATH — raw DDL; repeatable (concatenated in order), - reads stdin
• --snapshot PATH — a pgrls snapshot artifact

This makes the common CI gate — "lint the RLS in this PR's .sql diff" — a zero-dependency step. Closes #225.

verify is deliberately excluded: offline it could emit a false-positive LEAK (a counterexample a fuller, un-passed schema would prove safe), contradicting its "never reports a leak it cannot exhibit" guarantee. It returns under its own design.

Soundness contract

Offline lint/fix/generate can only ever under-report:

• Catalog-only rules (BYPASSRLS roles, SECURITY DEFINER functions, triggers, FKs, default ACLs, views, indexes) are explicitly skipped, counted, and surfaced — never silently "passing".
• The partial-coverage fact reaches the CI gate: --format json carries additive schema_source + skipped_rules keys on offline runs (live JSON is byte-identical), and lint --require-full-coverage fails a partial run. An offline exit 0 is not a clean bill of health unless paired with that flag.
• --snapshot is treated conservatively (same inert set as --sql-file) so an older/partial snapshot can't silently no-op a catalog rule.
• fix/generate are emit-only offline (--apply rejected); emitted SQL — including --output migration files — carries an offline-provenance header.
• Untrusted input is hardened: RecursionError → bad_sql, and an 8 MiB byte cap on both --sql-file and --snapshot.

How it was built

Brainstormed → spec → plan → executed task-by-task (TDD, per-task review), then put through a five-role parallel review (correctness, soundness/security, CLI-UX, architecture, tests). That review caught a real contract break the per-task reviews missed — PERF003 (missing-index) fired on every offline RLS policy because the parser doesn't model CREATE INDEX and the rule fires on index absence. Fixed (inerted offline, still fires live, fixer suppressed offline), along with coverage-gate scoping, offline generate --config validation, --output provenance, and a batch of UX/wording fixes.

Tests

Full suite 3332 passed, ruff clean. New coverage spans every command × source × stdin/multi-file × guard × exit-code/coverage cell.

Docs

AGENTS.md "Live database only" limitation flipped to a documented feature; README gains a no-Docker tier; github-formatter note narrowed (source ranges remain a #227 follow-on); CHANGELOG entry added.

🤖 Generated with Claude Code

[dmitrymaranik]

Thanks for taking this on — #225 is a genuinely useful capability and a lot of the hard parts here are done right. I ran the full gate locally (the fork CI didn't auto-trigger): tests/+demo/ 3445 + corpus/ pass, ruff clean, and I verified the load-bearing plumbing holds — live --format json is byte-identical to before (the extra keys are additive and json-only), --apply is rejected on every offline source with the provenance header on both stdout and --output files, the PERF003 offline-inerting is correct (inert offline, still fires live, fixer suppressed), the MCP module move is clean, and the 8 MiB cap / RecursionError / stdin hardening all behave. Nice work on those.

There's one blocker though, and it's the core soundness invariant the PR sets out to guarantee — so worth getting exactly right before merge.

CRITICAL — offline --sql-file can silently false-clean. _CATALOG_ONLY_INERT (schema_sources.py) lists 17 rules, but four rules that depend on model fields schema_from_sql never populates are not in it:

Rule	Field it gates on (empty offline)
SEC035	Table.indexes
SEC041	Table.partition_of
SEC043	Table.inherits
SEC048	Schema.owner_reachable_members

Because skipped_rules is computed as inert ∩ would_run, these four silently produce zero findings, are omitted from skipped_rules, and pass --require-full-coverage. Reproduced (SEC035 — a global UNIQUE is a real cross-tenant existence leak; no other rule compensates):

-- sec035.sql
CREATE TABLE public.accounts (id uuid, tenant_id integer, email text);
ALTER TABLE public.accounts ENABLE ROW LEVEL SECURITY;
CREATE POLICY tenant_isolation ON public.accounts USING (tenant_id = current_setting('app.tenant_id')::int);
CREATE UNIQUE INDEX accounts_email_key ON public.accounts (email);

pgrls lint --sql-file sec035.sql --rule SEC035 --require-full-coverage ; echo $?
# → exit 0   (SEC035 fires live; offline it's silent AND absent from skipped_rules)

That's the one outcome the feature promises can't happen — an offline exit 0 read as "clean" while a live lint flags a real leak. SEC035/SEC048 have no fallback rule; SEC041/SEC043 happen to get SEC001 alongside, but the specific bypass signal is still gone and falsely reported as covered.

HIGH — the skip-set is source-blind, so --snapshot over-skips. A current (v21) snapshot does serialize indexes/FKs/partition_of/owner-reachability, so those rules can and should run on it — but the same static set force-skips all 17, and lint --snapshot snap.json --require-full-coverage fails 100% of the time even on a fully-complete snapshot. (Safe direction, but the coverage gate is unusable for the snapshot source.)

Both point at the same fix: make the inert decision source-aware — key it on which fields the resolved Schema actually left empty (or on snapshot version), rather than a hardcoded list. That closes the --sql-file false-clean (the four rules get surfaced + fail the coverage gate) and lets a complete snapshot exercise its full rule set. A regression test per offline-noop rule asserting it lands in skipped_rules would lock it in.

MED — CI mypy. mypy src/pgrls (the lint job) has 4 new --strict errors not on main: _fix_dispatch/_generate_dispatch params (cli.py:1585/1600) and the bare dict on format_json's extra / format_violations's extra_json. Annotate those and the lint job goes green.

Everything else looked great — this is close. Happy to re-review once the skip-set is source-aware.

[ozimakov]

A linter that says "all clear" while a real leak walks past is just an optimist with a CLI — so let's make this one a pessimist where it can't see. Pushed in 8c5246e.

CRITICAL — --sql-file false-clean. Fixed. SEC035/SEC041/SEC043/SEC048 read model fields the DDL builder never populates and are now inert on sql= — surfaced in skipped_rules and failing --require-full-coverage. Your exact repro now exits 1 with skipped_rules: ["SEC035"].

HIGH — snapshot over-skip. Fixed, and via the route you pointed at: the inert decision is now source- and version-aware rather than a blanket list.

• _CATALOG_ONLY_INERT → _CATALOG_DEPENDENT_RULES, mapping each rule to the snapshot version at which every field it reads is serialized.
• inert_rule_ids is now source-aware: sql= inerts every catalog-dependent rule (the builder populates none of those fields); a snapshot inerts a rule only when its declared version predates that rule's threshold (older capture → field decodes empty → silent no-op); database_url inerts nothing. resolve_schema now returns the snapshot version so the CLI/MCP can scope this.
• A current snapshot meets every threshold, so it exercises the full rule set and lint --snapshot snap.json --require-full-coverage passes; a pre-threshold snapshot fails closed and names the gap.
• PERF005 left the set entirely — it reads the --perf runtime artifact, not a schema field, so it no-ops without --perf on every source exactly as on a live database (and was the one rule that made every snapshot fail the gate).

One I caught on the way out, in the same spirit as yours: the threshold has to be the MAX over every field a rule reads — including fields reached through a shared helper. SEC041/SEC043 gate on _is_directly_reachable, which reads Table.column_grants (v8), so SEC041's threshold is 8, not partition_of's v2 — otherwise a v3–v7 snapshot would have false-cleaned a column-grant-only partition bypass. Threshold caveat is documented at the dict and there's a regression test pinning it. The one residual sharp edge: the thresholds are still hand-derived (an assert <= SNAPSHOT_VERSION only guards the safe direction), so a future rule that picks up a new field read needs the threshold bumped — the test_offline_noop_rule_* tests fire each rule at its threshold to catch that.

MED — mypy. Fixed: _fix_dispatch/_generate_dispatch annotated, extra/extra_json are dict[str, Any]. (The connection annotations surfaced two latent Connection | None paths on --apply, asserted closed.)

Tests: a regression test per offline-noop rule asserting it lands in skipped_rules, plus version-gating and SEC041-threshold tests. Gate: ruff clean, mypy --strict clean, tests/+demo/+corpus/ green (3482). Ready for re-review.

[dmitrymaranik]

Re-reviewed 8c5246e — the rework is genuinely strong. I verified each round-1 finding is closed: the four catalog-dependent rules are now surfaced (SEC035 etc. land in skipped_rules and fail --require-full-coverage), the inert decision is source- and version-aware exactly as hoped (sql= inerts all of them; a current snapshot inerts nothing and runs the full set; an old snapshot fails closed and names the gap; database_url inerts nothing), and mypy --strict is clean. The catch that the threshold must be the MAX over every field a rule reads including via a shared helper (SEC041/SEC043 → column_grants v8) is the right instinct.

One residual issue, and it's exactly that instinct applied one rule short — CRITICAL. PERF004 is missing from _CATALOG_DEPENDENT_RULES. It gates every emission on perf003._has_leading_column_index(table, col) (imported into perf004.py), which reads Table.indexes — never populated by the offline DDL builder. So offline it silently produces zero findings, isn't surfaced in skipped_rules, and passes --require-full-coverage. Same class as the SEC035/041/043/048 bug, via the same shared-helper path the map's own CAUTION comment warns about — PERF003 (which reads indexes directly) got mapped, but PERF004 reaching the same field through PERF003's helper slipped by.

Repro (a textbook PERF004 — a plain index defeated by lower()):

CREATE TABLE public.users (id uuid, email text);
ALTER TABLE public.users ENABLE ROW LEVEL SECURITY;
CREATE INDEX users_email_idx ON public.users (email);
CREATE POLICY p ON public.users USING (lower(email) = current_setting('app.email', true));

pgrls lint --sql-file users.sql --rule PERF004 --require-full-coverage ; echo $?
# → exit 0   (PERF004 fires live; offline it's silent and absent from skipped_rules)

Fix is one line: "PERF004": ("policy-predicate function-wrapped column indexes", 7) — threshold 7 (same as PERF003; the helper reads only the v7-baseline Index.columns, not is_primary at v13). The parametrized test_offline_noop_rule_* picks it up automatically once it's mapped.

I enumerated all 61 rules against the un-populated fields (incl. every cross-rule helper import) and PERF004 is the only remaining gap — everything else is correctly mapped or legitimately fires offline (SEC001/SEC022/SEC025 read partition/inherits/views only to skip, never silently no-op; SEC045/SEC030 read column-grants/column-details which the builder does populate). All 20 thresholds check out against the model field-versions. So with PERF004 added, the map is complete and the contract holds. Really nice work on the rest.

[ozimakov]

Caught me one rule short — and through exactly the trapdoor my own CAUTION comment was pointing at. Fixed in 7d94195.

PERF004 — offline false-clean. Mapped: "PERF004": (..., 7). It reaches Table.indexes through perf003._has_leading_column_index, so offline it was silently zero, absent from skipped_rules, and passing --require-full-coverage. Your repro now exits 1 with skipped_rules: ["PERF004"]. Threshold 7 confirmed — the helper reads only the v7-baseline Index.columns, never is_primary (v13). The parametrized test_offline_noop_rule_* picks it up automatically now that it's in the map.

Since this is the second miss via a cross-rule helper, I re-walked every from pgrls.rules.* import in src/pgrls/rules/. Only two such imports reach an un-populated field: perf004 → perf003 (indexes, now mapped) and sec043 → sec041 (column_grants, already at v17 ≥ 8). The rest are constants (_DEFAULT_IDENTITY_COLUMNS, _DEFAULT_AUTH_FUNCTIONS) or builder-populated fields (_own_table_column, _scoping_columns → columns/policies/grants/column_grants/column_details). So that matches your 61-rule enumeration: PERF004 was the last gap.

Gate: ruff clean, mypy --strict clean, tests/+demo/+corpus/ green (3483).

[dmitrymaranik]

Verified 7d94195 — PERF004 is mapped (threshold 7), your repro now exits 1 with PERF004 in skipped_rules, and the systematic re-walk of every from pgrls.rules.* import is exactly the right way to close out the "is this the last one?" question — it matches the 61-rule enumeration (only perf004→perf003 and sec043→sec041 reach an un-populated field, both now handled). Full local gate is green here: ruff, mypy --strict, and tests/+demo/+corpus/ (3483). All findings across every round are resolved.

One mechanical thing before it can merge: the branch now shows CONFLICTING — the sole conflict is CHANGELOG.md [Unreleased] (#230 landed there in the meantime). A rebase onto main clears it; nothing else conflicts. Once that's pushed it's ready to go in. Really solid work seeing this all the way through.

[dmitrymaranik]

Went ahead and rebased this onto main for you (the branch had "allow edits from maintainers" on) — the only conflict was the CHANGELOG.md [Unreleased] section, which I resolved by keeping all entries (the offline-source bullet now sits under ### Added alongside the POLICY_RENAMED one, with #230's z3 fix preserved under ### Fixed). Your 16 commits and authorship are intact; just replayed on top of current main.

Re-ran the full gate on the rebased result — ruff, mypy --strict, and tests/+demo/+corpus/ all green (3512, now including the POLICY_RENAMED tests that landed in the meantime). The PR shows mergeable now. Thanks for the thorough back-and-forth on the soundness contract — this is good to go.

[dmitrymaranik]

Merged — thank you, @ozimakov. This was a genuinely hard feature to land soundly (the offline-coverage contract has a lot of sharp edges), and you took every round of review in stride and even out-enumerated the helper-import traps yourself. Your second merged contribution to pgrls, and a meaningful one — lint / fix / generate now run with no live database or Docker, which makes the CI gate a zero-dependency step. Much appreciated; keep them coming.