From be9ce8ede6142a67402f3749b24d2390e064fa93 Mon Sep 17 00:00:00 2001 From: timmy Date: Wed, 19 Aug 2026 07:37:35 +0000 Subject: [PATCH] security: remove plaintext journal metadata columns --- src/security_event_store.py | 30 +++++++++++++----------- tests/test_security_event_store.py | 37 ++++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 24 deletions(-) diff --git a/src/security_event_store.py b/src/security_event_store.py index fceb445..c60a4b7 100644 --- a/src/security_event_store.py +++ b/src/security_event_store.py @@ -75,10 +75,6 @@ class SecurityEventStore: """ CREATE TABLE IF NOT EXISTS security_events ( id INTEGER PRIMARY KEY AUTOINCREMENT, - kind TEXT, - method TEXT, - device_label TEXT, - target TEXT, payload TEXT, created_at INTEGER NOT NULL, status TEXT NOT NULL DEFAULT 'completed', @@ -97,8 +93,11 @@ class SecurityEventStore: connection.execute( "ALTER TABLE security_events ADD COLUMN operation_id TEXT" ) - if "payload" not in columns: - self._migrate_plaintext(connection) + plaintext_columns = {"kind", "method", "device_label", "target"} + if "payload" not in columns or plaintext_columns.intersection(columns): + self._migrate_plaintext( + connection, has_payload="payload" in columns + ) connection.execute( "CREATE INDEX IF NOT EXISTS security_events_created " "ON security_events(created_at DESC, id DESC)" @@ -113,20 +112,19 @@ class SecurityEventStore: "Security activity is temporarily unavailable" ) from exc - def _migrate_plaintext(self, connection: sqlite3.Connection) -> None: + def _migrate_plaintext( + self, connection: sqlite3.Connection, *, has_payload: bool + ) -> None: connection.execute("PRAGMA secure_delete = ON") + payload_column = "payload," if has_payload else "NULL AS payload," rows = connection.execute( - "SELECT id, kind, method, device_label, target, created_at, status, operation_id " - "FROM security_events ORDER BY id" + f"SELECT id, {payload_column} kind, method, device_label, target, " + "created_at, status, operation_id FROM security_events ORDER BY id" ).fetchall() connection.execute( """ CREATE TABLE security_events_encrypted ( id INTEGER PRIMARY KEY AUTOINCREMENT, - kind TEXT, - method TEXT, - device_label TEXT, - target TEXT, payload TEXT, created_at INTEGER NOT NULL, status TEXT NOT NULL DEFAULT 'completed', @@ -136,6 +134,7 @@ class SecurityEventStore: ) for ( event_id, + payload, kind, method, device_label, @@ -149,7 +148,10 @@ class SecurityEventStore: "(id, payload, created_at, status, operation_id) VALUES (?, ?, ?, ?, ?)", ( event_id, - self._seal_event(event_id, kind, method, device_label, target), + payload + or self._seal_event( + event_id, kind, method, device_label, target + ), created_at, status, operation_id, diff --git a/tests/test_security_event_store.py b/tests/test_security_event_store.py index 8c47b09..2be12ca 100644 --- a/tests/test_security_event_store.py +++ b/tests/test_security_event_store.py @@ -8,6 +8,26 @@ from src.security_event_store import SecurityEventStore, SecurityEventStoreError PRIVATE_KEY = b"e" * 32 +def test_security_event_schema_exposes_no_plaintext_metadata_columns(tmp_path): + store = SecurityEventStore( + tmp_path / "security.sqlite3", clock=lambda: 1_000, encryption_key=PRIVATE_KEY + ) + store.record("issue_closed", method="passkey", target="private/repo#42") + + with sqlite3.connect(store.path) as connection: + columns = { + row[1] + for row in connection.execute("PRAGMA table_info(security_events)") + } + assert columns == { + "id", + "payload", + "created_at", + "status", + "operation_id", + } + + def test_security_event_payload_is_encrypted_at_rest_and_survives_restart(tmp_path): path = tmp_path / "security.sqlite3" canaries = { @@ -21,11 +41,10 @@ def test_security_event_payload_is_encrypted_at_rest_and_survives_restart(tmp_pa store.record(**canaries) with sqlite3.connect(path) as connection: - row = connection.execute( - "SELECT kind, method, device_label, target, payload FROM security_events" - ).fetchone() - assert row[:4] == (None, None, None, None) - assert row[4].startswith("v1:") + payload = connection.execute( + "SELECT payload FROM security_events" + ).fetchone()[0] + assert payload.startswith("v1:") database_bytes = path.read_bytes() assert all(value.encode() not in database_bytes for value in canaries.values()) reopened = SecurityEventStore(path, clock=lambda: 1_001, encryption_key=PRIVATE_KEY) @@ -74,11 +93,10 @@ def test_legacy_security_events_migrate_without_changing_journal_semantics(tmp_p assert store.list(limit=1).events[0].status == "completed" with sqlite3.connect(path) as connection: rows = connection.execute( - "SELECT id, kind, method, device_label, target, payload FROM security_events ORDER BY id" + "SELECT id, payload FROM security_events ORDER BY id" ).fetchall() assert [row[0] for row in rows] == [7, 9] - assert all(row[1:5] == (None, None, None, None) for row in rows) - assert all(row[5].startswith("v1:") for row in rows) + assert all(row[1].startswith("v1:") for row in rows) database_bytes = path.read_bytes() assert b"legacy_sign_in_canary" not in database_bytes assert b"Legacy Tablet Canary" not in database_bytes @@ -144,8 +162,7 @@ def test_security_events_are_private_bounded_and_reverse_chronological(tmp_path) row[1] for row in sqlite3.connect(store.path).execute("PRAGMA table_info(security_events)") } assert columns == { - "id", "kind", "method", "device_label", "target", "payload", "created_at", - "status", "operation_id", + "id", "payload", "created_at", "status", "operation_id", }