feat(safety): scope execute_sql to semantic-model tables - #91
Merged
Merged
Conversation
Add a deterministic table-scope gate to the shared model-safety pass so a query run through the engine may only reference tables the semantic model declares. Any other table in the connected database is refused — closing the gap where execute_sql would happily read arbitrary tables and the model only affected the trust receipt / join-trap detection, never table access. - runtime.check_table_scope(sql, org): parses with sqlglot, matches referenced physical tables (bare name, case-insensitive) against _model_table_index. CTE names and derived-subquery aliases are excluded (not tables). Excluded (review_state='rejected') tables are dropped by the loader, so they fall into the same "not declared -> refuse" path. Degrades to allow when sqlglot is unavailable / SQL doesn't parse / the model declares no tables — same posture as the fan/chasm and sensitive-projection gates. - execute_sql._model_safety: wire it in as the FIRST gate (before fan/chasm and sensitive), reusing the exit-1 + stderr-JSON refusal mechanism, so it surfaces to the MCP client exactly like preflight_refused / sensitive_columns (no tools.py change). Enforcement is refuse-always; only --no-safety bypasses. - Reach matches the existing model-safety guards: runs in the full-package / self-hosted server path; no-ops in the stdlib-only vendored plugin slice (runtime.py is intentionally not vendored). - Vendored plugins/agami/lib/execute_sql.py regenerated via dev.py sync-lib. - 12 unit tests (tests/test_table_scope_gate.py); full suite 1327 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a semantic-model–enforced table allowlist to execute_sql, refusing queries that reference tables not declared in the loaded semantic model (while preserving the existing “degrade to allow” posture when parsing/deps/model are unavailable).
Changes:
- Implement
runtime.check_table_scope(sql, org)to detect out-of-model physical table references via sqlglot. - Wire the new table-scope gate as the first check in
execute_sql._model_safety(including the vendored plugin copy) and return a structured stderr JSON refusal. - Add unit tests covering declared/undeclared tables, joins, CTEs, subquery aliases, schema-qualified refs, and degradation behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/test_table_scope_gate.py | Adds unit tests for the new table-scope guard behavior. |
| packages/agami-core/src/semantic_model/runtime.py | Introduces check_table_scope and result dataclass in the semantic-model runtime. |
| packages/agami-core/src/execute_sql.py | Runs table-scope gate first within _model_safety, refusing out-of-scope queries with stderr JSON. |
| plugins/agami/lib/execute_sql.py | Syncs the same _model_safety table-scope enforcement into the vendored executor copy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+532
to
+533
| if tree is None or not isinstance(tree, exp.Select): | ||
| return TableScopeResult("allow") |
Comment on lines
+68
to
+69
|
|
||
|
|
Comment on lines
+535
to
+542
| cte_names = {c.alias_or_name.lower() for c in tree.find_all(exp.CTE)} | ||
| offending: set[str] = set() | ||
| for tbl in tree.find_all(exp.Table): | ||
| name = tbl.name | ||
| if not name or name.lower() in cte_names: | ||
| continue # a CTE reference, not a physical table | ||
| if name.lower() not in allow: | ||
| offending.add(name) |
ashwin-agami
approved these changes
Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
execute_sqlwill run a read-onlySELECTagainst any table in the connected database. The semantic model was consulted only for the trust receipt (unmapped tables silently omitted), join-trap detection, sensitive-column projection, and default filters — none of which restrict which tables a query may reference. So a query could read a table the model never declared, and it would succeed.Change
Add a deterministic table-scope gate to the shared model-safety pass so a query run through the engine may only reference tables the semantic model declares. Any other table is refused.
runtime.check_table_scope(sql, org)— parses with sqlglot (already a dependency), collects physical table references, and matches them (bare name, case-insensitive) against_model_table_index. CTE names and derived-subquery aliases are excluded — they aren't physical tables. Excluded (review_state='rejected') tables are dropped by the loader, so they fall into the same "not declared → refuse" path.execute_sql._model_safety— wired in as the first gate (before fan/chasm and sensitive), reusing the exit-1 + stderr-JSON refusal mechanism, so it surfaces to the MCP client exactly likepreflight_refused/sensitive_columns. Notools.pychange.--no-safetyoperator flag bypasses it (same as the other model-safety gates).Reach (matches the existing model-safety guards)
This lives in the model-safety pass alongside the fan/chasm and sensitive-column guards, so it inherits their reach: it runs where the full package is importable (pip-installed / self-hosted MCP server — the
agami-deployteam path) and no-ops in the stdlib-only vendored plugin slice (runtime.pyis intentionally not vendored). Not expanding that architecture here.Degrade posture
Allows (does not block) when sqlglot is unavailable, the SQL doesn't parse, or the model declares no tables — identical to the fan/chasm and sensitive gates. The upstream read-only guard already rejects multi-statement / DDL input.
Vendored copy
plugins/agami/lib/execute_sql.pyregenerated viauv run dev.py sync-lib(drift checktest_vendored_lib_matches_sourcepasses).Tests
tests/test_table_scope_gate.py— 12 unit tests: declared/undeclared, join with undeclared (lists only the bad one), CTE reference, CTE body reading an undeclared table, subquery alias, schema-qualified, case-insensitive, empty model, non-SELECT/unparseable degrade.🤖 Generated with Claude Code