diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 02db4e41..f690d57a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,6 +100,144 @@ jobs: pytest tests/e2e/test_safety_corpus.py tests/e2e/test_role_floor_pg.py -q -k "db_path or role" + # ── ACE-082: per-engine EXECUTION evidence ─────────────────────────────────────────────────── + # Deliberately SEPARATE jobs, with no `needs:` on integration-pg and none between themselves, so + # one flaky engine container cannot turn the Postgres role-floor result (the F9 done-bar) green + # or red. Each job selects only its own engine, and sets AGAMI_IT__REQUIRED so an + # unreachable server FAILS rather than skipping — pytest exits 0 when everything skips, and a + # silently-skipped job would claim per-engine coverage while proving nothing. + # These need no secrets, so they run on fork PRs too. + + integration-mysql: + # The highest-value engine here: backticks are MySQL's NATIVE identifier quote, so this is + # where a wrongly-dialected parse and a real execution can genuinely disagree. + name: integration (MySQL — dialect execution matrix) + runs-on: ubuntu-latest + services: + mysql: + image: mysql:8 + env: + MYSQL_DATABASE: shop + MYSQL_USER: agami_test + # Ephemeral localhost CI service on a throwaway container. MySQL has no `trust` auth + # equivalent, so a literal is unavoidable; this is a non-secret sentinel, not a credential. + MYSQL_PASSWORD: ci_local_only + MYSQL_ROOT_PASSWORD: ci_local_only_root + ports: + - 3306:3306 + # `mysqladmin ping` exits 0 once the server answers, even for an unauthenticated ping. + # `-h 127.0.0.1` is load-bearing: the entrypoint's temporary init server accepts on the + # unix socket while the real server has not yet bound 3306, so a socket ping reports ready + # during that window. Over TCP the probe stays refused until the port is really listening. + options: >- + --health-cmd "mysqladmin ping -h 127.0.0.1" + --health-interval 5s + --health-timeout 5s + --health-retries 20 + env: + AGAMI_IT_MYSQL_HOST: 127.0.0.1 + AGAMI_IT_MYSQL_PORT: "3306" + AGAMI_IT_MYSQL_USER: agami_test + # Same non-secret sentinel as the service block above — not a credential. + AGAMI_IT_MYSQL_PASSWORD: ci_local_only + AGAMI_IT_MYSQL_DB: shop + AGAMI_IT_MYSQL_REQUIRED: "1" + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 + with: + python-version: "3.12" + - name: pytest (MySQL safety corpus) + run: >- + uvx --python 3.12 + --with pytest --with pymysql + --with-editable "packages/agami-core[model,server]" + pytest tests/e2e/test_safety_corpus.py -q -k "engine and MySQL" + + integration-sqlserver: + # The only bracket-quoting engine, and the worst measured divergence: under a generic parse + # `SELECT TOP n [col] FROM [tbl]` resolves to no tables with `TOP` mistaken for a column. + name: integration (SQL Server — dialect execution matrix) + runs-on: ubuntu-latest + services: + sqlserver: + image: mcr.microsoft.com/mssql/server:2022-latest + env: + ACCEPT_EULA: "Y" + MSSQL_PID: Developer # free edition, no licence + # Non-secret sentinel for an ephemeral localhost service. Must still satisfy SQL Server's + # complexity rule (upper + lower + digit + non-alphanumeric) or the server refuses to start. + MSSQL_SA_PASSWORD: Ci_local_only_1 + ports: + - 1433:1433 + env: + AGAMI_IT_SQLSERVER_HOST: 127.0.0.1 + AGAMI_IT_SQLSERVER_PORT: "1433" + AGAMI_IT_SQLSERVER_USER: sa + # Same non-secret sentinel as the service block above — not a credential. + AGAMI_IT_SQLSERVER_PASSWORD: Ci_local_only_1 + AGAMI_IT_SQLSERVER_DB: shop + AGAMI_IT_SQLSERVER_REQUIRED: "1" + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 + with: + python-version: "3.12" + # SQL Server accepts TCP connections before it can serve queries, so a port check is not a + # readiness signal — poll an actual query. Explicit here rather than a `--health-cmd`, whose + # nested shell quoting cannot be verified without running it on a x86 runner. + - name: wait for SQL Server + run: | + uvx --python 3.12 --with pymssql python - <<'PY' + import os, sys, time, pymssql + + deadline, last = time.time() + 180, None + while time.time() < deadline: + try: + conn = pymssql.connect( + server=os.environ["AGAMI_IT_SQLSERVER_HOST"], + port=int(os.environ["AGAMI_IT_SQLSERVER_PORT"]), + user=os.environ["AGAMI_IT_SQLSERVER_USER"], + password=os.environ["AGAMI_IT_SQLSERVER_PASSWORD"], + login_timeout=5, + ) + conn.cursor().execute("SELECT 1") + conn.close() + print("SQL Server ready") + sys.exit(0) + except Exception as exc: + last = exc + time.sleep(5) + sys.exit(f"SQL Server never became ready: {last}") + PY + - name: pytest (SQL Server safety corpus) + run: >- + uvx --python 3.12 + --with pytest --with pymssql + --with-editable "packages/agami-core[model,server]" + pytest tests/e2e/test_safety_corpus.py -q -k "engine and SQLServer" + + integration-duckdb: + # In-process, no service. Cheap, and it closes a coverage claim the corpus previously made + # without having: DuckDB appeared only in a plugin-script test and an always-skipped one. + name: integration (DuckDB — dialect execution matrix) + runs-on: ubuntu-latest + env: + # This job is DuckDB's only evidence. Its sole availability risk is the driver below, so + # REQUIRED makes a missing one fail rather than skip the whole corpus green. + AGAMI_IT_DUCKDB_REQUIRED: "1" + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 + with: + python-version: "3.12" + - name: pytest (DuckDB safety corpus) + run: >- + uvx --python 3.12 + --with pytest --with duckdb + --with-editable "packages/agami-core[model,server]" + pytest tests/e2e/test_safety_corpus.py -q -k "engine and DuckDB" + gitleaks: name: gitleaks (secret scan) runs-on: ubuntu-latest diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index 2843ce82..15c4f9b7 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -3,9 +3,12 @@ from __future__ import annotations +import importlib import os import sys +from collections.abc import Callable from pathlib import Path +from typing import NoReturn import pytest @@ -18,6 +21,8 @@ import harness # noqa: E402 (tests/e2e is on sys.path during collection) +from safety.corpus import SCHEMA # noqa: E402 + BASE = "https://demo.example.com" @@ -138,17 +143,12 @@ def db_safety_env(pg_admin, tmp_path, monkeypatch): psycopg2, conn, sc = pg_admin cur = conn.cursor() - # 1) demo datasource tables in public (owner-seeded), from the single-sourced SCHEMA. - from safety.corpus import SCHEMA - - for name, spec in SCHEMA.items(): - cur.execute(f"DROP TABLE IF EXISTS {name} CASCADE") - pg_types = {"INTEGER": "integer", "REAL": "double precision", "TEXT": "text"} - ddl = ", ".join(f"{c} {pg_types[t]}" for c, t in spec["columns"]) - cur.execute(f"CREATE TABLE {name} ({ddl})") - placeholders = ", ".join("%s" for _ in spec["columns"]) - for row in spec["rows"]: - cur.execute(f"INSERT INTO {name} VALUES ({placeholders})", row) + # 1) demo datasource tables in public (owner-seeded), from the single-sourced SCHEMA — through + # the shared per-engine seeder (ACE-082), so Postgres and every other engine build the + # physical fixture the same way and a DDL-spelling difference lives in ONE table. + harness.seed_schema( + lambda sql, params=None: cur.execute(sql, params), "PostgreSQL", drop_first=True + ) # 2) the SELECT-only role the app connects to the datasource as. create_ro_role(cur, sc["dbname"]) @@ -181,6 +181,219 @@ def db_safety_env(pg_admin, tmp_path, monkeypatch): _reset_ro_role(cur) +# ── ACE-082: the per-engine EXECUTION fixtures (real engines, containerized) ─────────────────── +# ACE-079 proved the guard's VERDICT on all eleven engines, but with a spy executor and no +# database — the vetted statement was re-parsed by sqlglot, never accepted by an engine. These +# fixtures close that half: the same corpus, through the same tool, against a real server. +# +# Each engine follows `pg_admin`'s contract: absent connection settings SKIP, unless +# AGAMI_IT__REQUIRED is set — in the CI job that owns an engine's evidence, an +# unavailable engine FAILS, because pytest exits 0 when everything skips and a silently-skipped +# job would claim per-engine coverage while proving nothing. "Unavailable" deliberately includes a +# MISSING DRIVER, not just an unreachable server: these drivers are supplied only by `--with` flags +# on the CI `uvx` line, so dropping one in a future edit is the likeliest way to empty a job, and +# `pytest.importorskip` would turn exactly that into a green run. +# +# Like the Postgres fixtures above, these DROP/CREATE shared table names in a shared database, so +# they are SERIAL-ONLY — do not run this dir under pytest-xdist (`-n`) without per-worker table +# names, or workers would race. The current invocation is serial. + + +def _engine_creds(engine: str, default_port: str) -> dict[str, str]: + """AGAMI_IT__* connection settings. Defaults target the local containers; CI overrides.""" + e = engine.upper() + return { + "host": os.environ.get(f"AGAMI_IT_{e}_HOST", "127.0.0.1"), + "port": os.environ.get(f"AGAMI_IT_{e}_PORT", default_port), + "user": os.environ.get(f"AGAMI_IT_{e}_USER", ""), + "password": os.environ.get(f"AGAMI_IT_{e}_PASSWORD", ""), + "database": os.environ.get(f"AGAMI_IT_{e}_DB", "shop"), + } + + +def _unavailable(engine: str) -> Callable[[str], NoReturn]: + """`pytest.fail` in the job that owns this engine's evidence, else `pytest.skip`.""" + return pytest.fail if os.environ.get(f"AGAMI_IT_{engine.upper()}_REQUIRED") else pytest.skip + + +def _require_driver(engine: str, module: str): + """Import an engine's driver, honouring the REQUIRED contract. + + Deliberately not `pytest.importorskip`: that skips unconditionally, which in the job that owns + this engine's evidence would report success for a run that executed nothing. + """ + try: + return importlib.import_module(module) + except ImportError as exc: + _unavailable(engine)(f"{module} not installed, so {engine} proves nothing ({exc})") + + +def _write_credentials(art: Path, profile: str, fields: dict[str, str], monkeypatch) -> None: + """Write /local/credentials for engines with no DSN scheme (SQLServer, DuckDB). + + Both surfaces must read the SAME file: the stdio subprocess re-resolves the path from + AGAMI_ARTIFACTS_DIR in `execute_sql.main()`, but the in-process HTTP surface captured + CREDENTIALS_PATH at import — so the module constant is repointed here too. That is a + test-ordering artifact, not a product gap: a deployed server has its credentials file + before it boots. + """ + import execute_sql + + local = art / "local" + local.mkdir(parents=True, exist_ok=True) + path = local / "credentials" + body = f"[{profile}]\n" + "".join(f"{k} = {v}\n" for k, v in fields.items()) + path.write_text(body) + path.chmod(0o600) # the executor refuses a more permissive credentials file + monkeypatch.setattr(execute_sql, "CREDENTIALS_PATH", path) + + +def _engine_model_env(art: Path, engine: str, monkeypatch) -> None: + """File-served model declaring the engine's storage type — what the guard parses every statement + in, and what the executor is then reconciled against. The storage type is read from the engine's + `EngineFixture` rather than restated, so the model and the physical fixture cannot disagree.""" + (art / "acme").mkdir(parents=True, exist_ok=True) + harness.write_disk_model(art / "acme", storage_type=harness.ENGINE_FIXTURES[engine].storage_type) + monkeypatch.delenv("AGAMI_DB_URL", raising=False) + monkeypatch.delenv("APP_DATABASE_URL", raising=False) + monkeypatch.setenv("AGAMI_ARTIFACTS_DIR", str(art)) + monkeypatch.setenv("AGAMI_PROFILE", "acme") + monkeypatch.delenv("AGAMI_SQL_UNSCOPABLE_POSTURE", raising=False) + + +def _drop_corpus_tables(cur) -> None: + """Best-effort teardown. Tolerates a table that was never created — the next seed re-runs + DROP ... IF EXISTS first, so a failure here cannot leave stale rows behind.""" + for name in SCHEMA: + try: + cur.execute(f"DROP TABLE IF EXISTS {name}") + except Exception: + pass + + +@pytest.fixture +def mysql_safety_env(tmp_path, monkeypatch): + """MySQL 8 — the engine that matters most here: backticks are MySQL's NATIVE identifier quote, + so this is where a wrongly-dialected parse and a real execution can genuinely disagree.""" + unavailable = _unavailable("MySQL") + pymysql = _require_driver("MySQL", "pymysql") + c = _engine_creds("MySQL", "53307") + if not c["password"]: + unavailable("set AGAMI_IT_MYSQL_PASSWORD to run the MySQL safety corpus") + try: + conn = pymysql.connect( + host=c["host"], + port=int(c["port"]), + user=c["user"], + password=c["password"], + database=c["database"], + autocommit=True, + connect_timeout=10, + ) + except Exception as exc: + unavailable(f"no reachable MySQL ({exc})") + # Everything after the connect lives in the try, so a failure while seeding closes the + # connection instead of leaking one per test. + try: + cur = conn.cursor() + harness.seed_schema( + lambda sql, params=None: cur.execute(sql, params), "MySQL", drop_first=True + ) + art = tmp_path / "art" + _engine_model_env(art, "MySQL", monkeypatch) + monkeypatch.setenv( + "DATASOURCE_URL__ACME", + f"mysql://{c['user']}:{c['password']}@{c['host']}:{c['port']}/{c['database']}", + ) + yield + finally: + _drop_corpus_tables(conn.cursor()) + conn.close() + + +@pytest.fixture +def sqlserver_safety_env(tmp_path, monkeypatch): + """SQL Server 2022 — the only bracket-quoting engine, and the worst measured divergence: + under a generic parse `SELECT TOP n [col] FROM [tbl]` resolves to no tables and `TOP` as a + column, so every scope gate finds nothing to object to.""" + unavailable = _unavailable("SQLServer") + pymssql = _require_driver("SQLServer", "pymssql") + c = _engine_creds("SQLServer", "11433") + if not c["password"]: + unavailable("set AGAMI_IT_SQLSERVER_PASSWORD to run the SQL Server safety corpus") + connect = dict( + server=c["host"], + port=int(c["port"]), + user=c["user"], + password=c["password"], + autocommit=True, + login_timeout=15, + ) + try: + boot = pymssql.connect(**connect) + except Exception as exc: + unavailable(f"no reachable SQL Server ({exc})") + try: # the fixture database may not exist yet; create it before connecting into it + boot.cursor().execute( + f"IF DB_ID('{c['database']}') IS NULL CREATE DATABASE [{c['database']}]" + ) + finally: + boot.close() + + conn = pymssql.connect(database=c["database"], **connect) + try: + cur = conn.cursor() + harness.seed_schema( + lambda sql, params=None: cur.execute(sql, params), "SQLServer", drop_first=True + ) + art = tmp_path / "art" + _engine_model_env(art, "SQLServer", monkeypatch) + # SQL Server has no DSN scheme (`_env_datasource_dsn` handles postgres/mysql/sqlite/…), so + # the per-field credentials file is the supported channel. Clear the env DSN or it would win. + monkeypatch.delenv("DATASOURCE_URL__ACME", raising=False) + _write_credentials( + art, + "acme", + { + "type": harness.ENGINE_FIXTURES["SQLServer"].credential_type, + "host": c["host"], + "port": c["port"], + "user": c["user"], + "password": c["password"], + "database": c["database"], + }, + monkeypatch, + ) + yield + finally: + _drop_corpus_tables(conn.cursor()) + conn.close() + + +@pytest.fixture +def duckdb_safety_env(tmp_path, monkeypatch): + """DuckDB in-process — no server, no container. Included because it is cheap and because the + corpus previously claimed DuckDB coverage it did not have. + + Its only availability risk is the driver, so it goes through the same REQUIRED contract: the + `integration-duckdb` job is this engine's ONLY evidence, and a skipped run there would be a + green job that proved nothing.""" + _require_driver("DuckDB", "duckdb") + db = tmp_path / "shop.duckdb" + harness.seed_duckdb(db) + art = tmp_path / "art" + _engine_model_env(art, "DuckDB", monkeypatch) + # DuckDB has no DSN scheme either — same per-field credentials channel as SQL Server. + monkeypatch.delenv("DATASOURCE_URL__ACME", raising=False) + _write_credentials( + art, + "acme", + {"type": harness.ENGINE_FIXTURES["DuckDB"].credential_type, "path": str(db)}, + monkeypatch, + ) + yield + + @pytest.fixture def pg_ro_conn(pg_admin): """A RAW connection AS the read-only role, plus one seeded table — for the role-floor test. This diff --git a/tests/e2e/harness.py b/tests/e2e/harness.py index 8c62440b..e334eeb2 100644 --- a/tests/e2e/harness.py +++ b/tests/e2e/harness.py @@ -14,6 +14,8 @@ import sqlite3 import subprocess import sys +from collections.abc import Callable +from dataclasses import dataclass, field from pathlib import Path REPO_ROOT = Path(__file__).resolve().parents[2] @@ -28,6 +30,68 @@ _TYPE_MAP = {"INTEGER": "integer", "REAL": "float", "TEXT": "string"} +# ── the physical-fixture seam (ACE-082) ──────────────────────────────────────────────────────── +@dataclass(frozen=True) +class EngineFixture: + """How the single-sourced `SCHEMA` is materialized physically on ONE engine. + + The corpus never forks: the cases, the schema and the expected verdicts are shared. The only + thing that legitimately varies per engine is the physical fixture — how each column type is + spelled in DDL, which parameter placeholder the driver takes, and whether DROP needs a + CASCADE. Naming exactly those three here keeps every engine on one seeding code path, so + adding an engine is a row in `ENGINE_FIXTURES` rather than a second seeder that can drift. + """ + + storage_type: str # what the semantic model declares (m.StorageConnection.storage_type) + credential_type: str # what the executor dispatches on (`type` in the DSN / credentials file) + ddl_types: dict[str, str] = field(default_factory=dict) + placeholder: str = "?" + drop_suffix: str = "" # Postgres needs CASCADE; the others reject the keyword + + +ENGINE_FIXTURES: dict[str, EngineFixture] = { + "SQLite": EngineFixture( + "SQLite", "sqlite", {"INTEGER": "INTEGER", "REAL": "REAL", "TEXT": "TEXT"}, "?" + ), + "PostgreSQL": EngineFixture( + "PostgreSQL", + "postgres", + {"INTEGER": "integer", "REAL": "double precision", "TEXT": "text"}, + "%s", + " CASCADE", + ), + "MySQL": EngineFixture( + "MySQL", "mysql", {"INTEGER": "int", "REAL": "double", "TEXT": "varchar(255)"}, "%s" + ), + "SQLServer": EngineFixture( + "SQLServer", "sqlserver", {"INTEGER": "int", "REAL": "float", "TEXT": "nvarchar(255)"}, "%s" + ), + "DuckDB": EngineFixture( + "DuckDB", "duckdb", {"INTEGER": "INTEGER", "REAL": "DOUBLE", "TEXT": "VARCHAR"}, "?" + ), +} + + +def seed_schema( + execute: Callable[..., object], engine: str, *, drop_first: bool = False +) -> None: + """Materialize every `SCHEMA` table on `engine`, through the caller's `execute(sql, params=None)`. + + The adapter is passed in rather than a connection, so this function imports no driver and is + the single place that turns `SCHEMA` into physical tables — on every engine, including the two + that were previously seeded by hand (SQLite here, Postgres inline in `conftest.db_safety_env`). + """ + fx = ENGINE_FIXTURES[engine] + for name, spec in SCHEMA.items(): + if drop_first: + execute(f"DROP TABLE IF EXISTS {name}{fx.drop_suffix}") + ddl = ", ".join(f"{c} {fx.ddl_types[t]}" for c, t in spec["columns"]) + execute(f"CREATE TABLE {name} ({ddl})") + placeholders = ", ".join(fx.placeholder for _ in spec["columns"]) + for row in spec["rows"]: + execute(f"INSERT INTO {name} VALUES ({placeholders})", row) + + # ── model + datasource builders (single-sourced from SCHEMA) ─────────────────────────────────── def build_org(storage_type: str = "SQLite"): """The semantic model the guards scope against — built from SCHEMA (names + sensitive flags).""" @@ -113,16 +177,25 @@ def seed_sqlite(path: Path) -> None: """Create + seed the physical SQLite datasource governed queries execute against.""" con = sqlite3.connect(str(path)) try: - for name, spec in SCHEMA.items(): - ddl = ", ".join(f"{c} {t}" for c, t in spec["columns"]) - con.execute(f"CREATE TABLE {name} ({ddl})") - placeholders = ", ".join("?" for _ in spec["columns"]) - con.executemany(f"INSERT INTO {name} VALUES ({placeholders})", spec["rows"]) + seed_schema(lambda sql, params=None: con.execute(sql, params or ()), "SQLite") con.commit() finally: con.close() +def seed_duckdb(path: Path) -> None: + """Create + seed the physical DuckDB datasource. Seeded through a WRITE connection here because + the executor deliberately opens DuckDB `read_only=True` — the fixture must not need the executor + to be writable to exist.""" + import duckdb # type: ignore + + con = duckdb.connect(str(path)) + try: + seed_schema(lambda sql, params=None: con.execute(sql, params or []), "DuckDB") + finally: + con.close() + + def seed_db_model(url: str, ds: str = "acme", storage_type: str = "SQLite") -> None: """Write the DB-served model into an app DB at `url` (the hosted path's model source).""" import model_store diff --git a/tests/e2e/test_safety_corpus.py b/tests/e2e/test_safety_corpus.py index 2c3e107e..f0291aa0 100644 --- a/tests/e2e/test_safety_corpus.py +++ b/tests/e2e/test_safety_corpus.py @@ -6,6 +6,46 @@ scopability, or recon fails here on whichever surface it regressed; availability is asserted via the row-cap arm (the statement-timeout arm is proven end-to-end in `tests/test_resource_limits.py`). The read-only-role floor lives in `test_role_floor_pg.py` (Postgres, env-gated). + +ACE-082 — per-engine EXECUTION coverage +======================================= +ACE-079 made the guard read every statement in the datasource's own dialect, and proved the VERDICT +on all eleven supported engines (`tests/test_guard_dialect_parsing.py`). That proof uses a spy +executor and no database: the vetted statement is re-parsed by sqlglot, never accepted by an engine. +This module adds the other half — the statement the guard vetted is a statement a REAL engine +accepts — and the table below is the single place that says how far that goes per engine. + +| Engine | Identifier quote | Coverage here | What is NOT proven | +|------------|------------------|-----------------------------------|-------------------------------| +| SQLite | `"` `` ` `` `[]` | real, in-process (file path) | anything outside these cases | +| PostgreSQL | `"` | real server (`integration-pg`) | anything outside these cases | +| MySQL | `` ` `` | real server (`mysql:8`) | anything outside these cases | +| SQLServer | `[]` | real server (`mssql/server:2022`) | anything outside these cases | +| DuckDB | `"` | real, in-process | anything outside these cases | +| Redshift | `"` | verdict only (ACE-079) | execution parity | +| Snowflake | `"` | verdict only (ACE-079) | execution parity | +| BigQuery | `` ` `` | verdict only (ACE-079) | execution parity | +| Databricks | `` ` `` | verdict only (ACE-079) | execution parity | +| Oracle | `"` | verdict only (ACE-079) | execution parity | +| Trino | `"` | verdict only (ACE-079) | execution parity | + +An executing engine is proven for the corpus's own cases — that is a regression harness, not a +claim about the engine's whole grammar. + +The five executing engines cover all three identifier-quoting families — backtick (MySQL), bracket +(SQLServer) and double-quote (PostgreSQL/SQLite/DuckDB) — which is the axis ACE-079's defect class +turns on. The six verdict-only engines each share a quoting family with one that executes. + +Two engines named in ACE-082 are deliberately absent, because reaching them would require changing +runtime source, which that spec forbids: + - **BigQuery emulator** — `execute_sql._run_bigquery` builds `bigquery.Client(project=…)` with no + `client_options` / `api_endpoint`, so it cannot be pointed at an emulator without a source + change. (The emulator is also SQLite-backed, so it would prove parse/accept, not BigQuery + semantics.) + - **Databricks via local Spark** — `execute_sql._run_databricks` connects through the Databricks + SQL connector to a workspace; a local `SparkSession` is not reachable through it. Testing it + would mean asserting against a stub, which is what ACE-079 already does. +Both remain verdict-only above, and are recorded as residual risk in the spec rather than implied. """ from __future__ import annotations @@ -34,6 +74,12 @@ def assert_outcome(env: dict, expect: str, sql: str) -> None: assert env["status"] == "ok", (sql, env) assert "refusal" not in env assert env["audit_id"] + # A result set actually came back. Without this, "ok" only proves the guard ALLOWED the + # statement — a fixture that seeded the wrong database, or an engine that accepted the + # statement and matched nothing, would still pass, and the per-engine cases whose whole + # point is that the vetted statement EXECUTES would prove only half of it. Every governed + # case in the corpus selects from seeded rows, so an empty result is a broken fixture. + assert env["data"]["rows"], (sql, env) elif expect == "bounded": # Availability: a runaway result is bounded — EITHER a resource_limit refusal, OR an ok # Envelope flagged truncated with a row_cap in `applied` (capped + flagged, never silent). @@ -64,6 +110,49 @@ def test_safety_corpus_file_path(case, surface, file_safety_env): assert_outcome(env, case.expect, case.sql) +# ── ACE-082: the same corpus, against real engines beyond Postgres/SQLite ───────────────────── +# One env fixture per executing engine. Each SKIPS without connection settings and FAILS instead +# when AGAMI_IT__REQUIRED is set, so the job that owns an engine's evidence cannot pass +# green while proving nothing. Pairing engine×case here (rather than skipping inside the test) +# keeps the ids exact, so a CI job selects its engine with `-k "engine and MySQL"`. +ENGINE_ENV_FIXTURES = { + "MySQL": "mysql_safety_env", + "SQLServer": "sqlserver_safety_env", + "DuckDB": "duckdb_safety_env", +} +ENGINE_CASES = [(e, c) for e in ENGINE_ENV_FIXTURES for c in CASES if c.runs_on(e)] + + +@pytest.mark.parametrize( + "engine,case", ENGINE_CASES, ids=[f"{e}:{c.id}" for e, c in ENGINE_CASES] +) +def test_safety_corpus_engine(engine, case, surface, request): + # The verdict must be identical to the SQLite/Postgres paths AND the vetted statement must be + # one this engine actually accepts — an "ok" case here executes for real, so a statement the + # guard approved but the engine rejects fails as an executor error rather than passing. + request.getfixturevalue(ENGINE_ENV_FIXTURES[engine]) + env = surface(case.sql, datasource="acme", max_rows=case.max_rows) + assert_outcome(env, case.expect, case.sql) + + +def test_engine_cases_stay_out_of_the_postgres_gate(): + """The `integration-pg` job selects with `-k "db_path or role"` and carries the F9 done-bar — + the read-only role floor and file/db parity. Its result must stay independent of the engine + jobs, which is true today only because no engine node id contains `db_path` or `role`. + + That is an accident waiting to be broken by a case note, so it is asserted rather than assumed: + a new case called e.g. "role-column" would silently join the done-bar job and let a container + hiccup there turn the F9 gate red. `-k` matches case-insensitively, so this does too. + """ + for engine, case in ENGINE_CASES: + node_id = f"{engine}:{case.id}" + for selector in ("db_path", "role"): + assert selector not in node_id.lower(), ( + f"engine case {node_id!r} would be selected by integration-pg's " + f'-k "db_path or role", coupling the F9 done-bar to an engine container' + ) + + @pytest.mark.parametrize("case", DB_PATH_CASES, ids=[c.id for c in DB_PATH_CASES]) def test_safety_corpus_db_path(case, surface, db_safety_env): # DB-served model (Postgres app DB) + Postgres datasource read as the read-only role. IDENTICAL diff --git a/tests/safety/corpus.py b/tests/safety/corpus.py index b0331d10..1c558103 100644 --- a/tests/safety/corpus.py +++ b/tests/safety/corpus.py @@ -157,17 +157,45 @@ def id(self) -> str: # (The demo datasource is SQLite, which accepts both quoting styles.) # SQLite accepts backticks and brackets as identifier quoting; Postgres does not, where the # same text is not valid SQL at all — so these are pinned to the engine they mean something on. + # MySQL is where these matter most: the backtick is MySQL's NATIVE identifier quote, so this is + # the engine where a wrongly-dialected parse and a real execution can genuinely disagree. SQLite + # accepts backticks too, so both run the same expectation. Case("quoting", "SELECT `id` FROM `undeclared`", "table_out_of_scope", - "backtick-undeclared-table", engines=("SQLite",)), + "backtick-undeclared-table", engines=("SQLite", "MySQL")), Case("quoting", "SELECT `nosuchcol` FROM `orders`", "column_out_of_scope", - "backtick-undeclared-col", engines=("SQLite",)), + "backtick-undeclared-col", engines=("SQLite", "MySQL")), Case("quoting", "SELECT `email` FROM `customers`", "ok", - "backtick-sensitive-is-seen", engines=("SQLite",)), - Case("quoting", "SELECT * FROM `orders`", "select_star", "backtick-star", engines=("SQLite",)), + "backtick-sensitive-is-seen", engines=("SQLite", "MySQL")), + Case("quoting", "SELECT * FROM `orders`", "select_star", "backtick-star", + engines=("SQLite", "MySQL")), + # The other half of the guarantee: a backtick-quoted GOVERNED query is not merely allowed, it + # executes on the engine whose native quote that is. A vetted statement the engine then rejects + # would fail here as an executor error rather than passing. + Case("quoting", "SELECT `id`, `amount` FROM `orders`", "ok", + "backtick-governed-executes", engines=("SQLite", "MySQL")), + # SQL Server is the only bracket-quoting engine, and `TOP n` is where a generic parse fails + # worst: it resolves to no tables at all with `TOP` mistaken for a column, so every scope gate + # finds nothing to object to and allows the statement. Case("quoting", "SELECT [id] FROM [undeclared]", "table_out_of_scope", - "bracket-undeclared-table", engines=("SQLite",)), - # Valid identifier quoting on both engines, so it runs everywhere. - Case("quoting", 'SELECT "id" FROM "undeclared"', "table_out_of_scope", "quoted-undeclared-table"), + "bracket-undeclared-table", engines=("SQLite", "SQLServer")), + Case("quoting", "SELECT TOP 5 [id] FROM [undeclared]", "table_out_of_scope", + "bracket-top-undeclared-table", engines=("SQLServer",)), + Case("quoting", "SELECT TOP 5 [email] FROM [customers]", "ok", + "bracket-top-sensitive-is-seen", engines=("SQLServer",)), + Case("quoting", "SELECT TOP 5 [id], [amount] FROM [orders]", "ok", + "bracket-top-governed-executes", engines=("SQLServer",)), + # `"` is an identifier quote on SQLite/Postgres/SQL Server/DuckDB — but on MySQL it is a STRING + # literal, so the same text is a different statement there and gets its own expectation below. + Case("quoting", 'SELECT "id" FROM "undeclared"', "table_out_of_scope", "quoted-undeclared-table", + engines=("SQLite", "PostgreSQL", "SQLServer", "DuckDB")), + # On MySQL a double-quoted statement cannot be scoped at all (the "identifiers" are strings), so + # the guard must fail CLOSED rather than mis-read it as a query over declared objects. This is + # the positive proof that reading in the engine's own grammar changes the verdict, not just the + # parse — the generic reading of this statement is a scopable query over `orders`. + Case("quoting", 'SELECT "id" FROM "undeclared"', "unscopable_sql", + "double-quote-is-a-string-on-mysql", engines=("MySQL",)), + Case("quoting", 'SELECT "id", "amount" FROM "orders"', "unscopable_sql", + "double-quote-governed-unscopable-on-mysql", engines=("MySQL",)), # The read-only lexer is a separate code path from the dialect-aware parse; a write stays # refused whatever the quoting, on every engine. Case("quoting", "DELETE FROM `orders`", "permission", "backtick-delete"), @@ -175,7 +203,10 @@ def id(self) -> str: # change which tightens parsing has to prove it did not start refusing valid SQL. Without # them the "no false refusal" side of the corpus rests on four statements, and a tightening # that broke ordinary queries could still pass. - Case("governed", 'SELECT "id", "amount" FROM "orders"', "ok", "quoted-identifiers"), + # Pinned to the engines where `"` quotes an identifier; MySQL's counterpart is the + # `unscopable_sql` case above, where the same text is a string literal. + Case("governed", 'SELECT "id", "amount" FROM "orders"', "ok", "quoted-identifiers", + engines=("SQLite", "PostgreSQL", "SQLServer", "DuckDB")), Case("governed", "SELECT o.id, o.amount FROM orders o", "ok", "aliased-table"), Case("governed", "SELECT id FROM orders WHERE status = 'paid'", "ok", "where-literal"), Case("governed", "SELECT id, amount FROM orders ORDER BY amount DESC", "ok", "order-by"), diff --git a/tests/test_no_false_refusal_on_valid_sql.py b/tests/test_no_false_refusal_on_valid_sql.py index 4751644a..a328bf29 100644 --- a/tests/test_no_false_refusal_on_valid_sql.py +++ b/tests/test_no_false_refusal_on_valid_sql.py @@ -33,16 +33,26 @@ sys.path.insert(0, str(REPO_ROOT / "tests")) from semantic_model import runtime as rt # noqa: E402 +from semantic_model.sql_dialect import sqlglot_dialect # noqa: E402 from safety import corpus # noqa: E402 -# The corpus executes against SQLite; the seeded sample model declares SQLite too. Parsing -# each statement under the engine it is written for is the whole point — a statement is only -# valid with respect to some engine. -CORPUS_DIALECT = "sqlite" +# Parsing each statement under the engine it is written for is the whole point — a statement is +# only valid with respect to some engine. The corpus now carries statements written for engines +# other than SQLite (`SELECT TOP n [col]` is valid T-SQL and is not SQLite at all), so a case +# pinned to engines is checked under EACH of them, and only an unpinned case — one claimed to be +# valid everywhere — falls back to the default. Checking those under one fixed dialect would +# either fail on valid SQL or, worse, quietly stop covering the engines the corpus added. +DEFAULT_CORPUS_ENGINE = "SQLite" +CORPUS_DIALECT = sqlglot_dialect(DEFAULT_CORPUS_ENGINE) # the seeded sample model declares SQLite VALID_CORPUS_SQL = [c for c in corpus.CASES if c.expect == "ok"] +# (case, engine) pairs — the unit actually being measured, since one case can be valid on several. +VALID_CORPUS_PAIRS = [ + (c, e) for c in VALID_CORPUS_SQL for e in (c.engines or (DEFAULT_CORPUS_ENGINE,)) +] + SAMPLE_EXAMPLES = sorted( (REPO_ROOT / "plugins" / "agami" / "samples").rglob("prompt_examples/*/examples.yaml") ) @@ -78,10 +88,12 @@ def test_the_seeded_examples_were_found(): assert SEEDED_SQL, "no seeded example SQL found to check" -@pytest.mark.parametrize("case", VALID_CORPUS_SQL, ids=lambda c: c.id) -def test_valid_corpus_sql_still_parses(case): - tree, why = rt._parse_reporting(case.sql, CORPUS_DIALECT) - assert tree is not None, f"valid corpus SQL now refuses ({why}): {case.sql}" +@pytest.mark.parametrize( + "case,engine", VALID_CORPUS_PAIRS, ids=[f"{c.id}@{e}" for c, e in VALID_CORPUS_PAIRS] +) +def test_valid_corpus_sql_still_parses(case, engine): + tree, why = rt._parse_reporting(case.sql, sqlglot_dialect(engine)) + assert tree is not None, f"valid corpus SQL now refuses on {engine} ({why}): {case.sql}" @pytest.mark.parametrize( @@ -94,6 +106,8 @@ def test_seeded_example_sql_still_parses(path, sql): def test_the_measured_refusal_delta_is_zero(): """The number reported in the pull request, computed rather than asserted by hand.""" - checked = [c.sql for c in VALID_CORPUS_SQL] + [s for _, s in SEEDED_SQL] - refused = [s for s in checked if rt._parse_reporting(s, CORPUS_DIALECT)[0] is None] + checked = [(c.sql, sqlglot_dialect(e)) for c, e in VALID_CORPUS_PAIRS] + [ + (s, CORPUS_DIALECT) for _, s in SEEDED_SQL + ] + refused = [s for s, dialect in checked if rt._parse_reporting(s, dialect)[0] is None] assert not refused, f"{len(refused)}/{len(checked)} valid statements refused: {refused}" diff --git a/tests/test_resolve_guard_model.py b/tests/test_resolve_guard_model.py index 483d3d43..40251e57 100644 --- a/tests/test_resolve_guard_model.py +++ b/tests/test_resolve_guard_model.py @@ -31,6 +31,10 @@ def _write_disk_model(root: Path) -> None: "e2e_harness", REPO_ROOT / "tests" / "e2e" / "harness.py" ) h = importlib.util.module_from_spec(spec) + # Register BEFORE exec_module — the documented importlib idiom. A module executed while absent + # from sys.modules cannot resolve its own module globals, which anything reading them at class + # -creation time needs (`@dataclass` is one), so loading it detached fails at import. + sys.modules[spec.name] = h spec.loader.exec_module(h) h.write_disk_model(root)