From 8e2c3181a0984970d349c1bf92889b66a31016b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 08:39:46 +0000 Subject: [PATCH 1/2] fix(store): add a schema migration runner (#44) SCHEMA_VERSION moved from 1 to 2 when run_counter was added (#12), but _init_schema only wrote the version row when it was absent -- a database created under version 1 kept reporting version 1 forever, regardless of what its tables actually looked like. _init_schema now applies an ordered, idempotent list of migrations for any version between the database's recorded version and SCHEMA_VERSION, and writes the corrected version afterward. A recorded version newer than this build supports raises SchemaVersionError rather than being silently opened. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01H2jDXUwrbM4eupm7CyLivS --- CHANGELOG.md | 10 +++ docs/roadmap.md | 5 +- src/agentsec/store/sqlite.py | 64 +++++++++++--- tests/test_store_migrations.py | 150 +++++++++++++++++++++++++++++++++ 4 files changed, 214 insertions(+), 15 deletions(-) create mode 100644 tests/test_store_migrations.py diff --git a/CHANGELOG.md b/CHANGELOG.md index aa064b2..0261f3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,16 @@ drafts even when they appear in a release. reading arbitrary repository files, so "the producer promises no source text" stopped being a guarantee that could live in one place. +### Fixed + +- **Store schema migration runner** ([#44](https://github.com/trionnemesis/AgentSec/issues/44)). + `SCHEMA_VERSION` moved from 1 to 2 when `run_counter` was added, but the + stored version row was only ever written when absent — a database created + under 1 reported version 1 forever, regardless of its actual tables. Opening + a store now applies pending migrations in order and corrects the stored + version; a version newer than this build supports raises `SchemaVersionError` + rather than being silently opened. + ## [0.2.0] — 2026-08-06 The release that gives AgentSec a first step. In 0.1.0 the entry point was diff --git a/docs/roadmap.md b/docs/roadmap.md index 4d54792..46ded9c 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -31,6 +31,7 @@ agentsec init → agentsec scan → agentsec scan --verify -t → dashboard | Evidence collectors: Wazuh, OTel, tool audit, state diff | ✅ | file backends tested; timeline rebasing for fixtures | | Run provenance (`recorded` / `live` / `mixed`) | ✅ | a fixture-derived `secure` is labelled as such ([#27](https://github.com/trionnemesis/AgentSec/issues/27)) | | SQLite store (runs, findings, audit log) | ✅ | latest-run-per-scenario aggregates | +| Store schema migration runner | ✅ | ordered, idempotent, fails closed on an unrecognised future version ([#44](https://github.com/trionnemesis/AgentSec/issues/44)) | | CLI with meaningful exit codes | ✅ | `0` clean, `1` blocking, `2` could not tell | | Selected-project manifest and discovery | ✅ | `.agentsec/project.yaml`; relative locations only, traversal and symlink escape refused | | Runtime framework fingerprint engine | ✅ | deterministic, read-only detection for LangGraph/LangChain, OpenAI Agents SDK, AutoGen, Semantic Kernel, CrewAI and framework-neutral tool calling; development-agent config stays separate | @@ -58,10 +59,6 @@ agentsec init → agentsec scan → agentsec scan --verify -t → dashboard `environments: [ci, staging]` and `scan --verify` needs a real target. - [ ] Wazuh rule pack for the four original bundled scenarios (`100501`, `100610`, `100720`, `100810`) -- [ ] **Migration runner — overdue.** `SCHEMA_VERSION` is `2`, and - `store/sqlite.py:_init_schema` writes the version row only when absent, so - a database created under version 1 reports version 1 forever and nothing - reads that row to decide anything. --- diff --git a/src/agentsec/store/sqlite.py b/src/agentsec/store/sqlite.py index f4526da..e942a2c 100644 --- a/src/agentsec/store/sqlite.py +++ b/src/agentsec/store/sqlite.py @@ -19,24 +19,18 @@ from pathlib import Path from typing import Any -from agentsec.errors import FindingNotFound, RunNotFound +from agentsec.errors import AgentSecError, FindingNotFound, RunNotFound from agentsec.models.finding import Finding, FindingStatus from agentsec.models.run import Run SCHEMA_VERSION = 2 +# The version-1 shape: every table that existed before `run_counter`. Applied +# unconditionally and idempotently on every open, including against a legacy +# database, so it must never do more than `CREATE ... IF NOT EXISTS`. _SCHEMA = """ CREATE TABLE IF NOT EXISTS schema_version (version INTEGER NOT NULL); --- One row per day, incremented atomically. Deriving the next run id from --- MAX(run_id) in Python let two processes on one workspace mint the same id, --- and `save_run` upserts, so the second run silently overwrote the first -- --- losing a run without trace, in the component whose job is to be the record. -CREATE TABLE IF NOT EXISTS run_counter ( - day TEXT PRIMARY KEY, - next_n INTEGER NOT NULL -); - CREATE TABLE IF NOT EXISTS runs ( run_id TEXT PRIMARY KEY, scenario_id TEXT NOT NULL, @@ -85,6 +79,38 @@ CREATE INDEX IF NOT EXISTS idx_audit_at ON audit_log (at DESC); """ +# Ordered upgrades applied on top of _SCHEMA, oldest first. Each entry's `sql` +# takes a database already at `version - 1` to `version`; it must be safe to +# re-run (`IF NOT EXISTS` / idempotent DDL only), because a fresh database +# gets every migration applied in sequence rather than starting pre-upgraded. +_MIGRATIONS: list[tuple[int, str]] = [ + ( + 2, + """ + -- One row per day, incremented atomically. Deriving the next run id + -- from MAX(run_id) in Python let two processes on one workspace mint + -- the same id, and `save_run` upserts, so the second run silently + -- overwrote the first -- losing a run without trace, in the + -- component whose job is to be the record. + CREATE TABLE IF NOT EXISTS run_counter ( + day TEXT PRIMARY KEY, + next_n INTEGER NOT NULL + ); + """, + ), +] + + +class SchemaVersionError(AgentSecError): + """The database's recorded schema version is newer than this build knows. + + Raised rather than silently proceeding: a store that guessed at an + unrecognised shape could corrupt data or misreport what it read, and + either is worse than refusing to open. + """ + + code = "schema_version_unsupported" + class ResultStore: def __init__(self, path: Path) -> None: @@ -107,8 +133,24 @@ def _init_schema(self) -> None: with self._conn() as conn: conn.executescript(_SCHEMA) row = conn.execute("SELECT version FROM schema_version").fetchone() + current = row["version"] if row is not None else 0 + + if current > SCHEMA_VERSION: + raise SchemaVersionError( + f"{self.path} reports schema version {current}, newer than the " + f"{SCHEMA_VERSION} this build supports; refusing to open it", + details={"found_version": current, "supported_version": SCHEMA_VERSION}, + ) + + for target_version, migration_sql in _MIGRATIONS: + if current < target_version: + conn.executescript(migration_sql) + current = target_version + if row is None: - conn.execute("INSERT INTO schema_version (version) VALUES (?)", (SCHEMA_VERSION,)) + conn.execute("INSERT INTO schema_version (version) VALUES (?)", (current,)) + elif current != row["version"]: + conn.execute("UPDATE schema_version SET version = ?", (current,)) # -- runs --------------------------------------------------------------- diff --git a/tests/test_store_migrations.py b/tests/test_store_migrations.py new file mode 100644 index 0000000..ee0ed82 --- /dev/null +++ b/tests/test_store_migrations.py @@ -0,0 +1,150 @@ +"""Schema migration runner for the SQLite result store. + +`SCHEMA_VERSION` moved from 1 to 2 when `run_counter` was added (#12), but the +version row was only ever written when absent -- so a database created under +1 kept reporting 1 forever, regardless of what its actual tables looked like. +These tests pin the fix (#44): a stale version is upgraded and the row +corrected, migrations are idempotent, and a future version this build does +not recognise is refused rather than silently accepted. +""" + +from __future__ import annotations + +import sqlite3 +from pathlib import Path + +import pytest + +from agentsec.store.sqlite import SCHEMA_VERSION, ResultStore, SchemaVersionError + +#: The tables the version-1 schema shipped with, before `run_counter` existed. +_V1_SCHEMA = """ +CREATE TABLE schema_version (version INTEGER NOT NULL); +INSERT INTO schema_version (version) VALUES (1); + +CREATE TABLE runs ( + run_id TEXT PRIMARY KEY, + scenario_id TEXT NOT NULL, + target_id TEXT NOT NULL, + profile TEXT NOT NULL, + status TEXT NOT NULL, + purple_verdict TEXT, + prevention TEXT, + detection TEXT, + evidence TEXT, + response TEXT, + created_at TEXT NOT NULL, + finished_at TEXT, + scenario_digest TEXT, + payload TEXT NOT NULL +); + +CREATE TABLE findings ( + finding_id TEXT PRIMARY KEY, + scenario_id TEXT NOT NULL, + target_id TEXT NOT NULL, + status TEXT NOT NULL, + severity TEXT NOT NULL, + verdict TEXT NOT NULL, + first_seen_run TEXT NOT NULL, + last_seen_run TEXT NOT NULL, + updated_at TEXT NOT NULL, + payload TEXT NOT NULL +); + +CREATE TABLE audit_log ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + at TEXT NOT NULL, + actor TEXT NOT NULL, + action TEXT NOT NULL, + subject TEXT, + outcome TEXT NOT NULL, + detail TEXT +); +""" + + +def _make_v1_database(path: Path) -> None: + """Build a database in exactly the pre-#12 v1 shape: no `run_counter`.""" + conn = sqlite3.connect(path) + try: + conn.executescript(_V1_SCHEMA) + conn.commit() + finally: + conn.close() + + +def _stored_version(path: Path) -> int: + conn = sqlite3.connect(path) + try: + row = conn.execute("SELECT version FROM schema_version").fetchone() + assert row is not None + return int(row[0]) + finally: + conn.close() + + +def _has_table(path: Path, name: str) -> bool: + conn = sqlite3.connect(path) + try: + row = conn.execute( + "SELECT 1 FROM sqlite_master WHERE type='table' AND name=?", (name,) + ).fetchone() + return row is not None + finally: + conn.close() + + +def test_a_fresh_database_lands_directly_on_the_current_version(tmp_path: Path) -> None: + db_path = tmp_path / "fresh.db" + + ResultStore(db_path) + + assert _stored_version(db_path) == SCHEMA_VERSION + assert _has_table(db_path, "run_counter") + + +def test_a_v1_database_is_upgraded_to_v2_on_open(tmp_path: Path) -> None: + db_path = tmp_path / "legacy.db" + _make_v1_database(db_path) + assert not _has_table(db_path, "run_counter") + assert _stored_version(db_path) == 1 + + ResultStore(db_path) + + assert _stored_version(db_path) == SCHEMA_VERSION + assert _has_table(db_path, "run_counter") + + +def test_reopening_an_already_migrated_database_is_a_no_op(tmp_path: Path) -> None: + db_path = tmp_path / "legacy.db" + _make_v1_database(db_path) + + ResultStore(db_path) + assert _stored_version(db_path) == SCHEMA_VERSION + + # Second open must not fail, duplicate the migration, or move the version + # again -- CREATE TABLE without IF NOT EXISTS would raise here if the + # migration re-ran instead of recognising it already applied. + ResultStore(db_path) + assert _stored_version(db_path) == SCHEMA_VERSION + + +def test_a_future_schema_version_is_refused_rather_than_silently_opened( + tmp_path: Path, +) -> None: + db_path = tmp_path / "from_the_future.db" + _make_v1_database(db_path) + conn = sqlite3.connect(db_path) + try: + conn.execute("UPDATE schema_version SET version = ?", (SCHEMA_VERSION + 1,)) + conn.commit() + finally: + conn.close() + + with pytest.raises(SchemaVersionError): + ResultStore(db_path) + + # Refusal must not have rewritten the version row to make the problem + # disappear on the next attempt. + assert _stored_version(db_path) == SCHEMA_VERSION + 1 From 6ff293aa99f606de0cc198add72fe7e2573e53dc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 09:47:46 +0000 Subject: [PATCH 2/2] fix(store): seed run_counter from existing runs; check version before applying legacy schema Addresses two review findings from Codex on #46: - P1: a v1 database can already hold runs for "today" under the old MAX(run_id)-in-Python scheme. Migrating to run_counter with an empty counter would hand out RUN--001 again, and because save_run upserts on run_id, that would silently overwrite the pre-existing run. The migration now seeds each day's counter from the highest suffix already in use. - P2: _SCHEMA was applied before the version guard ran, so a database from a future build that had renamed or dropped a v1 table could be mutated, or raise a raw sqlite3.OperationalError, instead of getting the clean SchemaVersionError refusal. The version table is now bootstrapped and checked first, before _SCHEMA or any migration runs. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01H2jDXUwrbM4eupm7CyLivS --- src/agentsec/store/sqlite.py | 25 +++++++++++++++++- tests/test_store_migrations.py | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/agentsec/store/sqlite.py b/src/agentsec/store/sqlite.py index e942a2c..c14e5fd 100644 --- a/src/agentsec/store/sqlite.py +++ b/src/agentsec/store/sqlite.py @@ -96,6 +96,21 @@ day TEXT PRIMARY KEY, next_n INTEGER NOT NULL ); + + -- A v1 database predates run_counter, so it may already hold runs for + -- "today" under the old MAX(run_id)-in-Python scheme. Leaving the + -- counter empty would hand out RUN--001 again, and because + -- save_run upserts on run_id, that would silently overwrite the + -- earlier run. Seed each day's counter from the highest suffix + -- already used, so the next claim continues past it. + INSERT INTO run_counter (day, next_n) + SELECT substr(run_id, 5, 8) AS day, + MAX(CAST(substr(run_id, 14, 3) AS INTEGER)) AS next_n + FROM runs + WHERE run_id LIKE 'RUN-________-___' + GROUP BY day + ON CONFLICT(day) DO UPDATE SET + next_n = MAX(run_counter.next_n, excluded.next_n); """, ), ] @@ -131,7 +146,13 @@ def _conn(self) -> Iterator[sqlite3.Connection]: def _init_schema(self) -> None: with self._conn() as conn: - conn.executescript(_SCHEMA) + # Bootstrap only the version table first, and check it, before + # applying _SCHEMA or any migration: a database from a future + # build may have renamed or dropped a table _SCHEMA still + # expects, and running that script against it could mutate the + # database or raise a raw sqlite3.OperationalError instead of + # the clean refusal this guard promises. + conn.execute("CREATE TABLE IF NOT EXISTS schema_version (version INTEGER NOT NULL)") row = conn.execute("SELECT version FROM schema_version").fetchone() current = row["version"] if row is not None else 0 @@ -142,6 +163,8 @@ def _init_schema(self) -> None: details={"found_version": current, "supported_version": SCHEMA_VERSION}, ) + conn.executescript(_SCHEMA) + for target_version, migration_sql in _MIGRATIONS: if current < target_version: conn.executescript(migration_sql) diff --git a/tests/test_store_migrations.py b/tests/test_store_migrations.py index ee0ed82..f01fb52 100644 --- a/tests/test_store_migrations.py +++ b/tests/test_store_migrations.py @@ -130,6 +130,52 @@ def test_reopening_an_already_migrated_database_is_a_no_op(tmp_path: Path) -> No assert _stored_version(db_path) == SCHEMA_VERSION +def _insert_run_row(path: Path, run_id: str) -> None: + """Insert a minimal `runs` row directly, bypassing `next_run_id`. + + Mirrors what a real v1 database looks like: rows already exist under the + old MAX(run_id)-in-Python id scheme, with no `run_counter` involved. + """ + conn = sqlite3.connect(path) + try: + conn.execute( + """ + INSERT INTO runs (run_id, scenario_id, target_id, profile, status, + created_at, payload) + VALUES (?, 'AGT-XPIA-001', 'demo-agent-fixture', 'pr', 'completed', + '2026-08-11T00:00:00+00:00', '{}') + """, + (run_id,), + ) + conn.commit() + finally: + conn.close() + + +def test_migrating_a_v1_database_with_existing_runs_seeds_the_counter_past_them( + tmp_path: Path, +) -> None: + """A v1 database may already hold runs for "today" under the old + MAX(run_id)-in-Python scheme. An empty `run_counter` after migration + would hand out `RUN--001` again, and because `save_run` upserts on + `run_id`, that would silently overwrite the pre-existing run (#44, + review comment on #46). The migration must seed each day's counter past + the highest suffix already in use. + """ + db_path = tmp_path / "legacy_with_runs.db" + _make_v1_database(db_path) + _insert_run_row(db_path, "RUN-20260811-001") + _insert_run_row(db_path, "RUN-20260811-002") + _insert_run_row(db_path, "RUN-20260810-005") # a different day, lower number + + store = ResultStore(db_path) + + assert store.next_run_id("20260811") == "RUN-20260811-003" + assert store.next_run_id("20260810") == "RUN-20260810-006" + # A day with no prior runs still starts from scratch. + assert store.next_run_id("20260101") == "RUN-20260101-001" + + def test_a_future_schema_version_is_refused_rather_than_silently_opened( tmp_path: Path, ) -> None: