diff --git a/dev.py b/dev.py index ada5e1b..5be9708 100644 --- a/dev.py +++ b/dev.py @@ -42,7 +42,7 @@ # The module-load import closure of the 4 scripts (all stdlib-only). tests/test_plugin_lib_resolution.py # fails if a script gains a module-level import that's missing here; a new *lazy* import from the package # would also need adding to this list. -_VENDORED = ["agami_paths.py", "execute_sql.py", "sql_guard.py", "semantic_model/__init__.py", "semantic_model/units.py"] +_VENDORED = ["agami_paths.py", "execute_sql.py", "sql_guard.py", "guardrail.py", "semantic_model/__init__.py", "semantic_model/units.py"] def run(cmd: list[str], *, allow_fail: bool = False) -> int: diff --git a/migrations/core/012_guardrail_audit.sql b/migrations/core/012_guardrail_audit.sql new file mode 100644 index 0000000..11669c8 --- /dev/null +++ b/migrations/core/012_guardrail_audit.sql @@ -0,0 +1,23 @@ +-- The **guardrail audit trail** — one row per execute_sql call, keyed by the response Envelope's +-- `audit_id`. Written best-effort at the shared executor chokepoint (`tools.tool_execute_sql`) so +-- every ok/refused result is recorded on BOTH surfaces (stdio + HTTP), not only the HTTP path that +-- writes `tool_calls`. This is the trail the Envelope's `audit_id` points at. +-- +-- Deliberately thin for now — status + refusal kind + the query context; a richer verdict/action +-- list is a follow-on. Portable CREATE TABLE (runs on SQLite + Postgres unchanged). + +CREATE TABLE guardrail_audit ( + audit_id TEXT PRIMARY KEY, -- == the response Envelope's audit_id + ts TEXT NOT NULL, -- UTC ISO8601 + datasource TEXT, + status TEXT NOT NULL, -- 'ok' | 'refused' + refusal_kind TEXT, -- the refusal kind when status = 'refused' + sql TEXT, -- the query the caller sent (may be NULL on a bad request) + row_count INTEGER, -- on an ok result + execution_ms INTEGER, + correlation_id TEXT, -- the turn (one user question), self-reported; may be NULL + source TEXT -- 'mcp_server' +); + +-- The admin views read newest-first; index the time access path. +CREATE INDEX idx_guardrail_audit_ts ON guardrail_audit (ts); diff --git a/packages/agami-core/pyproject.toml b/packages/agami-core/pyproject.toml index 6084cee..fc7a105 100644 --- a/packages/agami-core/pyproject.toml +++ b/packages/agami-core/pyproject.toml @@ -45,7 +45,7 @@ server = [ package-dir = { "" = "src" } # Flat top-level modules (not under a parent package) — listed explicitly so the import # names stay flat regardless of disk layout. -py-modules = ["agami_paths", "async_offload", "execute_sql", "sql_guard", "mcp_harness", "mcp_http", "tools", "ports", "contracts", "oss_adapters", "store", "model_store", "model_deploy", "deploy_preflight", "oauth_server", "oidc", "passwords", "user_store", "admin", "ui", "onboarding"] +py-modules = ["agami_paths", "async_offload", "execute_sql", "sql_guard", "guardrail", "mcp_harness", "mcp_http", "tools", "ports", "contracts", "oss_adapters", "store", "model_store", "model_deploy", "deploy_preflight", "oauth_server", "oidc", "passwords", "user_store", "admin", "ui", "onboarding"] packages = ["semantic_model"] [tool.setuptools.package-data] diff --git a/packages/agami-core/src/contracts.py b/packages/agami-core/src/contracts.py index bb2be25..7c765be 100644 --- a/packages/agami-core/src/contracts.py +++ b/packages/agami-core/src/contracts.py @@ -57,21 +57,12 @@ class ExecuteSqlRequest(_Contract): # --------------------------------------------------------------------------- -# Errors (every tool may return {"error": {"kind", "remediation"}, ...}) +# Errors: execute_sql returns the shared guardrail Envelope (status / refusal — see +# `guardrail.py`); the other tools return a plain `{"error": {kind, remediation}}` body. Neither +# needs a pydantic type here. # --------------------------------------------------------------------------- -class ToolError(_Contract): - kind: str - remediation: str - - -class ErrorResult(_Contract): - error: ToolError - sql: str | None = None - execution_ms: int | None = None - - # --------------------------------------------------------------------------- # list_datasources # --------------------------------------------------------------------------- @@ -210,3 +201,19 @@ class ToolCallRecord(_Contract): agent_query: str | None = None thread_id: str | None = None correlation_id: str | None = None # the turn: one user question -> N agent sub-queries + + +class GuardrailAuditRecord(_Contract): + """The verdict trail for one execute_sql call, keyed by the response Envelope's `audit_id`. + Written best-effort at the shared executor chokepoint on every surface (see `guardrail_audit`).""" + + audit_id: str + ts: str + status: str # 'ok' | 'refused' + datasource: str | None = None + refusal_kind: str | None = None + sql: str | None = None + row_count: int | None = None + execution_ms: int | None = None + correlation_id: str | None = None + source: str | None = None diff --git a/packages/agami-core/src/execute_sql.py b/packages/agami-core/src/execute_sql.py index 737e293..341e2ca 100644 --- a/packages/agami-core/src/execute_sql.py +++ b/packages/agami-core/src/execute_sql.py @@ -53,6 +53,7 @@ from typing import TYPE_CHECKING, Any import agami_paths +from guardrail import Refusal, Verdict if TYPE_CHECKING: # ``Executor`` is the 5th port; imported only for type-checkers. At runtime ``execute_sql`` never @@ -82,6 +83,7 @@ def _resolve_default_profile() -> str: if CONFIG_PATH.exists(): try: import json as _json + cfg = _json.loads(CONFIG_PATH.read_text()) active = cfg.get("active_profile") if isinstance(active, str) and active: @@ -126,13 +128,14 @@ def __init__(self, msg: str, *, code: int) -> None: class GuardRefused(Exception): - """A guard refusal short-circuiting the envelope. ``envelope`` is the JSON error object the - caller must emit (the read-only guard's ``permission`` refusal), or ``None`` when the refusal - JSON was already written to stderr by ``_model_safety`` (carry only the exit ``code``).""" + """A guard refusal short-circuiting the executor. Carries the typed ``Refusal`` (the shared + guardrail shape) so BOTH callers build the SAME envelope from it: the subprocess ``main`` emits + ``Envelope.refused(refusal)`` JSON to stderr, and the in-process MCP handler returns it directly. + No stderr round-trip — so a model-safety refusal keeps its structured detail on both paths.""" - def __init__(self, envelope: dict | None, *, code: int) -> None: + def __init__(self, refusal: Refusal, *, code: int) -> None: super().__init__() - self.envelope = envelope + self.refusal = refusal self.code = code @@ -259,9 +262,12 @@ def _load_credentials(profile: str) -> dict[str, str]: # Schemes we accept. Strip "+driver" suffixes (e.g. postgresql+asyncpg, postgres+psycopg2). _POSTGRES_SCHEMES = {"postgres", "postgresql"} _MYSQL_SCHEMES = {"mysql", "mariadb"} -_REDSHIFT_SCHEMES = {"redshift"} # speaks Postgres wire protocol; port 5439, SSL required -_SNOWFLAKE_SCHEMES = {"snowflake"} # native CLI (snowsql) + snowflake-connector-python -_BIGQUERY_SCHEMES = {"bigquery", "bq"} # google-cloud-bigquery — auth via service-account JSON or ADC +_REDSHIFT_SCHEMES = {"redshift"} # speaks Postgres wire protocol; port 5439, SSL required +_SNOWFLAKE_SCHEMES = {"snowflake"} # native CLI (snowsql) + snowflake-connector-python +_BIGQUERY_SCHEMES = { + "bigquery", + "bq", +} # google-cloud-bigquery — auth via service-account JSON or ADC def _parse_dsn(dsn: str) -> dict[str, str]: @@ -330,7 +336,7 @@ def _parse_dsn(dsn: str) -> dict[str, str]: return out elif base_scheme == "sqlite": # sqlite:///absolute/path or sqlite:relative/path - path = dsn[len("sqlite://"):] + path = dsn[len("sqlite://") :] if path.startswith("/"): path = path[1:] if path[1:2] == "/" else path # handle `sqlite:////abs` # Trailing path normalization @@ -555,9 +561,7 @@ def _run_bigquery(creds: dict[str, str], sql: str) -> ExecResult: except Exception: pass try: - creds_obj = service_account.Credentials.from_service_account_file( - sa_path_expanded - ) + creds_obj = service_account.Credentials.from_service_account_file(sa_path_expanded) client_kwargs["credentials"] = creds_obj except Exception as e: raise ExecutorError(f"BigQuery credentials load failed: {e}", code=2) @@ -606,6 +610,7 @@ def _run_bigquery(creds: dict[str, str], sql: str) -> ExecResult: def _run_sqlite(creds: dict[str, str], sql: str) -> ExecResult: import sqlite3 # always available in stdlib + _require(creds, "path") path = os.path.expanduser(creds["path"]) try: @@ -632,9 +637,12 @@ def _run_sqlserver(creds: dict[str, str], sql: str) -> ExecResult: _require(creds, "host", "user", "password") try: conn = pymssql.connect( - server=creds["host"], port=int(creds.get("port", 1433)), - user=creds["user"], password=creds["password"], - database=creds.get("database", ""), login_timeout=15, + server=creds["host"], + port=int(creds.get("port", 1433)), + user=creds["user"], + password=creds["password"], + database=creds.get("database", ""), + login_timeout=15, ) except Exception as e: raise ExecutorError(f"SQL Server connect failed: {e}", code=4) @@ -662,8 +670,9 @@ def _run_oracle(creds: dict[str, str], sql: str) -> ExecResult: dsn = creds.get("dsn") or creds.get("url") if not dsn: _require(creds, "host", "service_name") - dsn = oracledb.makedsn(creds["host"], int(creds.get("port", 1521)), - service_name=creds["service_name"]) + dsn = oracledb.makedsn( + creds["host"], int(creds.get("port", 1521)), service_name=creds["service_name"] + ) try: conn = oracledb.connect(user=creds["user"], password=creds["password"], dsn=dsn) except Exception as e: @@ -694,7 +703,8 @@ def _run_databricks(creds: dict[str, str], sql: str) -> ExecResult: _require(creds, "host", "http_path", "token") try: conn = dbsql.connect( - server_hostname=creds["host"], http_path=creds["http_path"], + server_hostname=creds["host"], + http_path=creds["http_path"], access_token=creds["token"], ) except Exception as e: @@ -725,9 +735,13 @@ def _run_trino(creds: dict[str, str], sql: str) -> ExecResult: if creds.get("password"): auth = trino.auth.BasicAuthentication(creds["user"], creds["password"]) conn = trino.dbapi.connect( - host=creds["host"], port=int(creds.get("port", 8080)), user=creds["user"], - catalog=creds.get("catalog"), schema=creds.get("schema"), - http_scheme="https" if creds.get("password") else "http", auth=auth, + host=creds["host"], + port=int(creds.get("port", 8080)), + user=creds["user"], + catalog=creds.get("catalog"), + schema=creds.get("schema"), + http_scheme="https" if creds.get("password") else "http", + auth=auth, ) except Exception as e: raise ExecutorError(f"Trino connect failed: {e}", code=4) @@ -857,11 +871,10 @@ def _resolve_guard_model(profile: str): the DB when one is configured (hosted — the `/artifacts` disk mount may be absent), else the on-disk YAML (local). Returns an `Organization` or None if neither is available. - The DB import is lazy AND env-guarded on purpose: the local executor runs from a stdlib-lean - mirror that does not ship `store`/`model_store`, so we only reach for them when a DB is set. - Any DB-load failure degrades to disk rather than crashing the executor.""" - from semantic_model import loader as L - + The DB/loader imports are lazy AND (for the DB) env-guarded on purpose: the local executor runs + from a stdlib-lean mirror that does not ship `store`/`model_store`, so we only reach for them when + a DB is set; and the loader import sits inside the disk-path try so an import failure degrades to + None (hosted then fails closed) rather than crashing the executor.""" # Any load failure below degrades to the next source (DB → disk → None), silently: a freeform # error line here would (a) leak DB connection details from the exception and (b) precede the # JSON refusal `_model_safety` emits when both sources are absent on hosted, breaking the @@ -877,30 +890,45 @@ def _resolve_guard_model(profile: str): try: org = _load_db(store, profile) finally: - store.close() + try: + store.close() + except Exception: + pass # a close error must not discard a model that loaded fine (→ false refusal) if org is not None: return org except Exception: pass # DB unreachable/misconfigured -> fall through to disk - root = Path(os.environ.get("AGAMI_ARTIFACTS_DIR") or (Path.home() / "agami-artifacts")) / profile + root = ( + Path(os.environ.get("AGAMI_ARTIFACTS_DIR") or (Path.home() / "agami-artifacts")) / profile + ) if (root / "org.yaml").exists(): try: + from semantic_model import loader as L + return L.load_organization(root) except Exception: - pass # unparseable/absent on disk -> None (hosted then fails closed) + pass # unparseable/absent on disk, or loader import failure -> None (hosted fails closed) return None -def _model_safety(sql: str, profile: str, area: str | None): +def _refusal_from_verdict(kind: str, verdict: Verdict) -> Refusal: + """Build the shared ``Refusal`` from a safety ``Verdict``. A safety verdict always refuses + (``policy(safety)`` is ``reject`` in every tier), so the guard chain refuses unconditionally on a + non-None verdict. ``kind`` is the caller-supplied refusal kind (the verdict's ``rule`` names the + gate, e.g. ``read_only``; the refusal ``kind`` is the outward vocabulary, e.g. ``permission``).""" + return Refusal(kind=kind, reason=verdict.detail, remediation=verdict.remediation) + + +def _model_safety(sql: str, profile: str, area: str | None) -> tuple[str, Refusal | None]: """Semantic-model safety pass before execution: fan-trap / chasm-trap pre-flight + default_filters auto-application, over a model resolved from the DB (hosted) or disk (local). - Returns (sql_to_run, exit_code). exit_code is None to continue, or an int to - short-circuit (a refusal the caller must consume). Inert (returns the SQL unchanged) when the - model package isn't importable, or — on the LOCAL path only — when there is no model yet. On the - HOSTED path a model that can't be resolved fails closed (refuses), never runs unguarded (ACE-051). - """ + Returns (sql_to_run, refusal). ``refusal`` is None to continue, or a typed ``Refusal`` the caller + (``execute_guarded``) raises as a ``GuardRefused`` — so both the subprocess and in-process paths + build the same envelope from it. Inert (returns the SQL unchanged) when the model package isn't + importable, or — on the LOCAL path only — when there is no model yet. On the HOSTED path a model + that can't be resolved fails closed (refuses), never runs unguarded (ACE-051).""" try: from semantic_model import runtime as RT except Exception: @@ -910,11 +938,11 @@ def _model_safety(sql: str, profile: str, area: str | None): # sqlglot-unavailable / unparseable-SQL degrade-to-allow is a distinct fail-open owned by # ACE-037, not closed here.) if _hosted(): - json.dump({"error": {"kind": "model_unavailable", "reason": - "semantic-model package not importable; refusing to run unguarded on the " - "hosted server"}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + return sql, Refusal( + "model_unavailable", + "semantic-model package not importable; refusing to run unguarded on the " + "hosted server", + ) return sql, None # local: model package not available -> no-op org = _resolve_guard_model(profile) @@ -922,11 +950,11 @@ def _model_safety(sql: str, profile: str, area: str | None): if _hosted(): # Fail closed: a served query with no resolvable model must be refused, never run with # the fan/chasm/scope/PII guards silently off. - json.dump({"error": {"kind": "model_unavailable", "reason": - "no semantic model could be resolved (checked DB and disk); refusing to run " - "unguarded on the hosted server"}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + return sql, Refusal( + "model_unavailable", + "no semantic model could be resolved (checked DB and disk); refusing to run " + "unguarded on the hosted server", + ) return sql, None # local: no model yet -> no-op (unchanged) # Build the shared guard context ONCE — parse the SQL + build each model index a single @@ -939,37 +967,24 @@ def _model_safety(sql: str, profile: str, area: str | None): # declares; any other table in the connected database is refused. Runs FIRST # so the fan/chasm and sensitive checks below only evaluate in-scope tables. ts = RT.check_table_scope(sql, org, ctx=ctx) - if ts.action == "refuse": - json.dump({"error": {"kind": "table_out_of_scope", "tables": ts.offending_tables, - "reason": ts.reason, "suggestion": ts.suggestion}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + if ts is not None: + return sql, _refusal_from_verdict("table_out_of_scope", ts) # SELECT * ban — force every projected column to be named, so the column-scope # guard below can check what is actually returned (and nothing hides behind *). star = RT.check_no_select_star(sql, ctx=ctx) - if star.action == "refuse": - json.dump({"error": {"kind": "select_star", - "reason": star.reason, "suggestion": star.suggestion}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + if star is not None: + return sql, _refusal_from_verdict("select_star", star) # Column-scope guard — a column that binds to a declared table must be one that # table declares (a hallucinated column, or a physical column the model excluded). cs = RT.check_column_scope(sql, org, ctx=ctx) - if cs.action == "refuse": - json.dump({"error": {"kind": "column_out_of_scope", "columns": cs.columns, - "reason": cs.reason, "suggestion": cs.suggestion}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + if cs is not None: + return sql, _refusal_from_verdict("column_out_of_scope", cs) pf = RT.pre_flight_check(sql, org, ctx=ctx) if pf.risk and pf.action == "refuse": - json.dump({"error": {"kind": "preflight_refused", "risk": pf.risk, - "reason": pf.reason, "suggestion": pf.suggestion, - "triggering_joins": pf.triggering_joins}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + return sql, Refusal("preflight_refused", pf.reason, pf.suggestion or "") if pf.risk and pf.action == "auto_rewrite" and pf.rewritten_sql: sys.stderr.write(f"[agami] auto-corrected {pf.risk}: ran rewritten SQL. {pf.reason}\n") sql = pf.rewritten_sql @@ -981,10 +996,7 @@ def _model_safety(sql: str, profile: str, area: str | None): # path happened to read a prose rule). Aggregates / filters / joins are allowed. sens = RT.check_sensitive_projection(sql, org, ctx=ctx) if sens.action == "refuse": - json.dump({"error": {"kind": "sensitive_columns", "columns": sens.columns, - "reason": sens.reason, "suggestion": sens.suggestion}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + return sql, Refusal("sensitive_columns", sens.reason, sens.suggestion or "") new_sql, applied = RT.apply_default_filters(sql, org, area=area, ctx=ctx) if applied: @@ -1128,20 +1140,19 @@ def execute_guarded( ``no_safety``, which skips only the semantic-model pass, never write/RCE/DoS protection) -> semantic-model safety pass (fan/chasm pre-flight + scope + PII + ``default_filters`` rewrite) -> resolve the datasource -> ``executor.execute(vetted_sql, …)``. The executor only ever receives - SQL both guards have passed. Raises ``GuardRefused`` on a refusal (the read-only refusal carries - its JSON envelope for the caller to emit; a model-safety refusal already wrote its JSON to stderr - and carries only the exit code) and ``ExecutorError`` on a connect/run failure — so the - subprocess ``main`` and the in-process MCP handler apply the same guard and surface errors - identically. The row cap rides the request-scoped ``_max_rows_override`` ContextVar the caller sets.""" + SQL both guards have passed. Raises ``GuardRefused`` carrying the typed ``Refusal`` on a refusal, + and ``ExecutorError`` on a connect/run failure — so the subprocess ``main`` and the in-process MCP + handler apply the same guard and build the same envelope. The row cap rides the request-scoped + ``_max_rows_override`` ContextVar the caller sets.""" import sql_guard - reason = sql_guard.check_read_only(sql) - if reason is not None: - raise GuardRefused({"error": {"kind": "permission", "remediation": reason}}, code=1) + verdict = sql_guard.check_read_only(sql) + if verdict is not None: + raise GuardRefused(_refusal_from_verdict("permission", verdict), code=1) if not no_safety: - sql, rc = _model_safety(sql, profile, area) - if rc is not None: - raise GuardRefused(None, code=rc) + sql, refusal = _model_safety(sql, profile, area) + if refusal is not None: + raise GuardRefused(refusal, code=1) creds = _load_credentials(profile) return executor.execute(sql, creds, profile=profile) @@ -1164,13 +1175,23 @@ def main() -> int: src = p.add_mutually_exclusive_group(required=True) src.add_argument("--sql", help="SQL statement (use --sql-file for SQL with special characters)") src.add_argument("--sql-file", help="Path to a file containing one SQL statement") - p.add_argument("--area", default=None, - help="Subject area for the semantic-model safety pass (pre-flight + default_filters).") - p.add_argument("--no-safety", action="store_true", - help="Skip the semantic-model pre-flight / default_filters pass.") - p.add_argument("--max-rows", type=int, default=None, - help="Lower the row cap for this call (never raises it). Effective cap = " - "min(this, AGAMI_SQL_MAX_ROWS) — the env is the deployment cap, default 1000.") + p.add_argument( + "--area", + default=None, + help="Subject area for the semantic-model safety pass (pre-flight + default_filters).", + ) + p.add_argument( + "--no-safety", + action="store_true", + help="Skip the semantic-model pre-flight / default_filters pass.", + ) + p.add_argument( + "--max-rows", + type=int, + default=None, + help="Lower the row cap for this call (never raises it). Effective cap = " + "min(this, AGAMI_SQL_MAX_ROWS) — the env is the deployment cap, default 1000.", + ) args = p.parse_args() # Per-call cap (ACE-044); the sink reads it via _resolve_row_cap. No token/reset kept: main() is @@ -1187,18 +1208,18 @@ def main() -> int: # Route through the single guarded envelope with the built-in executor: guard -> model-safety -> # resolve -> connect-and-run, returning native rows we then serialize to stdout as CSV (the - # subprocess wire). Same guard, same verdicts, same connect-per-query behaviour as before — the - # split just makes the connect-and-run step swappable in-process (AH-012). The guard is the hard - # security gate for EVERY caller (both MCP servers, the agami-query skill, cron), NOT bypassable - # via --no-safety (which skips only the semantic-model pass, never write/RCE/DoS protection). + # subprocess wire). The guard is the hard security gate for EVERY caller (both MCP servers, the + # agami-query skill, cron), NOT bypassable via --no-safety (which skips only the semantic-model + # pass, never write/RCE/DoS protection). try: result = execute_guarded( sql, profile, args.area, executor=BUILTIN_EXECUTOR, no_safety=args.no_safety ) except GuardRefused as refusal: - if refusal.envelope is not None: # read-only refusal: emit its JSON (model-safety already did) - json.dump(refusal.envelope, sys.stderr) - sys.stderr.write("\n") + # Emit the shared refusal as one JSON line to stderr — the subprocess wire the MCP tool relays + # into a refused Envelope, and a direct CLI caller reads alongside the exit code. + json.dump({"refusal": refusal.refusal.as_dict()}, sys.stderr) + sys.stderr.write("\n") return refusal.code except ExecutorError as exc: sys.stderr.write(f"{exc.msg}\n") diff --git a/packages/agami-core/src/guardrail.py b/packages/agami-core/src/guardrail.py new file mode 100644 index 0000000..a040886 --- /dev/null +++ b/packages/agami-core/src/guardrail.py @@ -0,0 +1,213 @@ +"""Shared guardrail contract — ``Verdict``, ``policy``, and the response ``Envelope``. + +The one result shape for the three SQL-execution guardrails: **safety** (reject) · +**data-protection** (mask) · **governance** (warn). Every gate produces a ``Verdict``; one +``policy`` maps a verdict to an action by class + deployment tier; and every surface returns one +``Envelope``. Each guardrail builds to these types and never defines its own result shape. + +**Stdlib-only, dataclasses only — keep it that way.** This module is vendored into the +marketplace plugin (``plugins/agami/lib/``) and imported by ``ports`` (which must stay +dependency-free), so it must never grow a third-party import. The vendored-purity guard depends +on this staying pure. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any, Literal + +# ── Controlled vocabularies ────────────────────────────────────────────────── +Class = Literal["safety", "data_protection", "governance"] +Severity = Literal["high", "medium", "low"] +Certainty = Literal["provable", "heuristic", "uncertain"] +Action = Literal["allow", "reject", "mask", "row_filter", "rewrite", "warn"] +Status = Literal["ok", "refused"] +Tier = Literal["oss", "saas", "enterprise"] + +# Value sets exposed for validation + tests (the `action` set and the class set are stable). +CLASSES: tuple[str, ...] = ("safety", "data_protection", "governance") +ACTIONS: tuple[str, ...] = ("allow", "reject", "mask", "row_filter", "rewrite", "warn") + +# Every refusal kind the system emits, grouped by origin. `Refusal.kind` is an open ``str`` — +# operational failures (`syntax` / `auth` / `driver_missing`) come from the DB driver, not a fixed +# guardrail vocabulary — so this documents the known set (and a test pins the emit sites against it), +# rather than a `Literal` that nothing at runtime (no mypy in CI) would actually enforce. This is the +# canonical vocabulary for the whole safe-SQL feature: a few kinds are FORWARD-DECLARED here for the +# gates that land in the dependent slices on top of this contract (`unscopable_sql` — fail-closed +# scoping; `resource_limit` — the statement timeout; `recon` — the metadata deny-list), so the set is +# defined once, in the base, and the emit-site test only asserts emitted ⊆ documented (never the +# reverse, which would fail here until those slices land). +REFUSAL_KINDS: tuple[str, ...] = ( + # safety — integrity / confinement / object-scope / availability / fail-closed + "permission", + "table_out_of_scope", + "column_out_of_scope", + "select_star", + "unscopable_sql", # forward-declared: emitted by the fail-closed-scoping slice + "resource_limit", # forward-declared: emitted by the per-statement-timeout slice + "recon", # forward-declared: emitted by the recon deny-list slice + "model_unavailable", + # data-protection / governance — emitted by those gates + "sensitive_columns", + "preflight_refused", + # operational / execution failures — from the executor + DB driver + "timeout", + "dsn", + "driver_missing", + "auth", + "syntax", + "other", +) + + +@dataclass(frozen=True) +class Verdict: + """What a single gate returns. + + ``cls`` is serialized as ``class`` (a Python keyword can't be a field name). ``certainty`` + is the axis ``policy`` keys on: **safety** emits ``uncertain`` ⇒ reject (fail-closed on + doubt); **governance** emits ``heuristic`` ⇒ warn (undecidable) or ``provable`` ⇒ + reject/rewrite at an enforcing tier. ``severity`` is load-bearing only on the provable + governance path. ``rewritten_sql`` is present only when the action is ``rewrite``. + """ + + cls: Class + rule: str + severity: Severity + certainty: Certainty + detail: str + remediation: str + rewritten_sql: str | None = None + + def as_dict(self) -> dict[str, Any]: + d: dict[str, Any] = { + "class": self.cls, + "rule": self.rule, + "severity": self.severity, + "certainty": self.certainty, + "detail": self.detail, + "remediation": self.remediation, + } + if self.rewritten_sql is not None: + d["rewritten_sql"] = self.rewritten_sql + return d + + +def safety_verdict(rule: str, detail: str, remediation: str) -> Verdict: + """Build a safety-class ``Verdict``. Safety findings are always ``provable`` + ``high`` — a + safety gate that fires is deterministic and always rejects. ``rule`` names the gate (e.g. + ``read_only``, ``table_scope``, ``column_scope``, ``no_select_star``).""" + return Verdict( + cls="safety", + rule=rule, + severity="high", + certainty="provable", + detail=detail, + remediation=remediation, + ) + + +@dataclass(frozen=True) +class Refusal: + """The ``refusal`` block of a refused ``Envelope``: why the query was rejected, with a + remediation. + + ``reason`` is ALWAYS value-free — it never carries raw SQL or raw DB driver text (schema / column + / value names, a DSN), which must not cross the boundary between the model and the customer's + database. A **guardrail** refusal (a safety or model gate) reasons from the model; an + **operational** failure (a DB syntax / connection error surfaced by the executor) is classified + into a fixed value-free reason, with the raw driver text captured separately for the server-side + audit trail only. (The operational-error sanitization is wired by the recon/error-hardening + slice; the tool layer classifies via ``_classify_db_error`` on both surfaces.)""" + + kind: str # one of REFUSAL_KINDS — an open str (operational kinds come from the DB driver) + reason: str + remediation: str = "" + + def as_dict(self) -> dict[str, Any]: + return {"kind": self.kind, "reason": self.reason, "remediation": self.remediation} + + +@dataclass(frozen=True) +class Envelope: + """The one shape every surface returns. + + ``data`` is absent when refused; ``applied`` records transforms/bounds actually applied — today + only the fetch bound ``{row_cap: N}`` is wired; ``{mask: col}`` / ``{row_filter: expr}`` / + ``{rewrite: reason}`` land with the data-protection / governance gates. ``warnings`` are + governance annotations (``Verdict``s); ``refusal`` is present iff ``status == 'refused'``; + ``audit_id`` references the recorded verdict trail. + """ + + status: Status + audit_id: str = "" + data: Any | None = None + applied: list[dict[str, Any]] = field(default_factory=list) + warnings: list[Verdict] = field(default_factory=list) + refusal: Refusal | None = None + + @classmethod + def refused( + cls, + refusal: Refusal, + *, + audit_id: str = "", + warnings: list[Verdict] | None = None, + ) -> Envelope: + return cls(status="refused", audit_id=audit_id, refusal=refusal, warnings=warnings or []) + + @classmethod + def ok( + cls, + data: Any, + *, + audit_id: str = "", + applied: list[dict[str, Any]] | None = None, + warnings: list[Verdict] | None = None, + ) -> Envelope: + return cls( + status="ok", + audit_id=audit_id, + data=data, + applied=applied or [], + warnings=warnings or [], + ) + + def as_dict(self) -> dict[str, Any]: + d: dict[str, Any] = {"status": self.status, "audit_id": self.audit_id} + if self.data is not None: + d["data"] = self.data + d["applied"] = list(self.applied) + d["warnings"] = [w.as_dict() for w in self.warnings] + if self.refusal is not None: + d["refusal"] = self.refusal.as_dict() + return d + + +def policy(verdict: Verdict, tier: Tier = "oss") -> Action: + """Map a verdict to an action by class + deployment tier. + + The class is read from ``verdict.cls`` (it already lives on the verdict, so it is not passed + as a separate argument). + + - **safety** → always ``reject``, every tier; ``certainty == 'uncertain'`` also ⇒ ``reject`` + (fail-closed on doubt). Fully implemented here. + - **data_protection** → ``mask`` / ``row_filter`` where the gate names one, else ``reject`` + (fail-closed). *Stub:* the data-protection gates fill this branch in later work; until then + it fails closed. + - **governance** → graded by (severity, certainty, tier): ``reject``/``rewrite`` only for a + ``provable`` verdict under an enforcing tier, else ``warn`` (OSS warns · SaaS recommends · + Enterprise enforces). *Stub:* the governance gates fill this branch in later work. + """ + if verdict.cls == "safety": + # Safety is absolute: reject in every tier; uncertainty is already a reject (fail-closed). + return "reject" + if verdict.cls == "data_protection": + # Stub — fill mask/row_filter selection later. Fail closed until then (the safe default). + return "reject" + if verdict.cls == "governance": + # Stub — the graded (severity, certainty, tier) logic is filled in later. + if tier == "enterprise" and verdict.certainty == "provable": + return "rewrite" if verdict.rewritten_sql is not None else "reject" + return "warn" + raise ValueError(f"unknown verdict class: {verdict.cls!r}") diff --git a/packages/agami-core/src/mcp_http.py b/packages/agami-core/src/mcp_http.py index 90faa9d..73ce2f4 100644 --- a/packages/agami-core/src/mcp_http.py +++ b/packages/agami-core/src/mcp_http.py @@ -31,9 +31,9 @@ from execute_sql import BUILTIN_EXECUTOR from oss_adapters import ( FileActivitySink, + NoopGovernancePolicy, PresenceAuthProvider, SingleTenantOrgResolver, - WarnOnlyGovernancePolicy, ) from ports import Adapters, AuthProvider, Org, OrgResolver from starlette.applications import Starlette @@ -125,7 +125,7 @@ def default_adapters() -> Adapters: activity_sink=FileActivitySink(), org_resolver=_build_org_resolver(), auth_provider=_build_auth_provider(), - governance=WarnOnlyGovernancePolicy(), + governance=NoopGovernancePolicy(), executor=BUILTIN_EXECUTOR, ) diff --git a/packages/agami-core/src/model_store.py b/packages/agami-core/src/model_store.py index dd8fe8c..20eef09 100644 --- a/packages/agami-core/src/model_store.py +++ b/packages/agami-core/src/model_store.py @@ -350,6 +350,26 @@ def record_tool_call(self, record: Any) -> None: ) self._store.commit() + def record_guardrail_audit(self, record: Any) -> None: + self._store.execute( + "INSERT INTO guardrail_audit (audit_id, ts, datasource, status, refusal_kind, sql, " + "row_count, execution_ms, correlation_id, source) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + ( + record.audit_id, + record.ts, + record.datasource, + record.status, + record.refusal_kind, + record.sql, + record.row_count, + record.execution_ms, + record.correlation_id, + record.source, + ), + ) + self._store.commit() + # --------------------------------------------------------------------------- # Read path — the admin activity views (read-only). diff --git a/packages/agami-core/src/oss_adapters.py b/packages/agami-core/src/oss_adapters.py index b9161ad..f3a76b1 100644 --- a/packages/agami-core/src/oss_adapters.py +++ b/packages/agami-core/src/oss_adapters.py @@ -1,7 +1,8 @@ """OSS default adapters for the four ports. These defaults make the local product run out of the box — the single-tenant resolver, the -file/jsonl activity sink, presence-only auth, and warn-only governance. They live in agami-core +file/jsonl activity sink, presence-only auth, and no-op governance (warn-only posture, no rules +wired). They live in agami-core (not the ``agami-oss-adapters`` placeholder) so ``pip install agami-core`` is enough to run locally; richer adapters (a Postgres sink, real auth providers, enforcement) are supplied by their own consumers. @@ -13,10 +14,14 @@ import json from pathlib import Path +from typing import TYPE_CHECKING import agami_paths from contracts import QueryExecutionRecord -from ports import GovernanceVerdict, Org, Principal +from ports import Org, Principal + +if TYPE_CHECKING: + from guardrail import Verdict def _append_jsonl(path: Path, record: dict) -> bool: @@ -69,9 +74,13 @@ def validate_token(self, token: str) -> Principal | None: return Principal(subject=self._subject) if (token or "").strip() else None -class WarnOnlyGovernancePolicy: - """Default ``GovernancePolicy`` — never blocks (``allowed=True``); any warnings are advisory - ("basic governance warning"). Enforcement is a paid tier.""" +class NoopGovernancePolicy: + """Default ``GovernancePolicy`` for OSS: a **no-op** — ``evaluate`` emits **no** findings (an empty + ``Verdict`` list), so nothing is annotated, rewritten, or blocked. The OSS posture is warn-only + (governance never enforces), but the default adapter has no governance rules wired, so there is + nothing to warn about — hence the no-op name. A paid tier supplies its own adapter that returns + governance-class ``Verdict``s; whether any blocks is ``guardrail.policy(verdict, tier)``'s call, + not this adapter's.""" - def evaluate(self, ctx: object | None = None) -> GovernanceVerdict: - return GovernanceVerdict(allowed=True, warnings=[]) + def evaluate(self, ctx: object | None = None) -> list[Verdict]: + return [] diff --git a/packages/agami-core/src/ports.py b/packages/agami-core/src/ports.py index 4e7f6d4..1f8ed61 100644 --- a/packages/agami-core/src/ports.py +++ b/packages/agami-core/src/ports.py @@ -6,7 +6,7 @@ - ``ActivitySink`` — where query-execution records go (file by default) - ``OrgResolver`` — single vs multi tenancy as a config flag, not a schema fork - ``AuthProvider`` — bearer token → principal (presence by default) - - ``GovernancePolicy`` — warn-only by default; enforcement is a paid concern + - ``GovernancePolicy`` — no-op by default (warn-only posture, no rules wired); enforcement is paid - ``Executor`` — the connect-and-run step, *behind* the shared guard (built-in by default; a consumer injects a pooled/RBAC/tunnel executor without forking the guard) @@ -14,16 +14,18 @@ no import coupling back to core. The OSS default adapters live in ``oss_adapters`` (so the local product runs out of the box); a downstream consumer supplies its own. -The seam value types (``Org`` / ``Principal`` / ``GovernanceVerdict``) are stdlib dataclasses, not -pydantic models, so this module imports with **zero dependencies** — a consumer can depend on the -seams without pulling the model deps. The wire shapes that need validation (the 4-tool I/O) live -in ``contracts`` (pydantic). Each type is kept minimal — only what a default adapter or a -consumer needs. +The seam value types (``Org`` / ``Principal``) are stdlib dataclasses, not pydantic models, so +this module imports with **zero dependencies** — a consumer can depend on the seams without +pulling the model deps. The governance seam speaks the shared ``guardrail.Verdict`` (also +stdlib-only), so a ``GovernancePolicy`` folds its findings into the one ``Envelope`` every +surface returns instead of a bespoke result type. The wire shapes that need validation (the +4-tool I/O) live in ``contracts`` (pydantic). Each type is kept minimal — only what a default +adapter or a consumer needs. """ from __future__ import annotations -from dataclasses import dataclass, field +from dataclasses import dataclass from typing import TYPE_CHECKING, Protocol, runtime_checkable if TYPE_CHECKING: @@ -31,6 +33,7 @@ # consumer that needs only the seams) import without the pydantic model deps. With # `from __future__ import annotations` the method annotations are lazy strings, and # @runtime_checkable only checks method *names*, so isinstance() works without these. + # (``guardrail`` is stdlib-only, but is kept here too so ``import ports`` stays minimal.) from contracts import QueryExecutionRecord # ``ExecResult`` is defined in ``execute_sql`` (not here): it is the executor's result type and @@ -38,6 +41,7 @@ # cannot import ``ports`` at runtime. Referencing it under TYPE_CHECKING keeps the ``Executor`` # annotation resolvable for type-checkers without a runtime import cycle. from execute_sql import ExecResult + from guardrail import Verdict # --------------------------------------------------------------------------- # Seam value types (minimal — a consumer extends them when it needs more) @@ -61,15 +65,6 @@ class Principal: subject: str -@dataclass(frozen=True) -class GovernanceVerdict: - """The outcome of a governance check. The default is **warn-only** — ``allowed`` is always - True and ``warnings`` is advisory; only a paid enforcement tier may set allowed=False.""" - - allowed: bool = True - warnings: list[str] = field(default_factory=list) - - # --------------------------------------------------------------------------- # The five ports # --------------------------------------------------------------------------- @@ -105,11 +100,16 @@ def validate_token(self, token: str) -> Principal | None: ... @runtime_checkable class GovernancePolicy(Protocol): - """Evaluate a request and return a ``GovernanceVerdict`` (warnings; never blocks by default). + """Evaluate a request and return governance-class ``guardrail.Verdict``s (annotations that + fold into ``Envelope.warnings``; empty ⇒ nothing to say). Whether any of them actually blocks + or rewrites is ``guardrail.policy(verdict, tier)``'s call, not this adapter's — this is the + swappable *source* of governance findings (the governance injection seam). - OSS default = warn-only ("basic governance warning"); enforcement is a paid tier.""" + OSS default is a **no-op** — it returns an empty list (no findings, so nothing is annotated or + blocked). A paid tier supplies its own adapter that emits governance ``Verdict``s; the tier + posture (OSS warns · SaaS recommends · Enterprise enforces) is applied by ``policy``, not here.""" - def evaluate(self, ctx: object | None = None) -> GovernanceVerdict: ... + def evaluate(self, ctx: object | None = None) -> list[Verdict]: ... @runtime_checkable diff --git a/packages/agami-core/src/semantic_model/runtime.py b/packages/agami-core/src/semantic_model/runtime.py index a9cc073..1553ba9 100644 --- a/packages/agami-core/src/semantic_model/runtime.py +++ b/packages/agami-core/src/semantic_model/runtime.py @@ -35,6 +35,8 @@ from difflib import SequenceMatcher from typing import Any, Callable, Optional +from guardrail import Verdict, safety_verdict + try: import sqlglot from sqlglot import expressions as exp @@ -109,6 +111,7 @@ def build_guard_context(sql: str, org: Organization) -> "GuardContext | None": model_table_index=_model_table_index(org), ) + # Ambiguity threshold — "ask, don't guess" when top-two are within this delta. AMBIGUITY_DELTA = 0.15 @@ -213,9 +216,7 @@ def resolve_entities( "subject_area": area_name, "score": round(score, 3), "primary_mapping": ( - {"table": primary.table, "column": primary.column} - if primary - else None + {"table": primary.table, "column": primary.column} if primary else None ), "value_pattern": ent.value_pattern, }, @@ -343,9 +344,7 @@ def identify_entity( return IdentifyResult( "clarify", effective, - question_template=( - f"'{literal}' could be a {names}. Which did you mean?" - ), + question_template=(f"'{literal}' could be a {names}. Which did you mean?"), ) # no pattern/probe match: fallback probe of small-cardinality candidates @@ -433,8 +432,12 @@ class SensitiveCheckResult: suggestion: Optional[str] = None def as_dict(self) -> dict[str, Any]: - return {"action": self.action, "columns": self.columns, - "reason": self.reason, "suggestion": self.suggestion} + return { + "action": self.action, + "columns": self.columns, + "reason": self.reason, + "suggestion": self.suggestion, + } def _sensitive_by_table(org: Organization) -> tuple[dict[str, set[str]], set[str]]: @@ -498,8 +501,9 @@ def _output_selects(node: "exp.Expression") -> list["exp.Select"]: return [] -def check_sensitive_projection(sql: str, org: Organization, - ctx: "GuardContext | None" = None) -> SensitiveCheckResult: +def check_sensitive_projection( + sql: str, org: Organization, ctx: "GuardContext | None" = None +) -> SensitiveCheckResult: """Refuse a query that PROJECTS a `sensitive` column's raw values; allow the column in COUNT, filters, GROUP BY, and joins. Degrades to allow when sqlglot is unavailable or the SQL doesn't parse (same posture as the fan/chasm pass). @@ -540,7 +544,9 @@ def check_sensitive_projection(sql: str, org: Organization, offending.add(f"{tbl}.{col.name}") # same-named column on a non-sensitive table → not offending # (b) `*` / `t.*` that would expand a directly-FROM'd table holding sensitive cols - is_star = isinstance(proj, exp.Star) or (isinstance(proj, exp.Column) and isinstance(proj.this, exp.Star)) + is_star = isinstance(proj, exp.Star) or ( + isinstance(proj, exp.Column) and isinstance(proj.this, exp.Star) + ) if is_star: qualifier = proj.table if isinstance(proj, exp.Column) else None tables = {scope.get(qualifier, qualifier)} if qualifier else direct @@ -554,10 +560,11 @@ def check_sensitive_projection(sql: str, org: Organization, return SensitiveCheckResult( "refuse", columns=cols, - reason="query projects raw values of sensitive column(s): " + ", ".join(cols) - + " — sensitive columns may be counted or filtered, not output raw.", + reason="query projects raw values of sensitive column(s): " + + ", ".join(cols) + + " — sensitive columns may be counted or filtered, not output raw.", suggestion="Aggregate it (e.g. COUNT(DISTINCT )) for a count, or omit it and " - "select the entity's non-sensitive key (e.g. id) instead.", + "select the entity's non-sensitive key (e.g. id) instead.", ) @@ -574,20 +581,9 @@ def check_sensitive_projection(sql: str, org: Organization, # --------------------------------------------------------------------------- -@dataclass -class TableScopeResult: - action: str # "allow" | "refuse" - offending_tables: list[str] = field(default_factory=list) # bare names not in the model - reason: str = "" - suggestion: Optional[str] = None - - def as_dict(self) -> dict[str, Any]: - return {"action": self.action, "offending_tables": self.offending_tables, - "reason": self.reason, "suggestion": self.suggestion} - - -def check_table_scope(sql: str, org: Organization, - ctx: "GuardContext | None" = None) -> TableScopeResult: +def check_table_scope( + sql: str, org: Organization, ctx: "GuardContext | None" = None +) -> Verdict | None: """Refuse a query that references a table not declared in the semantic model. Only *physical* table references count: CTE names (defined by WITH) and @@ -603,23 +599,26 @@ def check_table_scope(sql: str, org: Organization, declared tables also allows — there is nothing to scope against. """ if not _HAVE_SQLGLOT: - return TableScopeResult("allow") - allow = {name.lower() for name in (ctx.model_table_index if ctx is not None else _model_table_index(org))} + return None + allow = { + name.lower() + for name in (ctx.model_table_index if ctx is not None else _model_table_index(org)) + } if not allow: - return TableScopeResult("allow") + return None if ctx is not None: tree = ctx.tree else: try: tree = sqlglot.parse_one(sql, error_level="ignore") except Exception: - return TableScopeResult("allow") + return None # A set operation (UNION/INTERSECT/EXCEPT) parses to exp.Union, not exp.Select, # so gate on "contains a SELECT" rather than "is a SELECT" — otherwise every # set-operation arm would bypass the guard. A non-SELECT statement has no SELECT # node and still degrades to allow (the upstream read-only guard owns those). if tree is None or tree.find(exp.Select) is None: - return TableScopeResult("allow") + return None cte_names = {c.alias_or_name.lower() for c in tree.find_all(exp.CTE)} offending: set[str] = set() @@ -630,17 +629,15 @@ def check_table_scope(sql: str, org: Organization, if name.lower() not in allow: offending.add(name) if not offending: - return TableScopeResult("allow") + return None tables = sorted(offending) - return TableScopeResult( - "refuse", - offending_tables=tables, - reason="query references table(s) not in the semantic model: " - + ", ".join(tables) - + " — only tables declared in the model may be queried.", - suggestion="Add the table to the model (agami-connect / '/agami-model'), " - "or remove it from the query.", + return safety_verdict( + "table_scope", + "query references table(s) not in the semantic model: " + + ", ".join(tables) + + " — only tables declared in the model may be queried.", + "Add the table to the model (agami-connect / '/agami-model'), or remove it from the query.", ) @@ -655,17 +652,7 @@ def check_table_scope(sql: str, org: Organization, # --------------------------------------------------------------------------- -@dataclass -class StarCheckResult: - action: str # "allow" | "refuse" - reason: str = "" - suggestion: Optional[str] = None - - def as_dict(self) -> dict[str, Any]: - return {"action": self.action, "reason": self.reason, "suggestion": self.suggestion} - - -def check_no_select_star(sql: str, ctx: "GuardContext | None" = None) -> StarCheckResult: +def check_no_select_star(sql: str, ctx: "GuardContext | None" = None) -> Verdict | None: """Refuse a query whose projection list contains `*` or `t.*`. A star defeats column-level scoping (an undeclared column hides behind it) and @@ -679,42 +666,33 @@ def check_no_select_star(sql: str, ctx: "GuardContext | None" = None) -> StarChe not a SELECT-bearing statement (the upstream read-only guard owns non-SELECTs). """ if not _HAVE_SQLGLOT: - return StarCheckResult("allow") + return None if ctx is not None: tree = ctx.tree else: try: tree = sqlglot.parse_one(sql, error_level="ignore") except Exception: - return StarCheckResult("allow") + return None if tree is None or tree.find(exp.Select) is None: - return StarCheckResult("allow") + return None for select in tree.find_all(exp.Select): for proj in select.expressions: - if isinstance(proj, exp.Star) or (isinstance(proj, exp.Column) and isinstance(proj.this, exp.Star)): - return StarCheckResult( - "refuse", - reason="query uses SELECT * — every column must be named so it can be " - "checked against the semantic model.", - suggestion="List the columns explicitly instead of '*'.", + if isinstance(proj, exp.Star) or ( + isinstance(proj, exp.Column) and isinstance(proj.this, exp.Star) + ): + return safety_verdict( + "no_select_star", + "query uses SELECT * — every column must be named so it can be " + "checked against the semantic model.", + "List the columns explicitly instead of '*'.", ) - return StarCheckResult("allow") - - -@dataclass -class ColumnScopeResult: - action: str # "allow" | "refuse" - columns: list[str] = field(default_factory=list) # offending "table.column" / "column" - reason: str = "" - suggestion: Optional[str] = None - - def as_dict(self) -> dict[str, Any]: - return {"action": self.action, "columns": self.columns, - "reason": self.reason, "suggestion": self.suggestion} + return None -def check_column_scope(sql: str, org: Organization, - ctx: "GuardContext | None" = None) -> ColumnScopeResult: +def check_column_scope( + sql: str, org: Organization, ctx: "GuardContext | None" = None +) -> Verdict | None: """Refuse a query that references a column not declared on the table it binds to. Strict where a column visibly binds to a declared physical table — qualified by @@ -732,19 +710,19 @@ def check_column_scope(sql: str, org: Organization, a SELECT, or the model declares no columns. """ if not _HAVE_SQLGLOT: - return ColumnScopeResult("allow") + return None colidx = ctx.column_index if ctx is not None else _column_index(org) if not colidx: - return ColumnScopeResult("allow") + return None if ctx is not None: tree = ctx.tree else: try: tree = sqlglot.parse_one(sql, error_level="ignore") except Exception: - return ColumnScopeResult("allow") + return None if tree is None or tree.find(exp.Select) is None: - return ColumnScopeResult("allow") + return None # case-insensitive declared-column index: lower(table) -> {lower(column)} declared = {t.lower(): {c.lower() for c in cols} for t, cols in colidx.items()} @@ -775,9 +753,9 @@ def _select_chain(node): # whether it reads from a CTE ref / derived subquery (→ a bare column we can't # match may be that source's output, so fail-open). alias_by_select: dict[int, dict[str, str]] = {} # id(select) -> {alias -> bare physical table} - direct_phys: dict[int, set[str]] = {} # id(select) -> {bare physical table read directly} - has_derived: dict[int, bool] = {} # id(select) -> reads a CTE ref / derived subquery directly - output_by_select: dict[int, set[str]] = {} # id(select) -> {select-list output alias} + direct_phys: dict[int, set[str]] = {} # id(select) -> {bare physical table read directly} + has_derived: dict[int, bool] = {} # id(select) -> reads a CTE ref / derived subquery directly + output_by_select: dict[int, set[str]] = {} # id(select) -> {select-list output alias} for tbl in tree.find_all(exp.Table): name = (tbl.name or "").lower() if not name: @@ -828,7 +806,11 @@ def _select_chain(node): # unqualified: judge against the tables its own SELECT reads directly if sel is not None and lname in output_by_select.get(id(sel), set()): continue # a select-list output alias of THIS select, not a base column - local = {t for t in direct_phys.get(id(sel), set()) if t in declared} if sel is not None else set() + local = ( + {t for t in direct_phys.get(id(sel), set()) if t in declared} + if sel is not None + else set() + ) if any(lname in declared[t] for t in local): continue # declared on a table this select reads (possibly ambiguous — don't false-reject) if sel is not None and has_derived.get(id(sel), False): @@ -838,15 +820,15 @@ def _select_chain(node): offending.add(name) if not offending: - return ColumnScopeResult("allow") + return None cols = sorted(offending) - return ColumnScopeResult( - "refuse", - columns=cols, - reason="query references column(s) not in the semantic model: " + ", ".join(cols) - + " — only columns declared on the model's tables may be queried.", - suggestion="Add the column to the model (agami-connect / '/agami-model'), " - "or remove it from the query.", + return safety_verdict( + "column_scope", + "query references column(s) not in the semantic model: " + + ", ".join(cols) + + " — only columns declared on the model's tables may be queried.", + "Add the column to the model (agami-connect / '/agami-model'), " + "or remove it from the query.", ) @@ -858,7 +840,9 @@ def _cardinality_index(org: Organization) -> list[Relationship]: return rels -def _one_side_facing_many(rels: list[Relationship], table: str, others: set[str]) -> list[Relationship]: +def _one_side_facing_many( + rels: list[Relationship], table: str, others: set[str] +) -> list[Relationship]: """Relationships where `table` is the ONE side and a joined `other` is the MANY side.""" hits = [] for r in rels: @@ -881,8 +865,9 @@ def _many_side_facing_one(rels: list[Relationship], table: str, dim: str) -> boo return False -def pre_flight_check(sql: str, org: Organization, - ctx: "GuardContext | None" = None) -> PreFlightResult: +def pre_flight_check( + sql: str, org: Organization, ctx: "GuardContext | None" = None +) -> PreFlightResult: """Detect fan-trap / chasm-trap and decide rewrite-vs-refuse-vs-allow. A set operation (UNION/INTERSECT/EXCEPT) parses to exp.SetOperation, not exp.Select; @@ -905,13 +890,26 @@ def pre_flight_check(sql: str, org: Organization, res = _preflight_select(arm, org, arm.sql(), allow_rewrite=False, ctx=ctx) if res.risk and res.action == "refuse": # tie the arm's diagnosis back to the full set-operation query - return PreFlightResult(res.risk, "refuse", sql, reason=res.reason, - suggestion=res.suggestion, triggering_joins=res.triggering_joins) - return PreFlightResult(None, "allow", sql, reason="no fan/chasm or aggregation issue in any arm") + return PreFlightResult( + res.risk, + "refuse", + sql, + reason=res.reason, + suggestion=res.suggestion, + triggering_joins=res.triggering_joins, + ) + return PreFlightResult( + None, "allow", sql, reason="no fan/chasm or aggregation issue in any arm" + ) -def _preflight_select(tree: "exp.Select", org: Organization, sql: str, allow_rewrite: bool, - ctx: "GuardContext | None" = None) -> PreFlightResult: +def _preflight_select( + tree: "exp.Select", + org: Organization, + sql: str, + allow_rewrite: bool, + ctx: "GuardContext | None" = None, +) -> PreFlightResult: """Fan/chasm + aggregation-semantics analysis of a SINGLE SELECT. `sql` is that select's own text (used for the join rewrite + messages). When `allow_rewrite` is False (a set-operation arm), a rewriteable fan trap is refused, not rewritten. @@ -1029,15 +1027,17 @@ def _column_index(org: Organization) -> dict[str, dict[str, Column]]: return idx -def _lookup_column(col: "exp.Column", scope: dict[str, str], - colidx: dict[str, dict[str, Column]]) -> Optional[Column]: +def _lookup_column( + col: "exp.Column", scope: dict[str, str], colidx: dict[str, dict[str, Column]] +) -> Optional[Column]: t = _resolve_col_table(col, scope) if t and col.name in colidx.get(t, {}): return colidx[t][col.name] # bare column, ambiguous table: only safe if exactly one in-scope table defines it if not t: - owners = [tt for tt, cols in colidx.items() - if tt in set(scope.values()) and col.name in cols] + owners = [ + tt for tt, cols in colidx.items() if tt in set(scope.values()) and col.name in cols + ] if len(owners) == 1: return colidx[owners[0]][col.name] return None @@ -1087,8 +1087,9 @@ def _semi_additive_columns(org: Organization) -> dict[tuple[str, str], "Metric"] return out -def _groups_by_time(tree: "exp.Select", scope: dict[str, str], - colidx: dict[str, dict[str, Column]]) -> bool: +def _groups_by_time( + tree: "exp.Select", scope: dict[str, str], colidx: dict[str, dict[str, Column]] +) -> bool: """Does the query GROUP BY a time grain — a date/timestamp column, or a DATE_TRUNC/EXTRACT/TO_CHAR/DATE_PART over one?""" grp = tree.args.get("group") @@ -1102,7 +1103,10 @@ def _groups_by_time(tree: "exp.Select", scope: dict[str, str], def _check_aggregation_semantics( - tree: "exp.Select", org: Organization, scope: dict[str, str], sql: str, + tree: "exp.Select", + org: Organization, + scope: dict[str, str], + sql: str, ctx: "GuardContext | None" = None, ) -> Optional[PreFlightResult]: colidx = ctx.column_index if ctx is not None else _column_index(org) @@ -1120,17 +1124,23 @@ def _check_aggregation_semantics( if c is None: continue cls = getattr(c, "aggregation", "unknown") - bad = (is_sum and cls in ("averageable", "dimension")) or (is_avg and cls == "dimension") + bad = (is_sum and cls in ("averageable", "dimension")) or ( + is_avg and cls == "dimension" + ) if bad: verb = "SUM" if is_sum else "AVG" return PreFlightResult( - "bad_aggregation", "refuse", sql, + "bad_aggregation", + "refuse", + sql, reason=( f"{verb}({col.name}) is meaningless: {col.name!r} is classified " f"`{cls}` (" - + ("a rate/ratio/price — summing it has no meaning" - if cls == "averageable" - else "an identifier/code, not a measure") + + ( + "a rate/ratio/price — summing it has no meaning" + if cls == "averageable" + else "an identifier/code, not a measure" + ) + ")." ), suggestion=( @@ -1156,7 +1166,9 @@ def _check_aggregation_semantics( if mm is not None: how = mm.semi_additive_agg or "last" return PreFlightResult( - "semi_additive", "refuse", sql, + "semi_additive", + "refuse", + sql, reason=( f"SUM({col.name}) across time is wrong: {col.name!r} backs the " f"semi-additive metric {mm.name!r} ({mm.non_additive_dimensions}) — " @@ -1259,9 +1271,14 @@ def assemble_receipt( (LLM-discovered) metrics to `metrics` after the fact. """ receipt: dict[str, Any] = { - "sql": sql, "model_version": model_version, - "tables_used": [], "relationships": [], "metrics": [], - "named_filters": [], "assumptions": [], "warnings": [], + "sql": sql, + "model_version": model_version, + "tables_used": [], + "relationships": [], + "metrics": [], + "named_filters": [], + "assumptions": [], + "warnings": [], } if not _HAVE_SQLGLOT: return receipt @@ -1272,7 +1289,7 @@ def assemble_receipt( if tree is None: return receipt - scope = _tables_in_scope(tree) # alias/name -> bare table name + scope = _tables_in_scope(tree) # alias/name -> bare table name used = set(scope.values()) tidx = _model_table_index(org) @@ -1282,12 +1299,14 @@ def assemble_receipt( continue t, _area = info ph = t.performance_hints - receipt["tables_used"].append({ - "qname": f"{t.schema_name}.{t.name}" if t.schema_name else t.name, - "rows": (ph.estimated_row_count if ph else None), - "rows_as_of": (ph.estimated_row_count_at if ph else None), - "freshness": freshness, - }) + receipt["tables_used"].append( + { + "qname": f"{t.schema_name}.{t.name}" if t.schema_name else t.name, + "rows": (ph.estimated_row_count if ph else None), + "rows_as_of": (ph.estimated_row_count_at if ph else None), + "freshness": freshness, + } + ) warnings: list[str] = [] for sa in org.subject_areas: @@ -1296,38 +1315,46 @@ def assemble_receipt( fq = (r.from_schema + ".") if (r.cross_schema and r.from_schema) else "" tq = (r.to_schema + ".") if (r.cross_schema and r.to_schema) else "" label = f"{fq}{r.from_table} → {tq}{r.to_table}" - receipt["relationships"].append({ - "name": f"{r.from_table}_to_{r.to_table}", - "from_to": label, - "cardinality": r.relationship, - "confidence": r.confidence, - "review_state": r.review_state, - "origin": "fk" if r.confidence == "confirmed" else "introspect_heuristic", - "signed_off_by": r.signed_off_by, - "signed_off_role": r.signed_off_role, - "signed_off_at": r.signed_off_at, - "cross_schema": r.cross_schema, - "on": r.on, - }) + receipt["relationships"].append( + { + "name": f"{r.from_table}_to_{r.to_table}", + "from_to": label, + "cardinality": r.relationship, + "confidence": r.confidence, + "review_state": r.review_state, + "origin": "fk" if r.confidence == "confirmed" else "introspect_heuristic", + "signed_off_by": r.signed_off_by, + "signed_off_role": r.signed_off_role, + "signed_off_at": r.signed_off_at, + "cross_schema": r.cross_schema, + "on": r.on, + } + ) if r.review_state != "approved": warnings.append(f"Used an unreviewed join ({label}).") nsql = _norm_sql(sql) for sa in org.subject_areas: for met in sa.metrics: - binding = next((b for b in (met.bindings or {}).values() - if b and _norm_sql(b) in nsql), "") + binding = next( + (b for b in (met.bindings or {}).values() if b and _norm_sql(b) in nsql), "" + ) if not binding: continue - receipt["metrics"].append({ - "name": met.name, "area": sa.name, - "definition_prose": met.calculation, "expression": binding, - "confidence": met.confidence, "review_state": met.review_state, - "origin": getattr(met, "source", None), - "signed_off_by": met.signed_off_by, - "signed_off_role": met.signed_off_role, - "signed_off_at": met.signed_off_at, - }) + receipt["metrics"].append( + { + "name": met.name, + "area": sa.name, + "definition_prose": met.calculation, + "expression": binding, + "confidence": met.confidence, + "review_state": met.review_state, + "origin": getattr(met, "source", None), + "signed_off_by": met.signed_off_by, + "signed_off_role": met.signed_off_role, + "signed_off_at": met.signed_off_at, + } + ) # metrics get their own approve/change banner — no duplicate warning line. # assumptions: the load-bearing columns the answer leaned on whose description is @@ -1344,9 +1371,9 @@ def _tables_defining(cname: str) -> list[str]: for col in tree.find_all(exp.Column): if not col.name: continue - if col.table: # qualified -> resolve via alias scope + if col.table: # qualified -> resolve via alias scope ref_cols.add((scope.get(col.table, col.table), col.name)) - else: # unqualified -> attribute only if unambiguous + else: # unqualified -> attribute only if unambiguous cands = _tables_defining(col.name) if len(cands) == 1: ref_cols.add((cands[0], col.name)) @@ -1368,14 +1395,19 @@ def _tables_defining(cname: str) -> list[str]: receipt["assumptions"] = (unknown + unval)[:3] if warnings: - warnings.append("Review these unreviewed joins in the agami model explorer " - "(/agami-model, or say 'open the review queue').") + warnings.append( + "Review these unreviewed joins in the agami model explorer " + "(/agami-model, or say 'open the review queue')." + ) receipt["warnings"] = warnings if applied_filters: receipt["default_filters_applied"] = applied_filters if pre_flight and pre_flight.risk: - receipt["pre_flight"] = {"risk": pre_flight.risk, "action": pre_flight.action, - "reason": pre_flight.reason} + receipt["pre_flight"] = { + "risk": pre_flight.risk, + "action": pre_flight.action, + "reason": pre_flight.reason, + } return receipt @@ -1642,8 +1674,17 @@ def resolve_result_units(org: Organization, sql: str) -> dict[str, str]: has_star = any(isinstance(p, exp.Star) or p.find(exp.Star) is not None for p in projs) # Unit-preserving scalar ops: an aggregate/round of a currency is still that currency. - _preserving = (exp.Sum, exp.Avg, exp.Min, exp.Max, exp.Round, exp.Coalesce, - exp.Abs, exp.Ceil, exp.Floor) + _preserving = ( + exp.Sum, + exp.Avg, + exp.Min, + exp.Max, + exp.Round, + exp.Coalesce, + exp.Abs, + exp.Ceil, + exp.Floor, + ) def _unit_of(e) -> Optional[str]: """Dimensional analysis: the unit a (sub)expression produces, or None when it's @@ -1657,12 +1698,12 @@ def _unit_of(e) -> Optional[str]: if isinstance(e, exp.Column): return col_units.get((e.name or "").lower()) if isinstance(e, exp.Count): - return None # a count is dimensionless + return None # a count is dimensionless if isinstance(e, _preserving): return _unit_of(e.this) if isinstance(e, exp.Div): num, den = _unit_of(e.this), _unit_of(e.expression) - return num if (num and not den) else None # currency/count → currency; X/X → none + return num if (num and not den) else None # currency/count → currency; X/X → none if isinstance(e, exp.Mul): return _unit_of(e.this) or _unit_of(e.expression) # currency × scalar → currency if isinstance(e, (exp.Add, exp.Sub)): @@ -1671,8 +1712,11 @@ def _unit_of(e) -> Optional[str]: # fallback: a single distinct column unit, only if no count/division muddies it if e.find(exp.Count) is not None or e.find(exp.Div) is not None: return None - units = {col_units[c.name.lower()] for c in e.find_all(exp.Column) - if c.name and c.name.lower() in col_units} + units = { + col_units[c.name.lower()] + for c in e.find_all(exp.Column) + if c.name and c.name.lower() in col_units + } return next(iter(units)) if len(units) == 1 else None def _unit_for(proj) -> Optional[str]: @@ -1687,9 +1731,9 @@ def _unit_for(proj) -> Optional[str]: continue name = proj.alias_or_name if name: - out[name] = unit # by output name (aliased / named columns) + out[name] = unit # by output name (aliased / named columns) if not has_star: - out[f"#{i}"] = unit # by position — covers unaliased MAX(amount) etc. + out[f"#{i}"] = unit # by position — covers unaliased MAX(amount) etc. return out diff --git a/packages/agami-core/src/sql_guard.py b/packages/agami-core/src/sql_guard.py index 802cd0b..df32c69 100644 --- a/packages/agami-core/src/sql_guard.py +++ b/packages/agami-core/src/sql_guard.py @@ -13,14 +13,16 @@ are neutral enough to be safe across the other supported engines. `check_read_only(sql)` returns `None` when the SQL is a single safe read-only -statement, else a short human-readable reason string. Callers decide how to wrap it -(the MCP tools attach `kind="permission"`). +statement, else a safety `Verdict` the shared executor maps to a refusal +(`kind="permission"`). The rejection ladder itself lives in `_read_only_reason`. """ from __future__ import annotations import re +from guardrail import Verdict, safety_verdict + # Hard cap on SQL length. Prevents a compromised client from POSTing a multi-MB # SQL blob that takes the parser / planner / this gate down a slow path. Real # analytics SQL fits in ~10KB; 50KB is conservative. @@ -231,7 +233,7 @@ def _neutralize(sql: str) -> str: ) -def check_read_only(sql: str | None) -> str | None: +def _read_only_reason(sql: str | None) -> str | None: """Return None if `sql` is a single safe read-only statement, else a reason string. Rejection ladder (each step has its own message so the caller can correct): @@ -295,3 +297,20 @@ def check_read_only(sql: str | None) -> str | None: "process-control / sleep / remote-SQL functions are blocked" ) return None + + +def check_read_only(sql: str | None) -> Verdict | None: + """Return ``None`` if ``sql`` is a single safe read-only statement, else a safety ``Verdict``. + + Thin wrapper over :func:`_read_only_reason` (which owns the rejection ladder). A fired gate + becomes a safety-class verdict; the shared executor maps it to a refusal (``kind=permission``). + """ + reason = _read_only_reason(sql) + if reason is None: + return None + return safety_verdict( + "read_only", + reason, + "Send a single read-only SELECT / WITH...SELECT — no DML, DDL, transaction/session " + "control, or multiple statements.", + ) diff --git a/packages/agami-core/src/tools.py b/packages/agami-core/src/tools.py index b52f5b3..f1e28b7 100644 --- a/packages/agami-core/src/tools.py +++ b/packages/agami-core/src/tools.py @@ -33,11 +33,13 @@ from contextvars import ContextVar from pathlib import Path from typing import Any +from uuid import uuid4 # --------------------------------------------------------------------------- # Paths & config resolution (mirrors execute_sql.py / file-layout.md exactly) # --------------------------------------------------------------------------- import agami_paths +from guardrail import Envelope, Refusal, Verdict # Secrets + per-user state live under /local/. Re-resolved after bootstrap() in main(). AGAMI_LOCAL = agami_paths.local_dir() @@ -45,6 +47,7 @@ CONFIG_PATH = agami_paths.config_path() QUERY_LOG = agami_paths.query_log_path() TOOL_CALL_LOG = AGAMI_LOCAL / "tool_calls.jsonl" +GUARDRAIL_AUDIT_LOG = AGAMI_LOCAL / "guardrail_audit.jsonl" SERVER_NAME = "agami" @@ -94,13 +97,14 @@ def bootstrap_paths() -> None: installs never create ~/.agami, so the migration is a no-op once there's nothing to move (it's a backward-compat shim, safe to drop in a later cleanup). Every entrypoint calls this so the paths reflect the resolved (possibly migrated) artifacts dir.""" - global AGAMI_LOCAL, CREDENTIALS_PATH, CONFIG_PATH, QUERY_LOG, TOOL_CALL_LOG + global AGAMI_LOCAL, CREDENTIALS_PATH, CONFIG_PATH, QUERY_LOG, TOOL_CALL_LOG, GUARDRAIL_AUDIT_LOG agami_paths.bootstrap() AGAMI_LOCAL = agami_paths.local_dir() CREDENTIALS_PATH = agami_paths.credentials_path() CONFIG_PATH = agami_paths.config_path() QUERY_LOG = agami_paths.query_log_path() TOOL_CALL_LOG = AGAMI_LOCAL / "tool_calls.jsonl" + GUARDRAIL_AUDIT_LOG = AGAMI_LOCAL / "guardrail_audit.jsonl" def _load_config() -> dict[str, Any]: @@ -184,8 +188,8 @@ def _db_type_for(profile: str, creds: dict[str, dict[str, str]]) -> str: # --------------------------------------------------------------------------- -def check_read_only(sql: str) -> str | None: - """Return None if the SQL is a safe single read-only statement, else a reason string. +def check_read_only(sql: str) -> Verdict | None: + """Return None if the SQL is a safe single read-only statement, else a safety Verdict. Thin fail-fast wrapper over the shared `sql_guard` — the SAME gate the executor (`execute_sql.py`) enforces — so the stdio server, the HTTP/OAuth server, the @@ -638,7 +642,12 @@ def _table_contexts(org, table_names: list[str], L, index=None) -> dict[str, Any def _schema_payload( - org, profile: str, mode: str, matched: list[str], metrics: dict[str, tuple[Any, str | None]], L, + org, + profile: str, + mode: str, + matched: list[str], + metrics: dict[str, tuple[Any, str | None]], + L, index=None, ) -> dict[str, Any]: """Build the structured schema payload at the given verbosity. `metric_index` + `large_tables` @@ -897,16 +906,12 @@ def set_injected_executor(executor: Any | None) -> None: def _finalize_execution( columns: list, data_rows: list, truncated: bool, *, profile: str, sql: str, - execution_ms: int, args: dict[str, Any], + execution_ms: int, args: dict[str, Any], audit_id: str, ) -> str: - """Shape a successful result (units + exact-render markdown + trust receipt), log the execution - through the single sink, and return the tool JSON. Shared by both execution paths — the subprocess - fork and the in-process executor — so a **successful** query returns the identical result envelope - whichever ran it. (A guard *refusal* is not yet identical across paths: the subprocess surfaces - execute_sql's stderr JSON as the remediation, while the in-process path returns a clean generic - refusal with the structured detail in the server log. Full structured-refusal parity needs - `_model_safety` to *return* its envelope — it currently writes it to stderr, pinned by the - fail-closed guard tests — so it's tracked as a follow-up, not folded into this seam.)""" + """Shape a successful result (units + exact-render markdown + trust receipt) into the ok + ``Envelope``, record it through the guardrail-audit chokepoint, and return the Envelope JSON. + Shared by BOTH execution paths — the subprocess fork and the in-process executor — so a + successful query returns the identical envelope whichever ran it.""" # Deterministic, exact rendering — so the numbers a user verifies don't depend on # how the host LLM chooses to format them. `markdown` is the table to display # verbatim; `rows` stays raw (exact CSV values) for charting / programmatic use. @@ -918,7 +923,7 @@ def _finalize_execution( except Exception: markdown = None - result = { + data = { "columns": columns, "rows": data_rows, "row_count": len(data_rows), @@ -933,6 +938,8 @@ def _finalize_execution( # (offer to approve/correct via the save_correction tool). "receipt": _resolve_receipt(profile, sql), } + # A bounded (transfer-capped) result is a valid answer that is flagged, not a refusal. + applied = [{"row_cap": len(data_rows)}] if truncated else [] # Log the execution through the single chokepoint: the DB sink when AGAMI_DB_URL is set (one # query_executions row), else the local jsonl the skills use. Best-effort either way. @@ -946,15 +953,22 @@ def _finalize_execution( "source": "mcp_server", } ) - return json.dumps(result, indent=2, default=str) + return _finish( + Envelope.ok(data=data, applied=applied, audit_id=audit_id), + args, + sql=sql, + execution_ms=execution_ms, + ) def _run_in_process( sql: str, profile: str, area: str | None, max_rows: int | None, executor: Any -) -> tuple[list, list, bool] | dict: +) -> tuple[list, list, bool] | Refusal: """Run through the in-process executor behind the shared guarded envelope (no subprocess, no CSV - round-trip). Returns ``(columns, data_rows, truncated)`` on success, or an error dict on a guard - refusal / execution failure — the same error shape the subprocess branch produces. + round-trip). Returns ``(columns, data_rows, truncated)`` on success, or the typed ``Refusal`` on a + guard refusal / execution failure — the SAME refusal the subprocess path produces (``GuardRefused`` + now carries the typed ``Refusal``, incl. the model-safety detail), so the two paths build an + identical refused Envelope. Rows are textualized to match the subprocess CSV wire (``None`` → ``""``, else ``str``) so the two paths return observably identical JSON. Native-typed rows are a deliberately deferred decision @@ -962,34 +976,20 @@ def _run_in_process( import execute_sql # The per-call cap rides execute_sql's `_max_rows_override` ContextVar (ACE-028) — request-scoped, - # so concurrent in-process queries with different caps don't stomp each other. Set/reset via the - # token (the `_current_org_ctx` pattern); `run_blocking` copied this request's context into the - # worker thread, so the set is isolated to this call. + # so concurrent in-process queries with different caps don't stomp each other. `run_blocking` + # copied this request's context into the worker thread, so the set is isolated to this call. cap_token = execute_sql._max_rows_override.set(max_rows) try: result = execute_sql.execute_guarded(sql, profile, area, executor=executor) except execute_sql.GuardRefused as refusal: - # A read-only refusal (envelope present) is already caught by tool_execute_sql's upstream - # check_read_only fast-fail, so in practice only the model-safety branch (envelope None) is - # reached here; both are handled for defence-in-depth. - if refusal.envelope is not None: - return {"error": refusal.envelope["error"]} - # A model-safety refusal wrote its structured detail to the server log (stderr); surface a - # clean refusal here. (The subprocess path instead surfaces that stderr JSON as remediation — - # the not-yet-identical refusal envelope tracked as a follow-up.) - return {"error": {"kind": "permission", - "remediation": "Query refused by the semantic-model safety pass " - "(see server logs for the specific rule)."}} + return refusal.refusal # typed Refusal (read-only OR model-safety) — identical to subprocess except execute_sql.ExecutorError as exc: - return {"error": {"kind": _classify_exit(exc.code), "remediation": exc.msg}} + return Refusal(_classify_exit(exc.code), reason=exc.msg) except SystemExit as exc: - # Defence-in-depth. The known credential/DSN failures now raise ExecutorError (handled above, - # carrying their detailed message), so this net catches only a residual/future sys.exit deep - # in a driver — ensuring an in-process query can never take down the host; it becomes a - # fail-closed tool error instead. + # Defence-in-depth: a residual/future sys.exit deep in a driver becomes a fail-closed refusal + # rather than taking down the host process. code = exc.code if isinstance(exc.code, int) else 2 - return {"error": {"kind": _classify_exit(code), - "remediation": "Datasource configuration error."}} + return Refusal(_classify_exit(code), reason="Datasource configuration error.") finally: execute_sql._max_rows_override.reset(cap_token) @@ -1002,6 +1002,69 @@ def _run_in_process( return columns, data_rows, truncated +def _refusal_from_stderr(stderr: str | None, returncode: int) -> Refusal: + """Build a Refusal from the executor's stderr. A guardrail refusal is a `{"refusal": {...}}` + line (safety / model gates); anything else is an execution/config failure classified by exit + code. Raw stderr is used only as the reason for the latter — never as a guardrail reason.""" + for line in (stderr or "").splitlines(): + line = line.strip() + if line.startswith("{") and '"refusal"' in line: + try: + r = json.loads(line).get("refusal") + except ValueError: + continue + if isinstance(r, dict) and r.get("kind"): + return Refusal( + kind=r["kind"], + reason=r.get("reason", ""), + remediation=r.get("remediation", ""), + ) + return Refusal( + kind=_classify_exit(returncode), + reason=(stderr or "").strip() or "execute_sql.py failed", + ) + + +def _envelope_json(env: Envelope) -> str: + return json.dumps(env.as_dict(), indent=2, default=str) + + +def _finish( + env: Envelope, + args: dict[str, Any], + *, + sql: str | None = None, + execution_ms: int | None = None, +) -> str: + """Record the guardrail audit row keyed by ``env.audit_id``, then return the Envelope JSON. + + This is the single chokepoint the audit trail relies on: every ok/refused execute_sql result + records its verdict trail here, on BOTH surfaces — unlike ``tool_calls``, which only the HTTP + transport writes. Best-effort: building OR writing the audit record never breaks the tool.""" + try: + d = env.as_dict() + data = d.get("data") if isinstance(d.get("data"), dict) else {} + _record_guardrail_audit( + { + "audit_id": env.audit_id, + "ts": _now_iso(), + "datasource": args.get("datasource"), + "status": env.status, + "refusal_kind": (d.get("refusal") or {}).get("kind"), + "sql": sql, + "row_count": data.get("row_count"), + "execution_ms": execution_ms, + "correlation_id": args.get("correlation_id"), + "source": "mcp_server", + } + ) + except Exception as exc: + # best-effort — a logging failure must never break an otherwise-good result — but surface a + # record-BUILD failure once (a Refusal/Envelope shape drift) so it isn't invisible forever. + _warn_once("guardrail_audit_build", f"guardrail audit record not built: {type(exc).__name__}") + return _envelope_json(env) + + def tool_execute_sql(args: dict[str, Any]) -> str: """Local analog of Ask Agami `execute_sql`: run a read-only SELECT locally. @@ -1011,24 +1074,30 @@ def tool_execute_sql(args: dict[str, Any]) -> str: Two execution paths behind the same guard: the default forks the execute_sql subprocess (isolation, byte-identical local/single-user); an injected executor (AH-012) runs in-process with - native rows. Both funnel through `_finalize_execution`, so a **successful** query's result - envelope is identical either way (a guard refusal's envelope is not yet identical across paths — - see `_finalize_execution` and the tracked structured-refusal-parity follow-up). + native rows. Both funnel through the same envelope assembly, so BOTH a **successful** result and a + **refusal** are identical either way: a refusal is reconstructed as the same typed `Refusal` + (kind + reason + remediation) on both paths — the subprocess path parses the executor's + `{"refusal": ...}` stderr line, the in-process path catches the typed `GuardRefused` — and both + return `Envelope.refused(...)` through `_finish`. """ + audit_id = uuid4().hex sql = args.get("sql") if not isinstance(sql, str) or not sql.strip(): - return json.dumps( - {"error": {"kind": "other", "remediation": "Pass a non-empty `sql` string."}} + return _finish( + Envelope.refused( + Refusal("other", "Pass a non-empty `sql` string."), audit_id=audit_id + ), + args, ) - reason = check_read_only(sql) - if reason is not None: - return json.dumps( - { - "error": {"kind": "permission", "remediation": reason}, - "sql": sql, - }, - indent=2, + verdict = check_read_only(sql) + if verdict is not None: + return _finish( + Envelope.refused( + Refusal("permission", verdict.detail, verdict.remediation), audit_id=audit_id + ), + args, + sql=sql, ) profile = resolve_profile(args.get("datasource")) @@ -1049,12 +1118,17 @@ def tool_execute_sql(args: dict[str, Any]) -> str: started = time.monotonic() outcome = _run_in_process(sql, profile, area, max_rows, _INJECTED_EXECUTOR) execution_ms = int((time.monotonic() - started) * 1000) - if isinstance(outcome, dict): # guard refusal / execution error - return json.dumps({**outcome, "sql": sql, "execution_ms": execution_ms}, indent=2) + if isinstance(outcome, Refusal): # guard refusal / execution error + return _finish( + Envelope.refused(outcome, audit_id=audit_id), + args, + sql=sql, + execution_ms=execution_ms, + ) columns, data_rows, truncated = outcome return _finalize_execution( columns, data_rows, truncated, - profile=profile, sql=sql, execution_ms=execution_ms, args=args, + profile=profile, sql=sql, execution_ms=execution_ms, args=args, audit_id=audit_id, ) # The model safety pass (fan/chasm pre-flight + default_filters) runs inside @@ -1078,22 +1152,21 @@ def tool_execute_sql(args: dict[str, Any]) -> str: timeout=240, ) except subprocess.TimeoutExpired: - return json.dumps( - {"error": {"kind": "timeout", "remediation": "Query exceeded 240s."}, "sql": sql} + return _finish( + Envelope.refused(Refusal("timeout", "Query exceeded 240s."), audit_id=audit_id), + args, + sql=sql, ) execution_ms = int((time.monotonic() - started) * 1000) if proc.returncode != 0: - return json.dumps( - { - "error": { - "kind": _classify_exit(proc.returncode), - "remediation": (proc.stderr or "").strip() or "execute_sql.py failed", - }, - "sql": sql, - "execution_ms": execution_ms, - }, - indent=2, + return _finish( + Envelope.refused( + _refusal_from_stderr(proc.stderr, proc.returncode), audit_id=audit_id + ), + args, + sql=sql, + execution_ms=execution_ms, ) # Parse the RFC-4180 CSV emitted on stdout. @@ -1110,7 +1183,7 @@ def tool_execute_sql(args: dict[str, Any]) -> str: return _finalize_execution( columns, data_rows, truncated, - profile=profile, sql=sql, execution_ms=execution_ms, args=args, + profile=profile, sql=sql, execution_ms=execution_ms, args=args, audit_id=audit_id, ) @@ -1122,6 +1195,24 @@ def _now_iso() -> str: return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") +_AUDIT_WARNED: set[str] = set() + + +def _warn_once(key: str, msg: str) -> None: + """Emit a ONE-TIME (per-process, per-key) warning to stderr. The audit sinks are best-effort so an + audit-write failure never breaks a query — but "best-effort AND silent" means a PERSISTENTLY dead + sink (a DB role without INSERT, an unwritable log dir) loses the security record undetectably. This + surfaces the first such failure without spamming a line per query. Keep `msg` value-free (an + exception TYPE, never its text) so a driver message can't leak a DSN/schema/value through it.""" + if key in _AUDIT_WARNED: + return + _AUDIT_WARNED.add(key) + try: + sys.stderr.write(f"[agami] {msg}\n") + except Exception: + pass + + def _append_jsonl(path: Path, record: dict[str, Any]) -> bool: try: path.parent.mkdir(parents=True, exist_ok=True) @@ -1164,9 +1255,10 @@ def record_tool_call( ) -> None: """Record one MCP tool call to the activity log (the transport calls this for **every** tool). The audit-grade fields are server-observed; `success`/`row_count`/`error_kind` are derived from the - result (execute_sql returns an `{"error": ...}` body on a bad query without raising). The self-report - fields (`user_question`/`agent_query`/`thread_id`) are whatever Claude supplied — may be None. - **Best-effort and never raises** — a logging failure must not break the tool.""" + result: execute_sql returns a response Envelope (`status`/`refusal`/`data`); the other tools return + a `{"error": ...}` body on failure. The self-report fields (`user_question`/`agent_query`/ + `thread_id`) are whatever Claude supplied — may be None. **Best-effort and never raises** — a + logging failure must not break the tool.""" args = arguments or {} success, row_count, error_kind = True, None, None if raised: @@ -1175,10 +1267,16 @@ def record_tool_call( try: parsed = json.loads(result_text) if result_text else None if isinstance(parsed, dict): - if isinstance(parsed.get("error"), dict): + if parsed.get("status") == "refused": # execute_sql response Envelope + success = False + error_kind = (parsed.get("refusal") or {}).get("kind") or "error" + elif isinstance(parsed.get("error"), dict): # other tools' {"error": ...} body success = False error_kind = parsed["error"].get("kind") or "error" - row_count = parsed.get("row_count") + data = parsed.get("data") + row_count = ( + data.get("row_count") if isinstance(data, dict) else parsed.get("row_count") + ) except (ValueError, TypeError): pass _record_tool_call( @@ -1224,6 +1322,30 @@ def _record_tool_call(rec: dict[str, Any]) -> None: pass # best-effort: never fail the tool because logging failed +def _record_guardrail_audit(rec: dict[str, Any]) -> None: + """Write a guardrail audit record through the DB sink (AGAMI_DB_URL) or the local jsonl. Wrapped + so the whole thing is best-effort — even opening the store can't surface an error to the caller.""" + try: + from store import Store + + store = Store.from_env() + if store is None: + if not _append_jsonl(GUARDRAIL_AUDIT_LOG, rec): + _warn_once("guardrail_audit_jsonl", "guardrail audit not recorded: audit log unwritable") + return + try: + from contracts import GuardrailAuditRecord + from model_store import DbActivitySink + + DbActivitySink(store).record_guardrail_audit(GuardrailAuditRecord(**rec)) + finally: + store.close() + except Exception as exc: + # best-effort: never fail the tool because logging failed — but surface a PERSISTENT DB-sink + # failure once (type only, no message text) so a dead audit trail is observable, not silent. + _warn_once("guardrail_audit_db", f"guardrail audit not recorded (sink error): {type(exc).__name__}") + + # --------------------------------------------------------------------------- # Tool registry (name → (handler, description, inputSchema)) # --------------------------------------------------------------------------- diff --git a/plugins/agami/lib/execute_sql.py b/plugins/agami/lib/execute_sql.py index 737e293..341e2ca 100644 --- a/plugins/agami/lib/execute_sql.py +++ b/plugins/agami/lib/execute_sql.py @@ -53,6 +53,7 @@ from typing import TYPE_CHECKING, Any import agami_paths +from guardrail import Refusal, Verdict if TYPE_CHECKING: # ``Executor`` is the 5th port; imported only for type-checkers. At runtime ``execute_sql`` never @@ -82,6 +83,7 @@ def _resolve_default_profile() -> str: if CONFIG_PATH.exists(): try: import json as _json + cfg = _json.loads(CONFIG_PATH.read_text()) active = cfg.get("active_profile") if isinstance(active, str) and active: @@ -126,13 +128,14 @@ def __init__(self, msg: str, *, code: int) -> None: class GuardRefused(Exception): - """A guard refusal short-circuiting the envelope. ``envelope`` is the JSON error object the - caller must emit (the read-only guard's ``permission`` refusal), or ``None`` when the refusal - JSON was already written to stderr by ``_model_safety`` (carry only the exit ``code``).""" + """A guard refusal short-circuiting the executor. Carries the typed ``Refusal`` (the shared + guardrail shape) so BOTH callers build the SAME envelope from it: the subprocess ``main`` emits + ``Envelope.refused(refusal)`` JSON to stderr, and the in-process MCP handler returns it directly. + No stderr round-trip — so a model-safety refusal keeps its structured detail on both paths.""" - def __init__(self, envelope: dict | None, *, code: int) -> None: + def __init__(self, refusal: Refusal, *, code: int) -> None: super().__init__() - self.envelope = envelope + self.refusal = refusal self.code = code @@ -259,9 +262,12 @@ def _load_credentials(profile: str) -> dict[str, str]: # Schemes we accept. Strip "+driver" suffixes (e.g. postgresql+asyncpg, postgres+psycopg2). _POSTGRES_SCHEMES = {"postgres", "postgresql"} _MYSQL_SCHEMES = {"mysql", "mariadb"} -_REDSHIFT_SCHEMES = {"redshift"} # speaks Postgres wire protocol; port 5439, SSL required -_SNOWFLAKE_SCHEMES = {"snowflake"} # native CLI (snowsql) + snowflake-connector-python -_BIGQUERY_SCHEMES = {"bigquery", "bq"} # google-cloud-bigquery — auth via service-account JSON or ADC +_REDSHIFT_SCHEMES = {"redshift"} # speaks Postgres wire protocol; port 5439, SSL required +_SNOWFLAKE_SCHEMES = {"snowflake"} # native CLI (snowsql) + snowflake-connector-python +_BIGQUERY_SCHEMES = { + "bigquery", + "bq", +} # google-cloud-bigquery — auth via service-account JSON or ADC def _parse_dsn(dsn: str) -> dict[str, str]: @@ -330,7 +336,7 @@ def _parse_dsn(dsn: str) -> dict[str, str]: return out elif base_scheme == "sqlite": # sqlite:///absolute/path or sqlite:relative/path - path = dsn[len("sqlite://"):] + path = dsn[len("sqlite://") :] if path.startswith("/"): path = path[1:] if path[1:2] == "/" else path # handle `sqlite:////abs` # Trailing path normalization @@ -555,9 +561,7 @@ def _run_bigquery(creds: dict[str, str], sql: str) -> ExecResult: except Exception: pass try: - creds_obj = service_account.Credentials.from_service_account_file( - sa_path_expanded - ) + creds_obj = service_account.Credentials.from_service_account_file(sa_path_expanded) client_kwargs["credentials"] = creds_obj except Exception as e: raise ExecutorError(f"BigQuery credentials load failed: {e}", code=2) @@ -606,6 +610,7 @@ def _run_bigquery(creds: dict[str, str], sql: str) -> ExecResult: def _run_sqlite(creds: dict[str, str], sql: str) -> ExecResult: import sqlite3 # always available in stdlib + _require(creds, "path") path = os.path.expanduser(creds["path"]) try: @@ -632,9 +637,12 @@ def _run_sqlserver(creds: dict[str, str], sql: str) -> ExecResult: _require(creds, "host", "user", "password") try: conn = pymssql.connect( - server=creds["host"], port=int(creds.get("port", 1433)), - user=creds["user"], password=creds["password"], - database=creds.get("database", ""), login_timeout=15, + server=creds["host"], + port=int(creds.get("port", 1433)), + user=creds["user"], + password=creds["password"], + database=creds.get("database", ""), + login_timeout=15, ) except Exception as e: raise ExecutorError(f"SQL Server connect failed: {e}", code=4) @@ -662,8 +670,9 @@ def _run_oracle(creds: dict[str, str], sql: str) -> ExecResult: dsn = creds.get("dsn") or creds.get("url") if not dsn: _require(creds, "host", "service_name") - dsn = oracledb.makedsn(creds["host"], int(creds.get("port", 1521)), - service_name=creds["service_name"]) + dsn = oracledb.makedsn( + creds["host"], int(creds.get("port", 1521)), service_name=creds["service_name"] + ) try: conn = oracledb.connect(user=creds["user"], password=creds["password"], dsn=dsn) except Exception as e: @@ -694,7 +703,8 @@ def _run_databricks(creds: dict[str, str], sql: str) -> ExecResult: _require(creds, "host", "http_path", "token") try: conn = dbsql.connect( - server_hostname=creds["host"], http_path=creds["http_path"], + server_hostname=creds["host"], + http_path=creds["http_path"], access_token=creds["token"], ) except Exception as e: @@ -725,9 +735,13 @@ def _run_trino(creds: dict[str, str], sql: str) -> ExecResult: if creds.get("password"): auth = trino.auth.BasicAuthentication(creds["user"], creds["password"]) conn = trino.dbapi.connect( - host=creds["host"], port=int(creds.get("port", 8080)), user=creds["user"], - catalog=creds.get("catalog"), schema=creds.get("schema"), - http_scheme="https" if creds.get("password") else "http", auth=auth, + host=creds["host"], + port=int(creds.get("port", 8080)), + user=creds["user"], + catalog=creds.get("catalog"), + schema=creds.get("schema"), + http_scheme="https" if creds.get("password") else "http", + auth=auth, ) except Exception as e: raise ExecutorError(f"Trino connect failed: {e}", code=4) @@ -857,11 +871,10 @@ def _resolve_guard_model(profile: str): the DB when one is configured (hosted — the `/artifacts` disk mount may be absent), else the on-disk YAML (local). Returns an `Organization` or None if neither is available. - The DB import is lazy AND env-guarded on purpose: the local executor runs from a stdlib-lean - mirror that does not ship `store`/`model_store`, so we only reach for them when a DB is set. - Any DB-load failure degrades to disk rather than crashing the executor.""" - from semantic_model import loader as L - + The DB/loader imports are lazy AND (for the DB) env-guarded on purpose: the local executor runs + from a stdlib-lean mirror that does not ship `store`/`model_store`, so we only reach for them when + a DB is set; and the loader import sits inside the disk-path try so an import failure degrades to + None (hosted then fails closed) rather than crashing the executor.""" # Any load failure below degrades to the next source (DB → disk → None), silently: a freeform # error line here would (a) leak DB connection details from the exception and (b) precede the # JSON refusal `_model_safety` emits when both sources are absent on hosted, breaking the @@ -877,30 +890,45 @@ def _resolve_guard_model(profile: str): try: org = _load_db(store, profile) finally: - store.close() + try: + store.close() + except Exception: + pass # a close error must not discard a model that loaded fine (→ false refusal) if org is not None: return org except Exception: pass # DB unreachable/misconfigured -> fall through to disk - root = Path(os.environ.get("AGAMI_ARTIFACTS_DIR") or (Path.home() / "agami-artifacts")) / profile + root = ( + Path(os.environ.get("AGAMI_ARTIFACTS_DIR") or (Path.home() / "agami-artifacts")) / profile + ) if (root / "org.yaml").exists(): try: + from semantic_model import loader as L + return L.load_organization(root) except Exception: - pass # unparseable/absent on disk -> None (hosted then fails closed) + pass # unparseable/absent on disk, or loader import failure -> None (hosted fails closed) return None -def _model_safety(sql: str, profile: str, area: str | None): +def _refusal_from_verdict(kind: str, verdict: Verdict) -> Refusal: + """Build the shared ``Refusal`` from a safety ``Verdict``. A safety verdict always refuses + (``policy(safety)`` is ``reject`` in every tier), so the guard chain refuses unconditionally on a + non-None verdict. ``kind`` is the caller-supplied refusal kind (the verdict's ``rule`` names the + gate, e.g. ``read_only``; the refusal ``kind`` is the outward vocabulary, e.g. ``permission``).""" + return Refusal(kind=kind, reason=verdict.detail, remediation=verdict.remediation) + + +def _model_safety(sql: str, profile: str, area: str | None) -> tuple[str, Refusal | None]: """Semantic-model safety pass before execution: fan-trap / chasm-trap pre-flight + default_filters auto-application, over a model resolved from the DB (hosted) or disk (local). - Returns (sql_to_run, exit_code). exit_code is None to continue, or an int to - short-circuit (a refusal the caller must consume). Inert (returns the SQL unchanged) when the - model package isn't importable, or — on the LOCAL path only — when there is no model yet. On the - HOSTED path a model that can't be resolved fails closed (refuses), never runs unguarded (ACE-051). - """ + Returns (sql_to_run, refusal). ``refusal`` is None to continue, or a typed ``Refusal`` the caller + (``execute_guarded``) raises as a ``GuardRefused`` — so both the subprocess and in-process paths + build the same envelope from it. Inert (returns the SQL unchanged) when the model package isn't + importable, or — on the LOCAL path only — when there is no model yet. On the HOSTED path a model + that can't be resolved fails closed (refuses), never runs unguarded (ACE-051).""" try: from semantic_model import runtime as RT except Exception: @@ -910,11 +938,11 @@ def _model_safety(sql: str, profile: str, area: str | None): # sqlglot-unavailable / unparseable-SQL degrade-to-allow is a distinct fail-open owned by # ACE-037, not closed here.) if _hosted(): - json.dump({"error": {"kind": "model_unavailable", "reason": - "semantic-model package not importable; refusing to run unguarded on the " - "hosted server"}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + return sql, Refusal( + "model_unavailable", + "semantic-model package not importable; refusing to run unguarded on the " + "hosted server", + ) return sql, None # local: model package not available -> no-op org = _resolve_guard_model(profile) @@ -922,11 +950,11 @@ def _model_safety(sql: str, profile: str, area: str | None): if _hosted(): # Fail closed: a served query with no resolvable model must be refused, never run with # the fan/chasm/scope/PII guards silently off. - json.dump({"error": {"kind": "model_unavailable", "reason": - "no semantic model could be resolved (checked DB and disk); refusing to run " - "unguarded on the hosted server"}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + return sql, Refusal( + "model_unavailable", + "no semantic model could be resolved (checked DB and disk); refusing to run " + "unguarded on the hosted server", + ) return sql, None # local: no model yet -> no-op (unchanged) # Build the shared guard context ONCE — parse the SQL + build each model index a single @@ -939,37 +967,24 @@ def _model_safety(sql: str, profile: str, area: str | None): # declares; any other table in the connected database is refused. Runs FIRST # so the fan/chasm and sensitive checks below only evaluate in-scope tables. ts = RT.check_table_scope(sql, org, ctx=ctx) - if ts.action == "refuse": - json.dump({"error": {"kind": "table_out_of_scope", "tables": ts.offending_tables, - "reason": ts.reason, "suggestion": ts.suggestion}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + if ts is not None: + return sql, _refusal_from_verdict("table_out_of_scope", ts) # SELECT * ban — force every projected column to be named, so the column-scope # guard below can check what is actually returned (and nothing hides behind *). star = RT.check_no_select_star(sql, ctx=ctx) - if star.action == "refuse": - json.dump({"error": {"kind": "select_star", - "reason": star.reason, "suggestion": star.suggestion}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + if star is not None: + return sql, _refusal_from_verdict("select_star", star) # Column-scope guard — a column that binds to a declared table must be one that # table declares (a hallucinated column, or a physical column the model excluded). cs = RT.check_column_scope(sql, org, ctx=ctx) - if cs.action == "refuse": - json.dump({"error": {"kind": "column_out_of_scope", "columns": cs.columns, - "reason": cs.reason, "suggestion": cs.suggestion}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + if cs is not None: + return sql, _refusal_from_verdict("column_out_of_scope", cs) pf = RT.pre_flight_check(sql, org, ctx=ctx) if pf.risk and pf.action == "refuse": - json.dump({"error": {"kind": "preflight_refused", "risk": pf.risk, - "reason": pf.reason, "suggestion": pf.suggestion, - "triggering_joins": pf.triggering_joins}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + return sql, Refusal("preflight_refused", pf.reason, pf.suggestion or "") if pf.risk and pf.action == "auto_rewrite" and pf.rewritten_sql: sys.stderr.write(f"[agami] auto-corrected {pf.risk}: ran rewritten SQL. {pf.reason}\n") sql = pf.rewritten_sql @@ -981,10 +996,7 @@ def _model_safety(sql: str, profile: str, area: str | None): # path happened to read a prose rule). Aggregates / filters / joins are allowed. sens = RT.check_sensitive_projection(sql, org, ctx=ctx) if sens.action == "refuse": - json.dump({"error": {"kind": "sensitive_columns", "columns": sens.columns, - "reason": sens.reason, "suggestion": sens.suggestion}}, sys.stderr) - sys.stderr.write("\n") - return sql, 1 + return sql, Refusal("sensitive_columns", sens.reason, sens.suggestion or "") new_sql, applied = RT.apply_default_filters(sql, org, area=area, ctx=ctx) if applied: @@ -1128,20 +1140,19 @@ def execute_guarded( ``no_safety``, which skips only the semantic-model pass, never write/RCE/DoS protection) -> semantic-model safety pass (fan/chasm pre-flight + scope + PII + ``default_filters`` rewrite) -> resolve the datasource -> ``executor.execute(vetted_sql, …)``. The executor only ever receives - SQL both guards have passed. Raises ``GuardRefused`` on a refusal (the read-only refusal carries - its JSON envelope for the caller to emit; a model-safety refusal already wrote its JSON to stderr - and carries only the exit code) and ``ExecutorError`` on a connect/run failure — so the - subprocess ``main`` and the in-process MCP handler apply the same guard and surface errors - identically. The row cap rides the request-scoped ``_max_rows_override`` ContextVar the caller sets.""" + SQL both guards have passed. Raises ``GuardRefused`` carrying the typed ``Refusal`` on a refusal, + and ``ExecutorError`` on a connect/run failure — so the subprocess ``main`` and the in-process MCP + handler apply the same guard and build the same envelope. The row cap rides the request-scoped + ``_max_rows_override`` ContextVar the caller sets.""" import sql_guard - reason = sql_guard.check_read_only(sql) - if reason is not None: - raise GuardRefused({"error": {"kind": "permission", "remediation": reason}}, code=1) + verdict = sql_guard.check_read_only(sql) + if verdict is not None: + raise GuardRefused(_refusal_from_verdict("permission", verdict), code=1) if not no_safety: - sql, rc = _model_safety(sql, profile, area) - if rc is not None: - raise GuardRefused(None, code=rc) + sql, refusal = _model_safety(sql, profile, area) + if refusal is not None: + raise GuardRefused(refusal, code=1) creds = _load_credentials(profile) return executor.execute(sql, creds, profile=profile) @@ -1164,13 +1175,23 @@ def main() -> int: src = p.add_mutually_exclusive_group(required=True) src.add_argument("--sql", help="SQL statement (use --sql-file for SQL with special characters)") src.add_argument("--sql-file", help="Path to a file containing one SQL statement") - p.add_argument("--area", default=None, - help="Subject area for the semantic-model safety pass (pre-flight + default_filters).") - p.add_argument("--no-safety", action="store_true", - help="Skip the semantic-model pre-flight / default_filters pass.") - p.add_argument("--max-rows", type=int, default=None, - help="Lower the row cap for this call (never raises it). Effective cap = " - "min(this, AGAMI_SQL_MAX_ROWS) — the env is the deployment cap, default 1000.") + p.add_argument( + "--area", + default=None, + help="Subject area for the semantic-model safety pass (pre-flight + default_filters).", + ) + p.add_argument( + "--no-safety", + action="store_true", + help="Skip the semantic-model pre-flight / default_filters pass.", + ) + p.add_argument( + "--max-rows", + type=int, + default=None, + help="Lower the row cap for this call (never raises it). Effective cap = " + "min(this, AGAMI_SQL_MAX_ROWS) — the env is the deployment cap, default 1000.", + ) args = p.parse_args() # Per-call cap (ACE-044); the sink reads it via _resolve_row_cap. No token/reset kept: main() is @@ -1187,18 +1208,18 @@ def main() -> int: # Route through the single guarded envelope with the built-in executor: guard -> model-safety -> # resolve -> connect-and-run, returning native rows we then serialize to stdout as CSV (the - # subprocess wire). Same guard, same verdicts, same connect-per-query behaviour as before — the - # split just makes the connect-and-run step swappable in-process (AH-012). The guard is the hard - # security gate for EVERY caller (both MCP servers, the agami-query skill, cron), NOT bypassable - # via --no-safety (which skips only the semantic-model pass, never write/RCE/DoS protection). + # subprocess wire). The guard is the hard security gate for EVERY caller (both MCP servers, the + # agami-query skill, cron), NOT bypassable via --no-safety (which skips only the semantic-model + # pass, never write/RCE/DoS protection). try: result = execute_guarded( sql, profile, args.area, executor=BUILTIN_EXECUTOR, no_safety=args.no_safety ) except GuardRefused as refusal: - if refusal.envelope is not None: # read-only refusal: emit its JSON (model-safety already did) - json.dump(refusal.envelope, sys.stderr) - sys.stderr.write("\n") + # Emit the shared refusal as one JSON line to stderr — the subprocess wire the MCP tool relays + # into a refused Envelope, and a direct CLI caller reads alongside the exit code. + json.dump({"refusal": refusal.refusal.as_dict()}, sys.stderr) + sys.stderr.write("\n") return refusal.code except ExecutorError as exc: sys.stderr.write(f"{exc.msg}\n") diff --git a/plugins/agami/lib/guardrail.py b/plugins/agami/lib/guardrail.py new file mode 100644 index 0000000..a040886 --- /dev/null +++ b/plugins/agami/lib/guardrail.py @@ -0,0 +1,213 @@ +"""Shared guardrail contract — ``Verdict``, ``policy``, and the response ``Envelope``. + +The one result shape for the three SQL-execution guardrails: **safety** (reject) · +**data-protection** (mask) · **governance** (warn). Every gate produces a ``Verdict``; one +``policy`` maps a verdict to an action by class + deployment tier; and every surface returns one +``Envelope``. Each guardrail builds to these types and never defines its own result shape. + +**Stdlib-only, dataclasses only — keep it that way.** This module is vendored into the +marketplace plugin (``plugins/agami/lib/``) and imported by ``ports`` (which must stay +dependency-free), so it must never grow a third-party import. The vendored-purity guard depends +on this staying pure. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any, Literal + +# ── Controlled vocabularies ────────────────────────────────────────────────── +Class = Literal["safety", "data_protection", "governance"] +Severity = Literal["high", "medium", "low"] +Certainty = Literal["provable", "heuristic", "uncertain"] +Action = Literal["allow", "reject", "mask", "row_filter", "rewrite", "warn"] +Status = Literal["ok", "refused"] +Tier = Literal["oss", "saas", "enterprise"] + +# Value sets exposed for validation + tests (the `action` set and the class set are stable). +CLASSES: tuple[str, ...] = ("safety", "data_protection", "governance") +ACTIONS: tuple[str, ...] = ("allow", "reject", "mask", "row_filter", "rewrite", "warn") + +# Every refusal kind the system emits, grouped by origin. `Refusal.kind` is an open ``str`` — +# operational failures (`syntax` / `auth` / `driver_missing`) come from the DB driver, not a fixed +# guardrail vocabulary — so this documents the known set (and a test pins the emit sites against it), +# rather than a `Literal` that nothing at runtime (no mypy in CI) would actually enforce. This is the +# canonical vocabulary for the whole safe-SQL feature: a few kinds are FORWARD-DECLARED here for the +# gates that land in the dependent slices on top of this contract (`unscopable_sql` — fail-closed +# scoping; `resource_limit` — the statement timeout; `recon` — the metadata deny-list), so the set is +# defined once, in the base, and the emit-site test only asserts emitted ⊆ documented (never the +# reverse, which would fail here until those slices land). +REFUSAL_KINDS: tuple[str, ...] = ( + # safety — integrity / confinement / object-scope / availability / fail-closed + "permission", + "table_out_of_scope", + "column_out_of_scope", + "select_star", + "unscopable_sql", # forward-declared: emitted by the fail-closed-scoping slice + "resource_limit", # forward-declared: emitted by the per-statement-timeout slice + "recon", # forward-declared: emitted by the recon deny-list slice + "model_unavailable", + # data-protection / governance — emitted by those gates + "sensitive_columns", + "preflight_refused", + # operational / execution failures — from the executor + DB driver + "timeout", + "dsn", + "driver_missing", + "auth", + "syntax", + "other", +) + + +@dataclass(frozen=True) +class Verdict: + """What a single gate returns. + + ``cls`` is serialized as ``class`` (a Python keyword can't be a field name). ``certainty`` + is the axis ``policy`` keys on: **safety** emits ``uncertain`` ⇒ reject (fail-closed on + doubt); **governance** emits ``heuristic`` ⇒ warn (undecidable) or ``provable`` ⇒ + reject/rewrite at an enforcing tier. ``severity`` is load-bearing only on the provable + governance path. ``rewritten_sql`` is present only when the action is ``rewrite``. + """ + + cls: Class + rule: str + severity: Severity + certainty: Certainty + detail: str + remediation: str + rewritten_sql: str | None = None + + def as_dict(self) -> dict[str, Any]: + d: dict[str, Any] = { + "class": self.cls, + "rule": self.rule, + "severity": self.severity, + "certainty": self.certainty, + "detail": self.detail, + "remediation": self.remediation, + } + if self.rewritten_sql is not None: + d["rewritten_sql"] = self.rewritten_sql + return d + + +def safety_verdict(rule: str, detail: str, remediation: str) -> Verdict: + """Build a safety-class ``Verdict``. Safety findings are always ``provable`` + ``high`` — a + safety gate that fires is deterministic and always rejects. ``rule`` names the gate (e.g. + ``read_only``, ``table_scope``, ``column_scope``, ``no_select_star``).""" + return Verdict( + cls="safety", + rule=rule, + severity="high", + certainty="provable", + detail=detail, + remediation=remediation, + ) + + +@dataclass(frozen=True) +class Refusal: + """The ``refusal`` block of a refused ``Envelope``: why the query was rejected, with a + remediation. + + ``reason`` is ALWAYS value-free — it never carries raw SQL or raw DB driver text (schema / column + / value names, a DSN), which must not cross the boundary between the model and the customer's + database. A **guardrail** refusal (a safety or model gate) reasons from the model; an + **operational** failure (a DB syntax / connection error surfaced by the executor) is classified + into a fixed value-free reason, with the raw driver text captured separately for the server-side + audit trail only. (The operational-error sanitization is wired by the recon/error-hardening + slice; the tool layer classifies via ``_classify_db_error`` on both surfaces.)""" + + kind: str # one of REFUSAL_KINDS — an open str (operational kinds come from the DB driver) + reason: str + remediation: str = "" + + def as_dict(self) -> dict[str, Any]: + return {"kind": self.kind, "reason": self.reason, "remediation": self.remediation} + + +@dataclass(frozen=True) +class Envelope: + """The one shape every surface returns. + + ``data`` is absent when refused; ``applied`` records transforms/bounds actually applied — today + only the fetch bound ``{row_cap: N}`` is wired; ``{mask: col}`` / ``{row_filter: expr}`` / + ``{rewrite: reason}`` land with the data-protection / governance gates. ``warnings`` are + governance annotations (``Verdict``s); ``refusal`` is present iff ``status == 'refused'``; + ``audit_id`` references the recorded verdict trail. + """ + + status: Status + audit_id: str = "" + data: Any | None = None + applied: list[dict[str, Any]] = field(default_factory=list) + warnings: list[Verdict] = field(default_factory=list) + refusal: Refusal | None = None + + @classmethod + def refused( + cls, + refusal: Refusal, + *, + audit_id: str = "", + warnings: list[Verdict] | None = None, + ) -> Envelope: + return cls(status="refused", audit_id=audit_id, refusal=refusal, warnings=warnings or []) + + @classmethod + def ok( + cls, + data: Any, + *, + audit_id: str = "", + applied: list[dict[str, Any]] | None = None, + warnings: list[Verdict] | None = None, + ) -> Envelope: + return cls( + status="ok", + audit_id=audit_id, + data=data, + applied=applied or [], + warnings=warnings or [], + ) + + def as_dict(self) -> dict[str, Any]: + d: dict[str, Any] = {"status": self.status, "audit_id": self.audit_id} + if self.data is not None: + d["data"] = self.data + d["applied"] = list(self.applied) + d["warnings"] = [w.as_dict() for w in self.warnings] + if self.refusal is not None: + d["refusal"] = self.refusal.as_dict() + return d + + +def policy(verdict: Verdict, tier: Tier = "oss") -> Action: + """Map a verdict to an action by class + deployment tier. + + The class is read from ``verdict.cls`` (it already lives on the verdict, so it is not passed + as a separate argument). + + - **safety** → always ``reject``, every tier; ``certainty == 'uncertain'`` also ⇒ ``reject`` + (fail-closed on doubt). Fully implemented here. + - **data_protection** → ``mask`` / ``row_filter`` where the gate names one, else ``reject`` + (fail-closed). *Stub:* the data-protection gates fill this branch in later work; until then + it fails closed. + - **governance** → graded by (severity, certainty, tier): ``reject``/``rewrite`` only for a + ``provable`` verdict under an enforcing tier, else ``warn`` (OSS warns · SaaS recommends · + Enterprise enforces). *Stub:* the governance gates fill this branch in later work. + """ + if verdict.cls == "safety": + # Safety is absolute: reject in every tier; uncertainty is already a reject (fail-closed). + return "reject" + if verdict.cls == "data_protection": + # Stub — fill mask/row_filter selection later. Fail closed until then (the safe default). + return "reject" + if verdict.cls == "governance": + # Stub — the graded (severity, certainty, tier) logic is filled in later. + if tier == "enterprise" and verdict.certainty == "provable": + return "rewrite" if verdict.rewritten_sql is not None else "reject" + return "warn" + raise ValueError(f"unknown verdict class: {verdict.cls!r}") diff --git a/plugins/agami/lib/sql_guard.py b/plugins/agami/lib/sql_guard.py index 802cd0b..df32c69 100644 --- a/plugins/agami/lib/sql_guard.py +++ b/plugins/agami/lib/sql_guard.py @@ -13,14 +13,16 @@ are neutral enough to be safe across the other supported engines. `check_read_only(sql)` returns `None` when the SQL is a single safe read-only -statement, else a short human-readable reason string. Callers decide how to wrap it -(the MCP tools attach `kind="permission"`). +statement, else a safety `Verdict` the shared executor maps to a refusal +(`kind="permission"`). The rejection ladder itself lives in `_read_only_reason`. """ from __future__ import annotations import re +from guardrail import Verdict, safety_verdict + # Hard cap on SQL length. Prevents a compromised client from POSTing a multi-MB # SQL blob that takes the parser / planner / this gate down a slow path. Real # analytics SQL fits in ~10KB; 50KB is conservative. @@ -231,7 +233,7 @@ def _neutralize(sql: str) -> str: ) -def check_read_only(sql: str | None) -> str | None: +def _read_only_reason(sql: str | None) -> str | None: """Return None if `sql` is a single safe read-only statement, else a reason string. Rejection ladder (each step has its own message so the caller can correct): @@ -295,3 +297,20 @@ def check_read_only(sql: str | None) -> str | None: "process-control / sleep / remote-SQL functions are blocked" ) return None + + +def check_read_only(sql: str | None) -> Verdict | None: + """Return ``None`` if ``sql`` is a single safe read-only statement, else a safety ``Verdict``. + + Thin wrapper over :func:`_read_only_reason` (which owns the rejection ladder). A fired gate + becomes a safety-class verdict; the shared executor maps it to a refusal (``kind=permission``). + """ + reason = _read_only_reason(sql) + if reason is None: + return None + return safety_verdict( + "read_only", + reason, + "Send a single read-only SELECT / WITH...SELECT — no DML, DDL, transaction/session " + "control, or multiple statements.", + ) diff --git a/tests/conftest.py b/tests/conftest.py index 0405364..3bfc4fc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,6 +27,35 @@ def _reset_org_cache(): tools._current_org_ctx.set(None) +@pytest.fixture(autouse=True) +def _reset_injected_executor(): + """The composition-root executor (AH-012 / ACE-028) is process-global; reset it around each test so + a test that injects one — or an HTTP app that defaults to in-process — can't make a later + subprocess-path test run in-process instead.""" + try: + import tools + except Exception: + yield + return + tools.set_injected_executor(None) + yield + tools.set_injected_executor(None) + + +@pytest.fixture(autouse=True) +def _reset_audit_warned(): + """The audit-sink first-failure warn dedup set is process-global; clear it around each test so a + test that asserts the one-time warning fires isn't suppressed by an earlier test's warning.""" + try: + import tools + except Exception: + yield + return + tools._AUDIT_WARNED.clear() + yield + tools._AUDIT_WARNED.clear() + + @pytest.fixture(autouse=True) def _reset_validation_cache(): """The incremental-curation-validation cache (ACE-046) is module-global too; clear it around diff --git a/tests/e2e/test_safety_envelope.py b/tests/e2e/test_safety_envelope.py new file mode 100644 index 0000000..c9c32f9 --- /dev/null +++ b/tests/e2e/test_safety_envelope.py @@ -0,0 +1,178 @@ +"""End-to-end: execute_sql returns ONE response Envelope on BOTH surfaces (stdio + HTTP). + +A safety violation → status=refused + refusal{kind} + no data; a clean query → status=ok + data + +audit_id. The full adversarial safety corpus + the read-only-DB-role test are out of scope here; +this locks the cross-surface Envelope shape the shared contract promises — the same shape whether a +client connects over stdio or HTTP. +""" + +from __future__ import annotations + +import json +import os +import re +import subprocess +import sys +from pathlib import Path + +import pytest + +pytest.importorskip("mcp") +pytest.importorskip("starlette") + +PKG_SRC = Path(__file__).resolve().parents[2] / "packages" / "agami-core" / "src" +if str(PKG_SRC) not in sys.path: + sys.path.insert(0, str(PKG_SRC)) + +import tools # noqa: E402 + +BASE = "https://demo.example.com" + + +class _FakeOkProc: + """A successful executor run: RFC-4180 CSV on stdout (header + one row), clean stderr.""" + + returncode = 0 + stdout = "n\n5\n" + stderr = "" + + +# --- transport drivers: each returns the execute_sql tool's Envelope (parsed) ---------------- + + +def _stdio_execute_sql(sql: str) -> dict: + """Drive execute_sql over the real stdio server (a subprocess), return the tool's Envelope.""" + msgs = [ + {"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": {}}, + { + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": {"name": "execute_sql", "arguments": {"sql": sql}}, + }, + ] + stdin = "".join(json.dumps(m) + "\n" for m in msgs) + proc = subprocess.run( + [sys.executable, "-m", "mcp_harness"], + input=stdin, + capture_output=True, + text=True, + timeout=30, + env={**os.environ}, + ) + by_id = {m.get("id"): m for m in (json.loads(x) for x in proc.stdout.splitlines() if x.strip())} + return json.loads(by_id[2]["result"]["content"][0]["text"]) + + +def _http_execute_sql(sql: str) -> dict: + """Drive execute_sql over the real HTTP transport (in-process TestClient), return the Envelope.""" + import mcp_http + from starlette.testclient import TestClient + + headers = { + "Authorization": "Bearer present", + "Content-Type": "application/json", + "Accept": "application/json, text/event-stream", + } + with TestClient(mcp_http.build_app()) as c: + init = c.post( + "/mcp", + headers=headers, + json={ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2025-06-18", + "capabilities": {}, + "clientInfo": {"name": "t", "version": "1"}, + }, + }, + ) + sid = init.headers.get("mcp-session-id") + h2 = {**headers, **({"mcp-session-id": sid} if sid else {})} + c.post("/mcp", headers=h2, json={"jsonrpc": "2.0", "method": "notifications/initialized"}) + r = c.post( + "/mcp", + headers=h2, + json={ + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": {"name": "execute_sql", "arguments": {"sql": sql}}, + }, + ) + rpc = json.loads(re.search(r"\{.*\}", r.text, re.DOTALL).group(0)) + return json.loads(rpc["result"]["content"][0]["text"]) + + +@pytest.fixture +def presence_auth(monkeypatch): + """HTTP bearer-presence mode: PUBLIC_BASE_URL set, no signing secret → 'Bearer present' works.""" + monkeypatch.setenv("PUBLIC_BASE_URL", BASE) + monkeypatch.delenv("AGAMI_SIGNING_SECRET", raising=False) + + +# --- the cross-surface contract --------------------------------------------------------------- + + +def _assert_refused(env: dict) -> None: + assert env["status"] == "refused" + assert env["refusal"]["kind"] == "permission" + assert env["refusal"]["reason"] # a human reason is present + assert "data" not in env # a refusal carries no data + assert env["audit_id"] # references the recorded trail + + +def test_write_is_refused_with_one_envelope_over_stdio(): + _assert_refused(_stdio_execute_sql("DELETE FROM users")) + + +def test_write_is_refused_with_one_envelope_over_http(presence_auth): + _assert_refused(_http_execute_sql("DELETE FROM users")) + + +def test_http_refusal_writes_a_guardrail_audit_row(presence_auth, tmp_path, monkeypatch): + # The audit fires at the shared chokepoint on BOTH surfaces (unlike tool_calls, which only the HTTP + # transport writes). Pin the actual claim on the HTTP transport: a refusal over HTTP writes ONE + # guardrail_audit row the Envelope's audit_id points at — not merely that the Envelope carries an + # audit_id. A regression that bypassed _finish on this surface would drop the row and be caught. + from store import Store + + url = "sqlite://" + str(tmp_path / "audit.db") + monkeypatch.setenv("AGAMI_DB_URL", url) + s = Store.connect(url) + s.run_migrations() + s.close() + + env = _http_execute_sql("DELETE FROM users") + assert env["status"] == "refused" and env["audit_id"] + + s = Store.connect(url) + rows = s.query("SELECT * FROM guardrail_audit") + s.close() + match = [r for r in rows if r["audit_id"] == env["audit_id"]] + assert len(match) == 1 # exactly the row the Envelope points at, written over HTTP + assert match[0]["status"] == "refused" and match[0]["refusal_kind"] == "permission" + + +def test_clean_query_returns_ok_envelope_over_http(presence_auth, monkeypatch): + # A governed query needs no live DB here — fake the guarded executor so the test stays hermetic. + # The HTTP transport runs execution IN-PROCESS by default (ACE-028), so fake execute_guarded (not + # the subprocess); the point is the Envelope shape the transport hands back, not the driver. + import execute_sql + + monkeypatch.setattr( + execute_sql, + "execute_guarded", + lambda *a, **k: execute_sql.ExecResult(columns=["n"], rows=[(1,)], truncated=False), + ) + monkeypatch.setattr(tools, "_resolve_units", lambda *a: {}) + monkeypatch.setattr(tools, "_resolve_receipt", lambda *a: None) + + env = _http_execute_sql("SELECT n FROM t") + assert env["status"] == "ok" + assert env["data"]["row_count"] == 1 # the ExecuteSqlResult payload rides under `data` + assert env["data"]["columns"] == ["n"] + assert env["audit_id"] + assert "refusal" not in env diff --git a/tests/test_ace028_in_process_default.py b/tests/test_ace028_in_process_default.py index 04c3795..12fdaf8 100644 --- a/tests/test_ace028_in_process_default.py +++ b/tests/test_ace028_in_process_default.py @@ -109,7 +109,7 @@ def test_http_server_runs_in_process_by_default_no_fork(monkeypatch): monkeypatch.setattr(tools.subprocess, "run", lambda *a, **k: pytest.fail("HTTP default must not fork")) out = json.loads(tools.tool_execute_sql({"sql": "SELECT 1 AS n", "datasource": "acme"})) - assert out["columns"] == ["n"] # ran in-process, no subprocess + assert out["data"]["columns"] == ["n"] # ran in-process, no subprocess def test_stdio_cli_path_still_forks_when_no_executor_injected(monkeypatch): @@ -133,7 +133,7 @@ def _fake_run(cmd, **k): out = json.loads(tools.tool_execute_sql({"sql": "SELECT n FROM t", "datasource": "acme"})) assert "-m" in captured["cmd"] and "execute_sql" in captured["cmd"] # forked - assert out["rows"] == [["1"]] + assert out["data"]["rows"] == [["1"]] def test_in_process_default_matches_subprocess_result_envelope(monkeypatch, tmp_path): @@ -172,6 +172,6 @@ def test_in_process_default_matches_subprocess_result_envelope(monkeypatch, tmp_ tools.set_injected_executor(execute_sql.BUILTIN_EXECUTOR) # in-process, as the HTTP default does inproc = json.loads(tools.tool_execute_sql(args)) - assert "columns" in sub, sub # subprocess actually produced a result (not a creds error) + assert "columns" in sub["data"], sub # subprocess actually produced a result (not a creds error) for key in ("columns", "rows", "row_count", "truncated"): - assert sub[key] == inproc[key] # identical successful envelope whichever ran it + assert sub["data"][key] == inproc["data"][key] # identical envelope whichever ran it diff --git a/tests/test_ace044_bounded_fetch.py b/tests/test_ace044_bounded_fetch.py index 2c1d8e7..038c6e0 100644 --- a/tests/test_ace044_bounded_fetch.py +++ b/tests/test_ace044_bounded_fetch.py @@ -292,4 +292,5 @@ def fake_run(cmd, **kw): resp = json.loads(tools.tool_execute_sql({"sql": "SELECT n FROM t", "datasource": "acme", "max_rows": 2})) cmd = captured["cmd"] assert "--max-rows" in cmd and cmd[cmd.index("--max-rows") + 1] == "2" # capped at the source - assert resp["truncated"] is True # executor's flag surfaced into the response + assert resp["data"]["truncated"] is True # executor's flag surfaced into the Envelope's data + assert resp["applied"] == [{"row_cap": resp["data"]["row_count"]}] # bound noted in `applied` diff --git a/tests/test_ace051_fail_closed.py b/tests/test_ace051_fail_closed.py index 8d799df..6de552a 100644 --- a/tests/test_ace051_fail_closed.py +++ b/tests/test_ace051_fail_closed.py @@ -5,7 +5,6 @@ from __future__ import annotations -import json import sys from pathlib import Path @@ -77,9 +76,9 @@ def test_hosted_fail_closed_refuses_when_no_model(tmp_path, monkeypatch, capsys) monkeypatch.setenv("AGAMI_DB_URL", url) monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path / "no_artifacts")) - _, code = execute_sql._model_safety("SELECT id FROM orders", "acme", None) - assert code == 1 # refused, not run - assert json.loads(capsys.readouterr().err.strip())["error"]["kind"] == "model_unavailable" + _, refusal = execute_sql._model_safety("SELECT id FROM orders", "acme", None) + assert refusal is not None and refusal.kind == "model_unavailable" # refused (returned, not run) + assert capsys.readouterr().err == "" # _model_safety returns the refusal, no stderr side-effect def test_local_missing_model_is_noop(tmp_path, monkeypatch): @@ -99,12 +98,12 @@ def test_db_sourced_model_enforces_guards(tmp_path, monkeypatch, capsys): monkeypatch.setenv("AGAMI_DB_URL", url) monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path / "no_disk")) - _, code = execute_sql._model_safety("SELECT id FROM sqlite_master", "acme", None) - assert code == 1 # undeclared table refused by the table-scope guard, sourced from the DB model - assert json.loads(capsys.readouterr().err.strip())["error"]["kind"] == "table_out_of_scope" + _, refusal = execute_sql._model_safety("SELECT id FROM sqlite_master", "acme", None) + assert refusal is not None and refusal.kind == "table_out_of_scope" # undeclared table, DB model + assert capsys.readouterr().err == "" - sql, code = execute_sql._model_safety("SELECT id FROM orders", "acme", None) - assert code is None # a declared table with a named projection passes + sql, refusal = execute_sql._model_safety("SELECT id FROM orders", "acme", None) + assert refusal is None # a declared table with a named projection passes def test_disk_db_verdict_parity(tmp_path, monkeypatch, capsys): @@ -147,9 +146,9 @@ def test_hosted_falls_back_to_disk_when_db_has_no_model(tmp_path, monkeypatch, c monkeypatch.setenv("AGAMI_DB_URL", url) monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path / "art")) - _, code = execute_sql._model_safety("SELECT id FROM sqlite_master", "acme", None) - assert code == 1 # refused by the disk-sourced model, NOT model_unavailable - assert json.loads(capsys.readouterr().err.strip())["error"]["kind"] == "table_out_of_scope" + _, refusal = execute_sql._model_safety("SELECT id FROM sqlite_master", "acme", None) + assert refusal is not None and refusal.kind == "table_out_of_scope" # disk model, NOT model_unavailable + assert capsys.readouterr().err == "" def test_hosted_db_load_error_falls_back_to_disk(tmp_path, monkeypatch): @@ -169,11 +168,12 @@ def test_refusal_stderr_is_a_single_clean_json_object(tmp_path, monkeypatch, cap monkeypatch.setenv("AGAMI_DB_URL", "postgres://user:pw@127.0.0.1:1/nope") # unreachable → raises monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(tmp_path / "no_disk")) # no disk model either - _, code = execute_sql._model_safety("SELECT id FROM orders", "acme", None) - assert code == 1 - err = capsys.readouterr().err.strip() - assert json.loads(err)["error"]["kind"] == "model_unavailable" # parses whole → single object - assert "127.0.0.1" not in err and "pw" not in err # no connection details leaked + _, refusal = execute_sql._model_safety("SELECT id FROM orders", "acme", None) + assert refusal is not None and refusal.kind == "model_unavailable" + # The DB load error must not leak connection details into the refusal reason, and _model_safety + # writes NOTHING to stderr — the JSON is emitted exactly once, by main()/execute_guarded. + assert "127.0.0.1" not in refusal.reason and "pw" not in refusal.reason + assert capsys.readouterr().err == "" def test_hosted_fail_closed_when_model_package_unimportable(tmp_path, monkeypatch, capsys): @@ -192,6 +192,6 @@ def boom(name, _globals=None, _locals=None, fromlist=(), level=0): monkeypatch.setattr(builtins, "__import__", boom) monkeypatch.setenv("AGAMI_DB_URL", "sqlite://" + str(tmp_path / "x.db")) - _, code = execute_sql._model_safety("SELECT id FROM orders", "acme", None) - assert code == 1 # fail closed — no DB load is even attempted (we never resolve a model) - assert json.loads(capsys.readouterr().err.strip())["error"]["kind"] == "model_unavailable" + _, refusal = execute_sql._model_safety("SELECT id FROM orders", "acme", None) + assert refusal is not None and refusal.kind == "model_unavailable" # fail closed, no DB load + assert capsys.readouterr().err == "" diff --git a/tests/test_ah012_executor_seam.py b/tests/test_ah012_executor_seam.py index a3aba81..197f18b 100644 --- a/tests/test_ah012_executor_seam.py +++ b/tests/test_ah012_executor_seam.py @@ -26,14 +26,21 @@ sys.path.insert(0, str(PKG_SRC)) import execute_sql # noqa: E402 +from guardrail import Refusal # noqa: E402 @pytest.fixture(autouse=True) -def _reset_override(): - # _max_rows_override is a request-scoped ContextVar (ACE-028); isolate every test from it. +def _reset_seam_state(): + # Isolate every test from the process-global seam state: the _max_rows_override ContextVar + # (ACE-028) and the injected executor (leaving one set would make a later subprocess-path test + # run in-process instead). + import tools + execute_sql._max_rows_override.set(None) + tools.set_injected_executor(None) yield execute_sql._max_rows_override.set(None) + tools.set_injected_executor(None) class _SpyExecutor: @@ -59,7 +66,7 @@ def test_readonly_guard_refuses_before_the_executor_is_reached(): with pytest.raises(execute_sql.GuardRefused) as ei: execute_sql.execute_guarded("DELETE FROM t", "acme", None, executor=spy) assert ei.value.code == 1 - assert ei.value.envelope["error"]["kind"] == "permission" + assert ei.value.refusal.kind == "permission" # GuardRefused carries the typed Refusal assert spy.calls == [] # executor never reached @@ -73,13 +80,15 @@ def test_readonly_guard_still_fires_under_no_safety(): def test_model_safety_refusal_short_circuits_before_the_executor(monkeypatch): - # A model-safety refusal already wrote its JSON to stderr, so the envelope is None and only the - # exit code is carried; the executor must not run. - monkeypatch.setattr(execute_sql, "_model_safety", lambda s, p, a: (s, 1)) + # A model-safety refusal short-circuits with its typed Refusal carried on the exception (so both + # the subprocess and in-process paths build the same envelope); the executor must not run. + monkeypatch.setattr( + execute_sql, "_model_safety", lambda s, p, a: (s, Refusal("preflight_refused", "fan-trap")) + ) spy = _SpyExecutor() with pytest.raises(execute_sql.GuardRefused) as ei: execute_sql.execute_guarded("SELECT 1", "acme", None, executor=spy) - assert ei.value.code == 1 and ei.value.envelope is None + assert ei.value.code == 1 and ei.value.refusal.kind == "preflight_refused" assert spy.calls == [] @@ -218,7 +227,7 @@ def test_injected_executor_runs_in_process_with_vetted_sql_and_no_fork(monkeypat out = json.loads(tools.tool_execute_sql({"sql": "SELECT n FROM t", "datasource": "acme"})) assert fake.calls[0][0] == "SELECT n FROM t /*vetted*/" # executor saw POST-guard SQL only - assert out["columns"] == ["n"] and out["rows"] == [["1"], ["2"]] and out["row_count"] == 2 + assert out["data"]["columns"] == ["n"] and out["data"]["rows"] == [["1"], ["2"]] and out["data"]["row_count"] == 2 def test_injected_executor_is_unreachable_for_a_write(monkeypatch): @@ -231,7 +240,7 @@ def test_injected_executor_is_unreachable_for_a_write(monkeypatch): tools.set_injected_executor(fake) out = json.loads(tools.tool_execute_sql({"sql": "DELETE FROM t"})) - assert out["error"]["kind"] == "permission" # refused by the read-only guard + assert out["refusal"]["kind"] == "permission" # refused by the read-only guard assert fake.calls == [] # the injected executor was never reached — un-bypassable @@ -249,8 +258,8 @@ def execute(self, vetted_sql, creds, *, profile): tools.set_injected_executor(_Boom()) out = json.loads(tools.tool_execute_sql({"sql": "SELECT 1", "datasource": "acme"})) - assert out["error"]["kind"] == tools._classify_exit(4) - assert "connect failed" in out["error"]["remediation"] + assert out["refusal"]["kind"] == tools._classify_exit(4) + assert "connect failed" in out["refusal"]["reason"] # ExecutorError msg rides the refusal reason def test_set_injected_executor_rejects_a_bad_shape(): @@ -283,7 +292,7 @@ def _bad(profile): out = json.loads(tools.tool_execute_sql({"sql": "SELECT 1", "datasource": "acme"})) - assert "DATASOURCE_URL" in out["error"]["remediation"] # detailed, not the generic net string + assert "DATASOURCE_URL" in out["refusal"]["reason"] # detailed, not the generic net string def test_injected_executor_systemexit_is_caught_not_fatal(monkeypatch): @@ -301,7 +310,7 @@ def _exit(*a, **k): out = json.loads(tools.tool_execute_sql({"sql": "SELECT 1", "datasource": "acme"})) - assert "error" in out # a tool error envelope, not a process exit + assert out["status"] == "refused" # a refused envelope, not a process exit def test_default_no_injected_executor_forks_the_subprocess(monkeypatch): @@ -325,25 +334,73 @@ def _fake_run(cmd, **kw): out = json.loads(tools.tool_execute_sql({"sql": "SELECT n FROM t", "datasource": "acme"})) assert "-m" in captured["cmd"] and "execute_sql" in captured["cmd"] # forked the CLI executor - assert out["rows"] == [["1"]] + assert out["data"]["rows"] == [["1"]] -def test_injected_executor_model_safety_refusal_returns_clean_error(monkeypatch): +def test_injected_executor_model_safety_refusal_returns_the_typed_refusal(monkeypatch): + # In-process now surfaces the SAME typed model-safety Refusal the subprocess path does (the + # detail rides GuardRefused, no longer lost to stderr) — full structured-refusal parity. import tools monkeypatch.setattr(tools, "resolve_profile", lambda ds: "acme") monkeypatch.setattr(execute_sql, "_load_credentials", lambda p: {"type": "sqlite", "path": ":memory:"}) - monkeypatch.setattr(execute_sql, "_model_safety", lambda s, p, a: (s, 1)) # refuse + monkeypatch.setattr( + execute_sql, + "_model_safety", + lambda s, p, a: (s, Refusal("table_out_of_scope", "table foo is not in the model")), + ) fake = _SpyExecutor() tools.set_injected_executor(fake) out = json.loads(tools.tool_execute_sql({"sql": "SELECT 1", "datasource": "acme"})) - assert out["error"]["kind"] == "permission" - assert "semantic-model safety pass" in out["error"]["remediation"] + assert out["status"] == "refused" and out["refusal"]["kind"] == "table_out_of_scope" + assert "not in the model" in out["refusal"]["reason"] # the real detail, not a generic string assert fake.calls == [] # refused before the executor +def test_injected_executor_real_pii_gate_produces_sensitive_columns_envelope(monkeypatch): + # A REAL sensitive-projection gate firing on a REAL model becomes a refused Envelope with kind + # "sensitive_columns" end-to-end (real gate -> _model_safety -> GuardRefused -> refused Envelope), + # closing the chain the synthetic-stderr test only stitched at the parse step. PII is top-severity, + # so pin the whole gate->refusal->envelope path, not just the stderr parser. The executor never runs. + pytest.importorskip("pydantic") + pytest.importorskip("sqlglot") + import tools + from semantic_model import models as m + + org = m.Organization( + organization="o", + version=1, + subject_areas=[ + m.SubjectArea( + name="area", + description="d", + tables_defined=[ + m.Table( + name="users", schema="public", storage_connection="c", grain=["id"], + columns=[ + m.Column(name="id", type="integer"), + m.Column(name="email", type="string", sensitive=True), + ], + ) + ], + ) + ], + ) + monkeypatch.setattr(tools, "resolve_profile", lambda ds: "acme") + monkeypatch.setattr(execute_sql, "_resolve_guard_model", lambda profile: org) # real _model_safety runs + fake = _SpyExecutor() + tools.set_injected_executor(fake) + + out = json.loads(tools.tool_execute_sql({"sql": "SELECT email FROM users", "datasource": "acme"})) + + assert out["status"] == "refused" + assert out["refusal"]["kind"] == "sensitive_columns" # the REAL PII gate, not a synthetic line + assert "data" not in out # a refusal carries no data (no raw sensitive values leak) + assert fake.calls == [] # refused BEFORE the executor ran + + def test_injected_executor_textualizes_null_as_empty_at_the_tool_edge(monkeypatch): # The deferred-decision contract: at the MCP JSON edge the in-process path renders SQL NULL as # "" (matching the CSV wire), NOT "None". Pins the one coercion a future native-typed switch flips. @@ -357,7 +414,7 @@ def test_injected_executor_textualizes_null_as_empty_at_the_tool_edge(monkeypatc out = json.loads(tools.tool_execute_sql({"sql": "SELECT n, s FROM t", "datasource": "acme"})) - assert out["rows"] == [["1", ""]] # int -> "1", NULL -> "" (never "None") + assert out["data"]["rows"] == [["1", ""]] # int -> "1", NULL -> "" (never "None") def test_injected_executor_backstop_trims_to_max_rows(monkeypatch): @@ -371,7 +428,7 @@ def test_injected_executor_backstop_trims_to_max_rows(monkeypatch): out = json.loads(tools.tool_execute_sql({"sql": "SELECT n FROM t", "datasource": "acme", "max_rows": 2})) - assert out["rows"] == [["1"], ["2"]] and out["truncated"] is True + assert out["data"]["rows"] == [["1"], ["2"]] and out["data"]["truncated"] is True # --- main() (the subprocess CLI entry) translates the envelope's outcomes byte-identically -------- @@ -388,7 +445,7 @@ def test_main_read_only_refusal_writes_json_and_returns_1(tmp_path, monkeypatch, rc = execute_sql.main() assert rc == 1 - assert json.loads(capsys.readouterr().err.strip())["error"]["kind"] == "permission" + assert json.loads(capsys.readouterr().err.strip())["refusal"]["kind"] == "permission" def test_main_executor_error_writes_message_and_returns_code(tmp_path, monkeypatch, capsys): diff --git a/tests/test_column_scope_adversarial.py b/tests/test_column_scope_adversarial.py index 1be278a..acac504 100644 --- a/tests/test_column_scope_adversarial.py +++ b/tests/test_column_scope_adversarial.py @@ -62,7 +62,7 @@ def _t(name, cols): @pytest.mark.parametrize("sql", STAR_EVASIONS) def test_star_evasion_refused(sql): - assert rt.check_no_select_star(sql).action == "refuse" + assert rt.check_no_select_star(sql) is not None # COUNT(*) / agg(*) must NOT be over-blocked by the star ban. @@ -72,7 +72,7 @@ def test_star_evasion_refused(sql): "SELECT status, COUNT(*) AS n FROM orders GROUP BY status", ]) def test_aggregate_star_allowed(sql): - assert rt.check_no_select_star(sql).action == "allow" + assert rt.check_no_select_star(sql) is None # =========================================================================== @@ -91,7 +91,7 @@ def test_aggregate_star_allowed(sql): @pytest.mark.parametrize("sql", CLAUSE_SMUGGLES) def test_undeclared_column_in_any_clause_refused(sql): - assert rt.check_column_scope(sql, _scope_org()).action == "refuse" + assert rt.check_column_scope(sql, _scope_org()) is not None # An undeclared column wrapped in an expression / function / window. @@ -106,22 +106,25 @@ def test_undeclared_column_in_any_clause_refused(sql): @pytest.mark.parametrize("sql", EXPR_WRAPS) def test_undeclared_column_in_expression_refused(sql): - assert rt.check_column_scope(sql, _scope_org()).action == "refuse" + assert rt.check_column_scope(sql, _scope_org()) is not None def test_alias_masquerade_refused(): # The OUTPUT alias `id` is declared, but the underlying `bogus` is not — we # validate the underlying column, not the alias it is renamed to. res = rt.check_column_scope("SELECT bogus AS id FROM orders", _scope_org()) - assert res.action == "refuse" - assert res.columns == ["bogus"] + assert res is not None + assert res.rule == "column_scope" # gate identity (restores the precision the old .columns field pinned) + assert "bogus" in res.detail # the underlying column is named… + assert "id" not in res.detail.split() # …and the declared alias `id` is NOT flagged def test_undeclared_column_in_union_arm_refused(): res = rt.check_column_scope( "SELECT id FROM orders UNION SELECT bogus FROM customers", _scope_org()) - assert res.action == "refuse" - assert res.columns == ["bogus"] + assert res is not None + assert res.rule == "column_scope" # gate identity, not merely "some refusal" + assert "bogus" in res.detail def test_correlated_subquery_qualified_smuggle_refused(): @@ -130,8 +133,8 @@ def test_correlated_subquery_qualified_smuggle_refused(): res = rt.check_column_scope( "SELECT o.id FROM orders o " "WHERE EXISTS (SELECT 1 FROM customers c WHERE c.id = o.bogus)", _scope_org()) - assert res.action == "refuse" - assert res.columns == ["orders.bogus"] + assert res is not None + assert "orders.bogus" in res.detail def test_undeclared_column_alongside_where_subquery_refused(): @@ -139,15 +142,15 @@ def test_undeclared_column_alongside_where_subquery_refused(): # undeclared column in the outer query is still caught. res = rt.check_column_scope( "SELECT bogus FROM orders WHERE id IN (SELECT id FROM customers)", _scope_org()) - assert res.action == "refuse" - assert res.columns == ["bogus"] + assert res is not None + assert "bogus" in res.detail def test_quoted_identifier_undeclared_refused(): # Documents the case-insensitive-match behavior: a quoted undeclared name is # still refused. res = rt.check_column_scope('SELECT "BOGUS" FROM orders', _scope_org()) - assert res.action == "refuse" + assert res is not None # =========================================================================== @@ -161,7 +164,7 @@ def test_alias_reused_across_scopes_resolves_locally(): res = rt.check_column_scope( "SELECT o.amount FROM orders o " "WHERE EXISTS (SELECT 1 FROM customers o WHERE o.id = 1)", _scope_org()) - assert res.action == "allow" + assert res is None def test_nested_output_alias_does_not_mask_outer_column(): @@ -170,8 +173,9 @@ def test_nested_output_alias_does_not_mask_outer_column(): res = rt.check_column_scope( "SELECT bogus FROM orders WHERE id IN (SELECT id AS bogus FROM customers)", _scope_org()) - assert res.action == "refuse" - assert res.columns == ["bogus"] + assert res is not None + assert res.rule == "column_scope" # gate identity, not merely "some refusal" + assert "bogus" in res.detail # =========================================================================== @@ -183,7 +187,7 @@ def test_fail_open_derived_alias_qualified_column(): # validated at the subquery's own body; the outer reference is not re-checked. res = rt.check_column_scope( "SELECT x.whatever FROM (SELECT id AS whatever FROM orders) x", _scope_org()) - assert res.action == "allow" + assert res is None def test_fail_open_cte_shadowing_table_name(): @@ -191,7 +195,7 @@ def test_fail_open_cte_shadowing_table_name(): # so the outer reference traces to no physical table (DB is the backstop). res = rt.check_column_scope( "WITH orders AS (SELECT 1 AS bogus) SELECT bogus FROM orders", _scope_org()) - assert res.action == "allow" + assert res is None # =========================================================================== diff --git a/tests/test_column_scope_gate.py b/tests/test_column_scope_gate.py index fc45c91..6da4ab7 100644 --- a/tests/test_column_scope_gate.py +++ b/tests/test_column_scope_gate.py @@ -43,88 +43,87 @@ def _t(name, cols): # --- SELECT * ban ---------------------------------------------------------- def test_select_star_refused(): - assert rt.check_no_select_star("SELECT * FROM orders").action == "refuse" + assert rt.check_no_select_star("SELECT * FROM orders") is not None def test_qualified_star_refused(): - assert rt.check_no_select_star("SELECT o.* FROM orders o").action == "refuse" + assert rt.check_no_select_star("SELECT o.* FROM orders o") is not None def test_named_columns_allowed(): - assert rt.check_no_select_star("SELECT id, amount FROM orders").action == "allow" + assert rt.check_no_select_star("SELECT id, amount FROM orders") is None def test_count_star_allowed(): # COUNT(*) is not a projection-level star. - assert rt.check_no_select_star("SELECT COUNT(*) FROM orders").action == "allow" + assert rt.check_no_select_star("SELECT COUNT(*) FROM orders") is None def test_star_non_select_degrades_to_allow(): # Non-SELECT is the upstream read-only guard's job; this gate defers (allow). - assert rt.check_no_select_star("DELETE FROM orders").action == "allow" + assert rt.check_no_select_star("DELETE FROM orders") is None def test_star_unparseable_degrades_to_allow(): - assert rt.check_no_select_star("SELECT FROM WHERE ((").action == "allow" + assert rt.check_no_select_star("SELECT FROM WHERE ((") is None # --- column-scope: declared columns pass ----------------------------------- def test_declared_columns_allowed(): - assert rt.check_column_scope("SELECT id, amount FROM orders", _scope_org()).action == "allow" + assert rt.check_column_scope("SELECT id, amount FROM orders", _scope_org()) is None def test_qualified_declared_column_allowed(): - assert rt.check_column_scope("SELECT o.amount FROM orders o", _scope_org()).action == "allow" + assert rt.check_column_scope("SELECT o.amount FROM orders o", _scope_org()) is None def test_join_column_from_each_side_allowed(): res = rt.check_column_scope( "SELECT o.amount, c.name FROM orders o JOIN customers c ON o.customer_id = c.id", _scope_org()) - assert res.action == "allow" + assert res is None def test_join_ambiguous_but_declared_allowed(): # `id` exists on both tables — declared, so allow (don't false-reject on ambiguity). res = rt.check_column_scope( "SELECT id FROM orders o JOIN customers c ON o.customer_id = c.id", _scope_org()) - assert res.action == "allow" + assert res is None def test_declared_column_in_where_and_group_allowed(): res = rt.check_column_scope( "SELECT status, COUNT(*) FROM orders WHERE amount > 0 GROUP BY status", _scope_org()) - assert res.action == "allow" + assert res is None # --- column-scope: undeclared columns refused ------------------------------ def test_undeclared_column_refused(): res = rt.check_column_scope("SELECT bogus FROM orders", _scope_org()) - assert res.action == "refuse" - assert res.columns == ["bogus"] - assert "bogus" in res.reason + assert res is not None + assert "bogus" in res.detail def test_qualified_undeclared_column_refused(): res = rt.check_column_scope("SELECT o.bogus FROM orders o", _scope_org()) - assert res.action == "refuse" - assert res.columns == ["orders.bogus"] + assert res is not None + assert "orders.bogus" in res.detail def test_undeclared_column_in_where_refused(): res = rt.check_column_scope("SELECT id FROM orders WHERE bogus > 1", _scope_org()) - assert res.action == "refuse" - assert res.columns == ["bogus"] + assert res is not None + assert "bogus" in res.detail def test_cte_body_undeclared_column_refused(): # `bogus` binds directly to the physical `orders` inside the CTE body -> caught. res = rt.check_column_scope( "WITH t AS (SELECT bogus FROM orders) SELECT id FROM t", _scope_org()) - assert res.action == "refuse" - assert res.columns == ["bogus"] + assert res is not None + assert "bogus" in res.detail # --- column-scope: legitimate complex SQL passes --------------------------- @@ -132,35 +131,35 @@ def test_cte_body_undeclared_column_refused(): def test_cte_output_column_allowed(): res = rt.check_column_scope( "WITH t AS (SELECT id, amount FROM orders) SELECT id, amount FROM t", _scope_org()) - assert res.action == "allow" + assert res is None def test_subquery_derived_column_allowed(): res = rt.check_column_scope( "SELECT x.total FROM (SELECT SUM(amount) AS total FROM orders) x", _scope_org()) - assert res.action == "allow" + assert res is None def test_select_list_alias_reuse_allowed(): res = rt.check_column_scope( "SELECT amount AS a FROM orders ORDER BY a", _scope_org()) - assert res.action == "allow" + assert res is None def test_case_insensitive_match(): - assert rt.check_column_scope("SELECT ID, AMOUNT FROM orders", _scope_org()).action == "allow" + assert rt.check_column_scope("SELECT ID, AMOUNT FROM orders", _scope_org()) is None # --- column-scope: degrade-to-allow ---------------------------------------- def test_column_empty_model_allows(): org = m.Organization(organization="Empty", subject_areas=[m.SubjectArea(name="s")]) - assert rt.check_column_scope("SELECT anything FROM whatever", org).action == "allow" + assert rt.check_column_scope("SELECT anything FROM whatever", org) is None def test_column_non_select_degrades_to_allow(): - assert rt.check_column_scope("DELETE FROM orders", _scope_org()).action == "allow" + assert rt.check_column_scope("DELETE FROM orders", _scope_org()) is None def test_column_unparseable_degrades_to_allow(): - assert rt.check_column_scope("SELECT FROM WHERE ((", _scope_org()).action == "allow" + assert rt.check_column_scope("SELECT FROM WHERE ((", _scope_org()) is None diff --git a/tests/test_contracts.py b/tests/test_contracts.py index 29b3e37..a8426f4 100644 --- a/tests/test_contracts.py +++ b/tests/test_contracts.py @@ -15,8 +15,8 @@ from contracts import ( # noqa: E402 CrossAreaRelationship, DatasourceSchemaResult, - ErrorResult, ExecuteSqlResult, + GuardrailAuditRecord, ListDatasourcesResult, PromptExamplesResult, QueryExecutionRecord, @@ -110,13 +110,24 @@ def test_execute_sql_result_with_receipt_roundtrip(): assert _roundtrip(ExecuteSqlResult, sample) == sample -def test_execute_sql_error_roundtrip(): +# The error/refusal wire moved to the shared guardrail Envelope + Refusal (see test_guardrail.py); +# the ad-hoc ErrorResult contract was retired. + + +def test_guardrail_audit_record_roundtrip(): sample = { - "error": {"kind": "permission", "remediation": "DML/DDL rejected."}, + "audit_id": "a1b2", + "ts": "2026-07-11T00:00:00Z", + "status": "refused", + "datasource": "sales", + "refusal_kind": "permission", "sql": "DELETE FROM orders", - "execution_ms": 0, + "row_count": None, + "execution_ms": None, + "correlation_id": "turn-1", + "source": "mcp_server", } - assert _roundtrip(ErrorResult, sample) == sample + assert _roundtrip(GuardrailAuditRecord, sample) == sample def test_activity_sink_records_roundtrip(): diff --git a/tests/test_execute_sql_envelope.py b/tests/test_execute_sql_envelope.py new file mode 100644 index 0000000..a1cce80 --- /dev/null +++ b/tests/test_execute_sql_envelope.py @@ -0,0 +1,82 @@ +"""tool_execute_sql assembles the response Envelope from the executor subprocess output — the +executor→tool refusal relay. + +The `permission` refusal short-circuits BEFORE the subprocess (in the read-only pre-check), so this +is the only coverage of how a `{"refusal": ...}` stderr line (the model/scope/PII gates) — or a bare +operational failure — becomes a refused Envelope with the right `kind`. A regression in the stderr +parse would silently degrade every model-scope refusal to a generic error; these pin it. +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + +PKG_SRC = Path(__file__).resolve().parent.parent / "packages" / "agami-core" / "src" +if str(PKG_SRC) not in sys.path: + sys.path.insert(0, str(PKG_SRC)) + +import tools # noqa: E402 + + +class _Proc: + def __init__(self, returncode: int, stdout: str = "", stderr: str = "") -> None: + self.returncode = returncode + self.stdout = stdout + self.stderr = stderr + + +@pytest.fixture(autouse=True) +def _no_db(monkeypatch, tmp_path): + # The audit write in _finish is best-effort; point its jsonl fallback at a temp file and ensure + # no DB is configured, so these stay hermetic. + monkeypatch.delenv("AGAMI_DB_URL", raising=False) + monkeypatch.setattr(tools, "GUARDRAIL_AUDIT_LOG", tmp_path / "audit.jsonl") + + +def _run(monkeypatch, proc: _Proc) -> dict: + monkeypatch.setattr(tools.subprocess, "run", lambda *a, **k: proc) + return json.loads(tools.tool_execute_sql({"sql": "SELECT id FROM t"})) + + +def test_stderr_refusal_line_becomes_a_refused_envelope(monkeypatch): + stderr = json.dumps( + { + "refusal": { + "kind": "table_out_of_scope", + "reason": "table foo not in the model", + "remediation": "add it to the model", + } + } + ) + env = _run(monkeypatch, _Proc(1, stderr=stderr)) + assert env["status"] == "refused" + assert env["refusal"] == { + "kind": "table_out_of_scope", + "reason": "table foo not in the model", + "remediation": "add it to the model", + } + assert "data" not in env # a refusal carries no data + assert env["audit_id"] + + +def test_refusal_line_is_found_among_interleaved_notices(monkeypatch): + # The executor prints `[agami] …` notices to stderr too; the parser must skip them and still + # find the refusal line. + stderr = "[agami] applied default_filters: deleted_at IS NULL\n" + json.dumps( + {"refusal": {"kind": "sensitive_columns", "reason": "raw PII", "remediation": "aggregate"}} + ) + env = _run(monkeypatch, _Proc(1, stderr=stderr)) + assert env["status"] == "refused" and env["refusal"]["kind"] == "sensitive_columns" + + +def test_bare_operational_failure_is_classified_by_exit_code(monkeypatch): + # Exit 5 = SQL execution error with no {"refusal"} line → classified as 'syntax' by exit code. + env = _run(monkeypatch, _Proc(5, stderr='relation "x" does not exist')) + assert env["status"] == "refused" + assert env["refusal"]["kind"] == "syntax" # _classify_exit(5) + # Operational stderr surfaces as the reason (proper sanitization is handled separately). + assert 'relation "x"' in env["refusal"]["reason"] diff --git a/tests/test_guard_context.py b/tests/test_guard_context.py index e3b0434..90a8b80 100644 --- a/tests/test_guard_context.py +++ b/tests/test_guard_context.py @@ -100,9 +100,9 @@ def test_verdict_parity_with_and_without_ctx(sql): handed a shared ctx — the behaviour-preserving guarantee.""" org = _org() ctx = rt.build_guard_context(sql, org) - assert rt.check_table_scope(sql, org).as_dict() == rt.check_table_scope(sql, org, ctx=ctx).as_dict() - assert rt.check_no_select_star(sql).as_dict() == rt.check_no_select_star(sql, ctx=ctx).as_dict() - assert rt.check_column_scope(sql, org).as_dict() == rt.check_column_scope(sql, org, ctx=ctx).as_dict() + assert rt.check_table_scope(sql, org) == rt.check_table_scope(sql, org, ctx=ctx) + assert rt.check_no_select_star(sql) == rt.check_no_select_star(sql, ctx=ctx) + assert rt.check_column_scope(sql, org) == rt.check_column_scope(sql, org, ctx=ctx) assert rt.pre_flight_check(sql, org).as_dict() == rt.pre_flight_check(sql, org, ctx=ctx).as_dict() assert (rt.check_sensitive_projection(sql, org).as_dict() == rt.check_sensitive_projection(sql, org, ctx=ctx).as_dict()) @@ -115,5 +115,5 @@ def test_unparseable_sql_ctx_tree_is_none_and_guards_allow(): org = _org() bad = "NOT SQL AT ALL ;;;" ctx = rt.build_guard_context(bad, org) - assert rt.check_table_scope(bad, org, ctx=ctx).action == "allow" - assert rt.check_no_select_star(bad, ctx=ctx).action == "allow" + assert rt.check_table_scope(bad, org, ctx=ctx) is None + assert rt.check_no_select_star(bad, ctx=ctx) is None diff --git a/tests/test_guardrail.py b/tests/test_guardrail.py new file mode 100644 index 0000000..cda807c --- /dev/null +++ b/tests/test_guardrail.py @@ -0,0 +1,191 @@ +"""The shared guardrail contract: ``Verdict`` · ``policy`` · ``Envelope``. + +Covers the type shapes (field names + serialization, incl. the ``action`` set) and that +``policy(safety, …, tier)`` returns ``reject`` for every tier and on ``certainty=uncertain`` +(fail-closed). + +Pure-stdlib module — no importorskip needed (unlike the model-backed gate tests). +""" + +from __future__ import annotations + +import pytest +from guardrail import ( + ACTIONS, + CLASSES, + REFUSAL_KINDS, + Envelope, + Refusal, + Verdict, + policy, +) + +TIERS = ("oss", "saas", "enterprise") + + +def _safety(**over) -> Verdict: + base = dict( + cls="safety", + rule="read_only", + severity="high", + certainty="provable", + detail="write statements are not allowed", + remediation="send a single read-only SELECT", + ) + base.update(over) + return Verdict(**base) + + +# ── type shapes + serialization ────────────────────────────────────────────── + + +def test_verdict_fields_and_serialization(): + v = _safety() + # The dataclass attribute is `cls` (a Python keyword can't be a field), but the WIRE key is + # `class` — the contract name. This is the pin that keeps serialization contract-faithful. + assert v.cls == "safety" + d = v.as_dict() + assert d == { + "class": "safety", + "rule": "read_only", + "severity": "high", + "certainty": "provable", + "detail": "write statements are not allowed", + "remediation": "send a single read-only SELECT", + } + assert "rewritten_sql" not in d # omitted unless the action is rewrite + + +def test_verdict_rewritten_sql_present_only_when_set(): + v = _safety(cls="governance", certainty="provable", rewritten_sql="SELECT 1") + assert v.as_dict()["rewritten_sql"] == "SELECT 1" + + +def test_verdict_is_frozen(): + v = _safety() + with pytest.raises(Exception): + v.rule = "other" # frozen dataclass — immutable verdicts + + +def test_action_and_class_sets_match_the_contract(): + # The `action` set and the class set are fixed by the shared contract. + assert set(ACTIONS) == {"allow", "reject", "mask", "row_filter", "rewrite", "warn"} + assert set(CLASSES) == {"safety", "data_protection", "governance"} + + +def test_every_emitted_refusal_kind_is_documented(): + # `Refusal.kind` is an open str, so this pins the kinds the code ACTUALLY emits (executor gates + + # the tools layer + _classify_exit) against the documented REFUSAL_KINDS set, so the vocabulary + # can never silently drift from reality. Keep in sync with the emit sites if you add a kind. + emitted = { + # execute_sql._model_safety + main + tools.tool_execute_sql pre-check (guardrail refusals) + "permission", + "table_out_of_scope", + "select_star", + "column_out_of_scope", + "model_unavailable", + "preflight_refused", + "sensitive_columns", + # tools.tool_execute_sql + _classify_exit (operational / execution failures) + "other", + "timeout", + "dsn", + "driver_missing", + "auth", + "syntax", + } + assert emitted <= set(REFUSAL_KINDS), emitted - set(REFUSAL_KINDS) + + +def test_refusal_shape(): + r = Refusal(kind="permission", reason="write not allowed", remediation="use SELECT") + assert r.as_dict() == { + "kind": "permission", + "reason": "write not allowed", + "remediation": "use SELECT", + } + + +def test_envelope_refused_shape(): + r = Refusal(kind="table_out_of_scope", reason="unknown table foo") + env = Envelope.refused(r, audit_id="a1") + assert env.status == "refused" + d = env.as_dict() + assert d["status"] == "refused" + assert d["audit_id"] == "a1" + assert d["refusal"] == { + "kind": "table_out_of_scope", + "reason": "unknown table foo", + "remediation": "", + } + assert "data" not in d # no data on a refusal + assert d["applied"] == [] and d["warnings"] == [] + + +def test_envelope_ok_shape_with_warnings(): + warn = _safety(cls="governance", certainty="heuristic", rule="ungoverned_metric") + env = Envelope.ok( + {"columns": ["n"], "rows": [[3]]}, + audit_id="a2", + applied=[{"row_filter": "region = 'US'"}], + warnings=[warn], + ) + d = env.as_dict() + assert d["status"] == "ok" + assert d["data"] == {"columns": ["n"], "rows": [[3]]} + assert d["applied"] == [{"row_filter": "region = 'US'"}] + assert d["warnings"] == [warn.as_dict()] # warnings serialize as a list of Verdict dicts + assert "refusal" not in d + assert d["audit_id"] == "a2" + + +# ── policy: the safety branch is absolute + fail-closed ────────────────────── + + +def test_policy_safety_rejects_every_tier(): + v = _safety() + for tier in TIERS: + assert policy(v, tier) == "reject" + + +def test_policy_safety_rejects_on_uncertainty(): + # Fail-closed on doubt: an `uncertain` safety verdict is still a reject, in every tier. + v = _safety(certainty="uncertain") + for tier in TIERS: + assert policy(v, tier) == "reject" + + +def test_policy_default_tier_is_oss_and_safety_still_rejects(): + assert policy(_safety()) == "reject" # default tier + + +def test_policy_unknown_class_raises(): + bad = Verdict( + cls="bogus", rule="x", severity="low", certainty="provable", detail="", remediation="" + ) + with pytest.raises(ValueError): + policy(bad) + + +# ── policy stubs for the data-protection / governance branches (regression pins) ─ + + +def test_policy_data_protection_stub_fails_closed(): + v = _safety(cls="data_protection", rule="sensitive_projection") + # Stub branch: fail-closed until the data-protection gates fill mask/row_filter selection. + assert policy(v) == "reject" + + +def test_policy_governance_stub_warns_by_default_enforces_at_enterprise(): + heuristic = _safety(cls="governance", certainty="heuristic", rule="ungoverned_metric") + assert policy(heuristic, "oss") == "warn" + assert policy(heuristic, "saas") == "warn" # SaaS recommends — never blocks + assert policy(heuristic, "enterprise") == "warn" # heuristic never blocks + provable = _safety(cls="governance", certainty="provable", rule="fan_trap") + assert policy(provable, "oss") == "warn" # non-enforcing tier + assert policy(provable, "saas") == "warn" # SaaS recommends, does not enforce + assert policy(provable, "enterprise") == "reject" # provable + enforcing tier + rewrite = _safety( + cls="governance", certainty="provable", rule="fan_trap", rewritten_sql="SELECT 1" + ) + assert policy(rewrite, "enterprise") == "rewrite" diff --git a/tests/test_guardrail_audit.py b/tests/test_guardrail_audit.py new file mode 100644 index 0000000..81a2f31 --- /dev/null +++ b/tests/test_guardrail_audit.py @@ -0,0 +1,146 @@ +"""The guardrail audit trail — every execute_sql result (ok or refused) writes one row keyed by +the response Envelope's `audit_id`, at the shared chokepoint so BOTH surfaces are covered. + +Also checks the jsonl fallback (no datastore) and that a logging failure never breaks the tool. +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + +PKG_SRC = Path(__file__).resolve().parent.parent / "packages" / "agami-core" / "src" +if str(PKG_SRC) not in sys.path: + sys.path.insert(0, str(PKG_SRC)) + +import tools # noqa: E402 +from store import Store # noqa: E402 + + +@pytest.fixture +def db(tmp_path, monkeypatch): + url = "sqlite://" + str(tmp_path / "audit.db") + monkeypatch.setenv("AGAMI_DB_URL", url) + s = Store.connect(url) + s.run_migrations() + s.close() + return url + + +def _audit_rows(url): + s = Store.connect(url) + rows = s.query("SELECT * FROM guardrail_audit ORDER BY ts") + s.close() + return rows + + +class _FakeProc: + """A successful executor run: RFC-4180 CSV on stdout (header + one row), clean stderr.""" + + returncode = 0 + stdout = "n\n5\n" + stderr = "" + + +def test_migration_creates_the_table(db): + assert _audit_rows(db) == [] # table exists, empty + + +def test_refused_query_writes_a_refused_audit_row(db): + # A write is rejected by the read-only gate before any subprocess — the refusal still audits. + resp = json.loads( + tools.tool_execute_sql( + {"sql": "DELETE FROM t", "datasource": "sales", "correlation_id": "turn-1"} + ) + ) + assert resp["status"] == "refused" and resp["refusal"]["kind"] == "permission" + + (row,) = _audit_rows(db) + assert row["audit_id"] == resp["audit_id"] # the row the Envelope points at + assert row["status"] == "refused" + assert row["refusal_kind"] == "permission" + assert row["sql"] == "DELETE FROM t" + assert row["datasource"] == "sales" + assert row["correlation_id"] == "turn-1" + assert row["row_count"] is None + + +def test_ok_query_writes_an_ok_audit_row(db, monkeypatch): + monkeypatch.setattr(tools.subprocess, "run", lambda *a, **k: _FakeProc()) + monkeypatch.setattr(tools, "_resolve_units", lambda *a: {}) + monkeypatch.setattr(tools, "_resolve_receipt", lambda *a: None) + + resp = json.loads(tools.tool_execute_sql({"sql": "SELECT n FROM t", "datasource": "sales"})) + assert resp["status"] == "ok" and resp["data"]["row_count"] == 1 + + (row,) = _audit_rows(db) + assert row["audit_id"] == resp["audit_id"] + assert row["status"] == "ok" + assert row["refusal_kind"] is None + assert row["sql"] == "SELECT n FROM t" + assert row["row_count"] == 1 + + +def test_jsonl_fallback_when_no_datastore(tmp_path, monkeypatch): + # No AGAMI_DB_URL → the audit row lands in the local jsonl instead of the DB. + monkeypatch.delenv("AGAMI_DB_URL", raising=False) + log = tmp_path / "guardrail_audit.jsonl" + monkeypatch.setattr(tools, "GUARDRAIL_AUDIT_LOG", log) + + resp = json.loads(tools.tool_execute_sql({"sql": "DROP TABLE t", "datasource": "sales"})) + assert resp["status"] == "refused" + + rec = json.loads(log.read_text().splitlines()[0]) + assert rec["audit_id"] == resp["audit_id"] + assert rec["status"] == "refused" and rec["refusal_kind"] == "permission" + + +def test_audit_write_is_best_effort_and_never_raises(tmp_path, monkeypatch): + # A failing write (here: the jsonl append blows up) is swallowed inside the recorder, so the + # tool that calls it through _finish can never be broken by a logging failure. + monkeypatch.delenv("AGAMI_DB_URL", raising=False) + + def _boom(*_a, **_k): + raise OSError("disk full") + + monkeypatch.setattr(tools, "_append_jsonl", _boom) + tools._record_guardrail_audit({"audit_id": "x", "ts": "t", "status": "ok"}) # must not raise + + +def test_audit_sink_failure_warns_exactly_once(tmp_path, monkeypatch, capsys): + # Best-effort must not be SILENT: a PERSISTENTLY unwritable sink emits a one-time warning so a dead + # audit trail is observable to an operator — but only ONCE (not a line per query), and never with + # raw sql/driver text, and never breaking the tool. + monkeypatch.delenv("AGAMI_DB_URL", raising=False) + monkeypatch.setattr(tools, "_append_jsonl", lambda *_a, **_k: False) # log dir unwritable + + for i in range(3): + tools._record_guardrail_audit({"audit_id": str(i), "ts": "t", "status": "ok"}) # never raises + + err = capsys.readouterr().err + assert err.count("guardrail audit not recorded") == 1 # warned ONCE across 3 failures, not per-call + assert "SELECT" not in err and "status" not in err # value-free: no sql / record contents leaked + + +def test_audit_db_sink_error_warns_once(tmp_path, monkeypatch, capsys): + # The DB-sink branch (exception, not a False return) is also surfaced once — type only, no message. + url = "sqlite://" + str(tmp_path / "audit.db") + monkeypatch.setenv("AGAMI_DB_URL", url) + + def _boom_store(*_a, **_k): + raise RuntimeError("permission denied for table guardrail_audit on host db-42.internal") + + monkeypatch.setattr(tools, "_append_jsonl", lambda *_a, **_k: True) + import store as _store + + monkeypatch.setattr(_store.Store, "from_env", staticmethod(_boom_store)) + + tools._record_guardrail_audit({"audit_id": "a", "ts": "t", "status": "ok"}) + tools._record_guardrail_audit({"audit_id": "b", "ts": "t", "status": "ok"}) + + err = capsys.readouterr().err + assert err.count("guardrail audit not recorded (sink error)") == 1 + assert "db-42.internal" not in err # value-free: the driver message never rides the warning diff --git a/tests/test_mcp_harness.py b/tests/test_mcp_harness.py index 109d63c..1f332f5 100644 --- a/tests/test_mcp_harness.py +++ b/tests/test_mcp_harness.py @@ -245,7 +245,8 @@ def test_execute_sql_rejects_mutation_over_protocol(): ]) by_id = {m.get("id"): m for m in out} payload = json.loads(by_id[2]["result"]["content"][0]["text"]) - assert payload["error"]["kind"] == "permission" + assert payload["status"] == "refused" + assert payload["refusal"]["kind"] == "permission" def test_unknown_method_returns_error(): diff --git a/tests/test_plugin_lib_resolution.py b/tests/test_plugin_lib_resolution.py index 60917d0..3546306 100644 --- a/tests/test_plugin_lib_resolution.py +++ b/tests/test_plugin_lib_resolution.py @@ -25,7 +25,7 @@ SCRIPTS = REPO / "plugins" / "agami" / "scripts" LIB = REPO / "plugins" / "agami" / "lib" SRC = REPO / "packages" / "agami-core" / "src" -VENDORED = ["agami_paths.py", "execute_sql.py", "sql_guard.py", "semantic_model/__init__.py", "semantic_model/units.py"] +VENDORED = ["agami_paths.py", "execute_sql.py", "sql_guard.py", "guardrail.py", "semantic_model/__init__.py", "semantic_model/units.py"] # `-S` disables site.py, so an installed (incl. editable) agami-core is not on the path — the same # "the package isn't available" state a marketplace user's plain python3 is in. diff --git a/tests/test_ports.py b/tests/test_ports.py index c426fea..dbb3e40 100644 --- a/tests/test_ports.py +++ b/tests/test_ports.py @@ -14,7 +14,6 @@ ActivitySink, AuthProvider, GovernancePolicy, - GovernanceVerdict, Org, OrgResolver, Principal, @@ -45,15 +44,15 @@ def test_ports_module_imports_without_model_deps(): def test_default_adapters_satisfy_protocols(): from oss_adapters import ( FileActivitySink, + NoopGovernancePolicy, PresenceAuthProvider, SingleTenantOrgResolver, - WarnOnlyGovernancePolicy, ) assert isinstance(FileActivitySink(), ActivitySink) assert isinstance(SingleTenantOrgResolver(), OrgResolver) assert isinstance(PresenceAuthProvider(), AuthProvider) - assert isinstance(WarnOnlyGovernancePolicy(), GovernancePolicy) + assert isinstance(NoopGovernancePolicy(), GovernancePolicy) # --- adapter behavior ------------------------------------------------------- @@ -81,11 +80,12 @@ def test_presence_auth_accepts_nonempty_rejects_empty(): def test_warn_only_governance_never_blocks(): - from oss_adapters import WarnOnlyGovernancePolicy + from oss_adapters import NoopGovernancePolicy - v = WarnOnlyGovernancePolicy().evaluate() - assert isinstance(v, GovernanceVerdict) - assert v.allowed is True + # The OSS default emits no governance findings (an empty Verdict list), so nothing is + # warned/rewritten/blocked. Enforcement is a paid adapter that returns governance Verdicts. + verdicts = NoopGovernancePolicy().evaluate() + assert verdicts == [] def test_file_activity_sink_writes_jsonl(tmp_path): diff --git a/tests/test_sql_guard.py b/tests/test_sql_guard.py index 7edfcd5..cb5daa3 100644 --- a/tests/test_sql_guard.py +++ b/tests/test_sql_guard.py @@ -257,8 +257,8 @@ def test_rejects_row_level_locks(sql: str) -> None: ) def test_row_lock_rule_names_the_lock(sql: str) -> None: # These use SHARE (not a deny keyword) so the row-lock rule is what fires. - reason = check_read_only(sql) - assert reason is not None and "lock" in reason.lower(), reason + v = check_read_only(sql) + assert v is not None and "lock" in v.detail.lower(), v @pytest.mark.parametrize( @@ -270,9 +270,9 @@ def test_row_lock_rule_names_the_lock(sql: str) -> None: ], ) def test_rejects_select_into_write_path(sql: str) -> None: - reason = check_read_only(sql) - assert reason is not None, f"SELECT INTO not blocked: {sql!r}" - assert "INTO" in reason + v = check_read_only(sql) + assert v is not None, f"SELECT INTO not blocked: {sql!r}" + assert "INTO" in v.detail @pytest.mark.parametrize( @@ -328,9 +328,9 @@ def test_rejects_dangerous_functions(sql: str) -> None: def test_rejects_over_length_cap() -> None: payload = "SELECT 1, " + ("a, " * 30_000) + "1" assert len(payload) > _MAX_SQL_CHARS - reason = check_read_only(payload) - assert reason is not None - assert "50000" in reason or "caps" in reason + v = check_read_only(payload) + assert v is not None + assert "50000" in v.detail or "caps" in v.detail def test_length_cap_exact_boundary() -> None: @@ -634,8 +634,8 @@ def test_executor_blocks_dangerous_sql_even_with_no_safety(tmp_path, extra) -> N envelope = json.loads(line) except ValueError: continue - assert envelope is not None, f"no JSON error envelope on stderr; got: {proc.stderr!r}" - assert envelope["error"]["kind"] == "permission", envelope + assert envelope is not None, f"no JSON refusal envelope on stderr; got: {proc.stderr!r}" + assert envelope["refusal"]["kind"] == "permission", envelope def test_executor_dangerous_function_blocked(tmp_path) -> None: diff --git a/tests/test_table_scope_gate.py b/tests/test_table_scope_gate.py index 87fab91..45a30f6 100644 --- a/tests/test_table_scope_gate.py +++ b/tests/test_table_scope_gate.py @@ -37,60 +37,60 @@ def _t(name): def test_declared_table_allowed(): - assert rt.check_table_scope("SELECT * FROM orders", _scope_org()).action == "allow" + assert rt.check_table_scope("SELECT * FROM orders", _scope_org()) is None def test_undeclared_table_refused(): res = rt.check_table_scope("SELECT * FROM sqlite_master", _scope_org()) - assert res.action == "refuse" - assert res.offending_tables == ["sqlite_master"] - assert "sqlite_master" in res.reason + assert res is not None + assert "sqlite_master" in res.detail def test_join_all_declared_allowed(): res = rt.check_table_scope( "SELECT * FROM orders o JOIN customers c ON o.customer_id = c.id", _scope_org()) - assert res.action == "allow" + assert res is None def test_join_with_undeclared_refused_lists_only_bad_one(): res = rt.check_table_scope( "SELECT * FROM orders o JOIN payments p ON p.order_id = o.id", _scope_org()) - assert res.action == "refuse" - assert res.offending_tables == ["payments"] + assert res is not None + assert "payments" in res.detail + assert "orders" not in res.detail # only the undeclared table is named, not the declared one def test_cte_reference_allowed(): # `t` is a CTE name, not a physical table — must not be flagged. res = rt.check_table_scope( "WITH t AS (SELECT * FROM orders) SELECT * FROM t", _scope_org()) - assert res.action == "allow" + assert res is None def test_cte_body_referencing_undeclared_refused(): res = rt.check_table_scope( "WITH t AS (SELECT * FROM secret_table) SELECT * FROM t", _scope_org()) - assert res.action == "refuse" - assert res.offending_tables == ["secret_table"] + assert res is not None + assert "secret_table" in res.detail def test_subquery_alias_allowed(): # derived-table alias `x` is not a table; the inner `orders` is declared. res = rt.check_table_scope("SELECT * FROM (SELECT id FROM orders) x", _scope_org()) - assert res.action == "allow" + assert res is None def test_schema_qualified_declared_allowed(): - assert rt.check_table_scope("SELECT * FROM public.orders", _scope_org()).action == "allow" + assert rt.check_table_scope("SELECT * FROM public.orders", _scope_org()) is None def test_case_insensitive_match(): - assert rt.check_table_scope("SELECT * FROM ORDERS", _scope_org()).action == "allow" + assert rt.check_table_scope("SELECT * FROM ORDERS", _scope_org()) is None def test_empty_model_allows(): org = m.Organization(organization="Empty", subject_areas=[m.SubjectArea(name="s")]) - assert rt.check_table_scope("SELECT * FROM anything", org).action == "allow" + assert rt.check_table_scope("SELECT * FROM anything", org) is None def test_set_operation_arm_scoped(): @@ -98,20 +98,21 @@ def test_set_operation_arm_scoped(): # arm (regression for the set-operation bypass), not blanket-allow. res = rt.check_table_scope( "SELECT id FROM orders UNION SELECT id FROM secret_table", _scope_org()) - assert res.action == "refuse" - assert res.offending_tables == ["secret_table"] + assert res is not None + assert "secret_table" in res.detail + assert "orders" not in res.detail # the declared arm isn't flagged — only the undeclared one def test_set_operation_all_declared_allowed(): res = rt.check_table_scope( "SELECT id FROM orders UNION ALL SELECT id FROM customers", _scope_org()) - assert res.action == "allow" + assert res is None def test_non_select_degrades_to_allow(): # Non-SELECT is the upstream read-only guard's job; this gate defers (allow). - assert rt.check_table_scope("DELETE FROM orders", _scope_org()).action == "allow" + assert rt.check_table_scope("DELETE FROM orders", _scope_org()) is None def test_unparseable_degrades_to_allow(): - assert rt.check_table_scope("SELECT FROM WHERE ((", _scope_org()).action == "allow" + assert rt.check_table_scope("SELECT FROM WHERE ((", _scope_org()) is None diff --git a/tests/test_tool_extension_seam.py b/tests/test_tool_extension_seam.py index 220553b..00b8cbc 100644 --- a/tests/test_tool_extension_seam.py +++ b/tests/test_tool_extension_seam.py @@ -20,9 +20,9 @@ import tools # noqa: E402 from oss_adapters import ( # noqa: E402 FileActivitySink, + NoopGovernancePolicy, PresenceAuthProvider, SingleTenantOrgResolver, - WarnOnlyGovernancePolicy, ) from ports import Adapters, Org # noqa: E402 from starlette.testclient import TestClient # noqa: E402 @@ -114,7 +114,7 @@ def test_adapters_none_uses_the_oss_defaults(base_url): assert isinstance(a.org_resolver, SingleTenantOrgResolver) assert isinstance(a.auth_provider, PresenceAuthProvider) # presence when no signing secret assert isinstance(a.activity_sink, FileActivitySink) - assert isinstance(a.governance, WarnOnlyGovernancePolicy) + assert isinstance(a.governance, NoopGovernancePolicy) def test_create_app_uses_the_passed_adapters(base_url): @@ -124,7 +124,7 @@ def test_create_app_uses_the_passed_adapters(base_url): activity_sink=FileActivitySink(), org_resolver=resolver, auth_provider=auth, - governance=WarnOnlyGovernancePolicy(), + governance=NoopGovernancePolicy(), ) kwargs = _auth_middleware_kwargs(mcp_http.create_app(adapters=adapters)) assert kwargs["resolver"] is resolver # the passed adapters are used at the composition root