ACE-037: fail-closed on unscopable SQL

[sandeep-agami]

Stacked on #111 (the shared guardrail contract) — it depends on that PR's Verdict/Envelope/safety_verdict and the refactored gates. Base is the ACE-035-shared-guardrail-contract branch; retarget to main once #111 merges. Review/merge #111 first.

Summary

Closes a silent fail-open in the SQL object-scope gates: they only reject the exp.Table sources they find, so a query that passes the read-only guard but whose FROM/JOIN source can't be scoped — a table-function / ROWS FROM, a VALUES / UNNEST / LATERAL source, or SQL that doesn't parse at all — leaves the scope walk with nothing to reject and is allowed to run. This adds a fail-closed scopability gate: such a query is refused (unscopable_sql) rather than run blind, at the single shared executor, on every surface.

Changes

• New runtime.check_scopable(sql, org, ctx) -> Verdict | None — refuses when the tree is absent/unparseable, has no SELECT, or any source isn't a scopable named exp.Table / derived exp.Subquery. Two whole-tree sweeps (an empty-name exp.Table catches table-functions / ROWS FROM; a LATERAL sweep; and a non-Table/Subquery From/Join source catches VALUES/UNNEST) — so every set-operation arm is covered. Reuses the existing single parse (ctx.tree) — no second parser. Inert when the model declares no tables (consistent with the object-scope gates).
• Wired first in execute_sql._model_safety (before the object-scope gates), so an unscopable query fails closed instead of reaching their degrade-to-allow branches.
• AGAMI_SQL_UNSCOPABLE_POSTURE flag — enforce (default) refuses; warn logs + allows (a staged-rollout escape hatch, never the shipped default; any non-warn value fails closed). Documented in both agami.env.example copies.

Design decisions

• The gate is a separate, self-contained check placed first — it doesn't change each object-scope gate's internals; it closes the gap around them.
• Fail-closed is uniform: unparseable, no-SELECT, and parser-unavailable all refuse under enforce.
• Inert when no tables are declared — a deployment with no declared surface isn't scoping (the object-scope gates behave the same); it adds no new exposure.
• The gate never touches the datasource (it reads the parsed tree + the resolved model), so it produces the same verdict whether the model is file-served or DB-served — proven by extending the existing model-source parity test.

Test plan

• Full gate green: uv run dev.py check — ruff, format, gitleaks, vendored-lib drift, and the pytest suite across py3.10–3.12.
• New tests/test_scopable_gate.py (unit corpus: named tables / CTE / derived subquery / comma-join / set-op / SELECT 1 allow; table-functions / ROWS FROM / VALUES / UNNEST / LATERAL / LATERAL VIEW / unparseable / no-SELECT / parser-unavailable refuse; ctx-parity).
• tests/test_ace051_fail_closed.py additions: the differential corpus refuses through the wired _model_safety under enforce; warn logs + allows; any non-warn posture value fails closed; and the model-source parity test asserts an unscopable query yields the same refusal on a file-served vs DB-served model.
• Verified end-to-end on the real executor subprocess: enforce → a {"refusal":{"kind":"unscopable_sql"}} line + non-zero exit before any DB connection; warn → the log line, then proceeds.

Checklist

• Full gate green (lint, format, tests, secret scan, vendored-lib drift)
• Fail-closed by default (enforce); warn cannot silently be the default; both surfaces covered
• No second SQL parser (reuses the single parse); single execute_sql enforcement point preserved
• Path-agnostic — same fail-closed verdict on file-served vs DB-served models
• Reviewed (code-review + security-review panel: no bypass found); findings dispositioned

Spec: ACE-037

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR adds a new “scopability” safety gate to refuse SQL that cannot be fully scoped to declared tables (e.g., unparseable SQL, table-functions/ROWS FROM, VALUES/UNNEST/LATERAL sources), and wires it into the shared execute_sql._model_safety path with an operational rollout flag.

Changes:

• Introduces semantic_model.runtime.check_scopable(...) -> Verdict | None to fail-closed on unscopable queries while reusing the existing single parse (ctx.tree).
• Wires the gate first in both executor entrypoints’ _model_safety and adds AGAMI_SQL_UNSCOPABLE_POSTURE (enforce default, warn escape hatch).
• Adds focused unit + integration coverage to pin fail-closed behavior and parity across model sources.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_scopable_gate.py	New unit tests for check_scopable allow/refuse corpus and ctx-parity.
tests/test_ace051_fail_closed.py	Extends fail-closed integration tests to ensure the wired gate refuses under enforce and allows+logs under warn.
packages/agami-core/src/semantic_model/runtime.py	Adds check_scopable fail-closed gate implemented via sqlglot tree sweeps.
packages/agami-core/src/execute_sql.py	Wires check_scopable into _model_safety and adds rollout posture handling.
plugins/agami/lib/execute_sql.py	Mirrors the same _model_safety wiring and rollout posture handling for the plugin copy.
deploy/agami.env.example	Documents AGAMI_SQL_UNSCOPABLE_POSTURE and intended semantics.
plugins/agami/skills/agami-deploy/bundle/agami.env.example	Documents AGAMI_SQL_UNSCOPABLE_POSTURE in the bundled env example.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ashwin-agami]

Why even have a flag to warn? Why not just make it strict? Where do we have the use case for warning?

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[sandeep-agami]

Closing: superseded, and its base is doubly stale.

Its base is ACE-035-shared-guardrail-contract — but ACE-035 shipped from a different branch entirely (ACE-035-guardrail-contract-rewrite, #170, 9d3fd8d), because the contract was reduced rather than ported. Verdict{class, severity, certainty} and policy(verdict, tier) → Action are gone; what shipped is two dataclasses, Refusal and Envelope. This branch is built on the version that was deleted.

ACE-037 itself is still live and still needed — fail-closed on unparseable or unscopable SQL is Wave 3, and it now has a second reason to exist: ACE-039's residual is that on the vendored plugin layout semantic_model.runtime is absent by construction, so catalog relations have no gate there at all. Failing closed when the guard cannot run is the general form of that fix.

It gets a fresh branch off main when Wave 3 starts. What the re-authored version inherits is the mechanism — the scopability walk and the comma-join handling from 5a31525/a1e5065 — not this diff: the refusal is now returned rather than raised, GuardRefused no longer exists, and RULE_UNSCOPABLE is deliberately left unpinned in REASON_FOR_RULE for the gate that produces it to fill (test_ace035_guardrail_contract.py asserts exactly that).