diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c8db64..3ec5ff2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,8 @@ All notable changes are documented here, following [Keep a Changelog](https://ke ### Added +- Foreign keys (`worm-audit:11-12`), `ON DELETE RESTRICT` only: chain link -> core audit header, sealed export -> its first and last checkpoints, export verification -> its export, with their indexes. Link references (ranges, break link, purge anchor) stay unconstrained because the retention purge deletes the links they outlive. Pre-existing orphans are copied to `qqq_fk_quarantine` (`KEPT`), never modified, and leave that constraint `NOT VALID`. Re-apply the rendered DDL (idempotent). - Guided empty states: `help/qbit-worm-audit.yaml` (QQQ help-bundle format) gives the app and all six tables a summary and a "How rows get created" section, applied by the `WormAuditHelp` stand-in (`STAND-IN(core=QRun-IO/qqq#1198, removeBy=4.1.0-RC.7)`) without replacing host help. `WormAuditTablesGuardrailTest` pins runtime use, seeding or a documented reason, and help for every table; `WormAuditSettingsSeedTest` runs the script's settings seed on H2. - - RC.2 settings: editable, FIELD-audited singleton with insert-only defaults, validation, separate edit permission and a 30-second effective-settings cache. Schedules load at startup. - GOVERNANCE exports with 30-day default retention; per-export retention verification, administrator-bypass diagnostics, home warnings and documented COMPLIANCE transition. - New `worm-audit:10` settings migration; historical changesets remain unchanged. diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 0cde182..49ad2d4 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -111,6 +111,17 @@ file into their changelog (kof22: `changesets/features/YYYY-MM-DD-worm-audit.sql qbit tables, the functions, the triggers, the purger role (`CREATE ROLE ... NOLOGIN` when it is missing) and the grants. It is idempotent (`CREATE OR REPLACE`, `DROP TRIGGER IF EXISTS`). +Foreign keys (`worm-audit:11-12`) are `ON DELETE RESTRICT` only, so nothing cascades into or out +of evidence: `worm_audit_chain.audit_id` -> the core audit header (the purge deletes details, then +links, then headers, in that order), `worm_audit_export.from/to_checkpoint_id` -> checkpoints and +`worm_audit_verification.export_id` -> exports (neither is ever deleted). References to chain links +(checkpoint, export and verification ranges, `first_break_link_id`, the purge anchor) are not +foreign keys: the purge deletes the sealed prefix those rows outlive. On upgrade, rows whose parent +is already missing are evidence and are never changed: they are copied to `qqq_fk_quarantine` +(action `KEPT`) and that constraint stays `NOT VALID` (new rows are still checked). With the +foreign key, a plain `TRUNCATE` of `audit` (or of checkpoints or exports) is refused by PostgreSQL +before the guard trigger; `TRUNCATE ... CASCADE` reaches the guards, which refuse it. + ### 4.5 Threat model summary | Actor | Can modify audit rows? | Detected by | diff --git a/docs/ddl/worm-audit-postgres.sql b/docs/ddl/worm-audit-postgres.sql index 1561309..97f5b3d 100644 --- a/docs/ddl/worm-audit-postgres.sql +++ b/docs/ddl/worm-audit-postgres.sql @@ -351,3 +351,37 @@ INSERT INTO public.worm_audit_settings export_cron, verify_cron) SELECT 1, 6, 'GOVERNANCE', 30, 10000, 60, 30, 60, 24, '0 0 2 * * ?', '0 0 3 * * ?' WHERE NOT EXISTS (SELECT 1 FROM public.worm_audit_settings WHERE id = 1); + +-- changeset worm-audit:11 +-- comment: Index the foreign key columns added by worm-audit:12 (worm_audit_chain.audit_id already leads worm_audit_chain_audit_all). +CREATE INDEX IF NOT EXISTS worm_audit_export_from_checkpoint ON public.worm_audit_export(from_checkpoint_id); +CREATE INDEX IF NOT EXISTS worm_audit_export_to_checkpoint ON public.worm_audit_export(to_checkpoint_id); +CREATE INDEX IF NOT EXISTS worm_audit_verification_export ON public.worm_audit_verification(export_id); + +-- changeset worm-audit:12 splitStatements:false +-- comment: Foreign keys, ON DELETE RESTRICT only: nothing cascades into or out of audit evidence. A chain link needs its audit header (the purge deletes links before headers); a sealed export needs its signed checkpoints and an export verification its export (neither is ever deleted). Links referenced by checkpoints, exports, verifications and purges are not foreign keys: the purge deletes the sealed prefix those rows outlive. Rows whose parent is already missing are evidence and are never changed: they are copied to qqq_fk_quarantine (action KEPT) and the constraint is left NOT VALID (enforced for new rows only). +DO $$ +DECLARE + fk record; + orphans bigint; +BEGIN + CREATE TABLE IF NOT EXISTS qqq_fk_quarantine (id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, table_name VARCHAR(100) NOT NULL, column_name VARCHAR(100) NOT NULL, row_id VARCHAR(100), missing_parent_id VARCHAR(100), action VARCHAR(20) NOT NULL, row_data TEXT, changeset VARCHAR(200), quarantined_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP); + FOR fk IN SELECT * FROM (VALUES + ('fk_worm_audit_chain_audit_id', 'worm_audit_chain', 'audit_id', '"audit"'), + ('fk_worm_audit_export_from_checkpoint_id', 'worm_audit_export', 'from_checkpoint_id', 'public.worm_audit_checkpoint'), + ('fk_worm_audit_export_to_checkpoint_id', 'worm_audit_export', 'to_checkpoint_id', 'public.worm_audit_checkpoint'), + ('fk_worm_audit_verification_export_id', 'worm_audit_verification', 'export_id', 'public.worm_audit_export') + ) AS t(name, child, col, parent) LOOP + IF NOT EXISTS (SELECT FROM pg_constraint WHERE conname = fk.name) THEN + EXECUTE format('INSERT INTO qqq_fk_quarantine (table_name, column_name, row_id, missing_parent_id, action, row_data, changeset)' + || ' SELECT %L, %L, c.id::text, c.%I::text, ''KEPT'', row_to_json(c)::text, ''worm-audit:12'' FROM public.%I c' + || ' WHERE c.%I IS NOT NULL AND NOT EXISTS (SELECT FROM %s p WHERE p.id = c.%I)', fk.child, fk.col, fk.col, fk.child, fk.col, fk.parent, fk.col); + GET DIAGNOSTICS orphans = ROW_COUNT; + EXECUTE format('ALTER TABLE public.%I ADD CONSTRAINT %I FOREIGN KEY (%I) REFERENCES %s (id) ON DELETE RESTRICT NOT VALID', fk.child, fk.name, fk.col, fk.parent); + IF orphans = 0 THEN + EXECUTE format('ALTER TABLE public.%I VALIDATE CONSTRAINT %I', fk.child, fk.name); + END IF; + END IF; + END LOOP; +END; +$$; diff --git a/src/main/resources/com/kingsrook/qbits/wormaudit/ddl/postgres/worm-audit.sql b/src/main/resources/com/kingsrook/qbits/wormaudit/ddl/postgres/worm-audit.sql index 7b85a92..e5135c2 100644 --- a/src/main/resources/com/kingsrook/qbits/wormaudit/ddl/postgres/worm-audit.sql +++ b/src/main/resources/com/kingsrook/qbits/wormaudit/ddl/postgres/worm-audit.sql @@ -351,3 +351,37 @@ INSERT INTO public.worm_audit_settings export_cron, verify_cron) SELECT ${settings_values} WHERE NOT EXISTS (SELECT 1 FROM public.worm_audit_settings WHERE id = 1); + +-- changeset worm-audit:11 +-- comment: Index the foreign key columns added by worm-audit:12 (worm_audit_chain.audit_id already leads worm_audit_chain_audit_all). +CREATE INDEX IF NOT EXISTS worm_audit_export_from_checkpoint ON public.worm_audit_export(from_checkpoint_id); +CREATE INDEX IF NOT EXISTS worm_audit_export_to_checkpoint ON public.worm_audit_export(to_checkpoint_id); +CREATE INDEX IF NOT EXISTS worm_audit_verification_export ON public.worm_audit_verification(export_id); + +-- changeset worm-audit:12 splitStatements:false +-- comment: Foreign keys, ON DELETE RESTRICT only: nothing cascades into or out of audit evidence. A chain link needs its audit header (the purge deletes links before headers); a sealed export needs its signed checkpoints and an export verification its export (neither is ever deleted). Links referenced by checkpoints, exports, verifications and purges are not foreign keys: the purge deletes the sealed prefix those rows outlive. Rows whose parent is already missing are evidence and are never changed: they are copied to qqq_fk_quarantine (action KEPT) and the constraint is left NOT VALID (enforced for new rows only). +DO $$ +DECLARE + fk record; + orphans bigint; +BEGIN + CREATE TABLE IF NOT EXISTS qqq_fk_quarantine (id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, table_name VARCHAR(100) NOT NULL, column_name VARCHAR(100) NOT NULL, row_id VARCHAR(100), missing_parent_id VARCHAR(100), action VARCHAR(20) NOT NULL, row_data TEXT, changeset VARCHAR(200), quarantined_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP); + FOR fk IN SELECT * FROM (VALUES + ('fk_worm_audit_chain_audit_id', 'worm_audit_chain', 'audit_id', '${audit}'), + ('fk_worm_audit_export_from_checkpoint_id', 'worm_audit_export', 'from_checkpoint_id', 'public.worm_audit_checkpoint'), + ('fk_worm_audit_export_to_checkpoint_id', 'worm_audit_export', 'to_checkpoint_id', 'public.worm_audit_checkpoint'), + ('fk_worm_audit_verification_export_id', 'worm_audit_verification', 'export_id', 'public.worm_audit_export') + ) AS t(name, child, col, parent) LOOP + IF NOT EXISTS (SELECT FROM pg_constraint WHERE conname = fk.name) THEN + EXECUTE format('INSERT INTO qqq_fk_quarantine (table_name, column_name, row_id, missing_parent_id, action, row_data, changeset)' + || ' SELECT %L, %L, c.id::text, c.%I::text, ''KEPT'', row_to_json(c)::text, ''worm-audit:12'' FROM public.%I c' + || ' WHERE c.%I IS NOT NULL AND NOT EXISTS (SELECT FROM %s p WHERE p.id = c.%I)', fk.child, fk.col, fk.col, fk.child, fk.col, fk.parent, fk.col); + GET DIAGNOSTICS orphans = ROW_COUNT; + EXECUTE format('ALTER TABLE public.%I ADD CONSTRAINT %I FOREIGN KEY (%I) REFERENCES %s (id) ON DELETE RESTRICT NOT VALID', fk.child, fk.name, fk.col, fk.parent); + IF orphans = 0 THEN + EXECUTE format('ALTER TABLE public.%I VALIDATE CONSTRAINT %I', fk.child, fk.name); + END IF; + END IF; + END LOOP; +END; +$$; diff --git a/src/test/java/com/kingsrook/qbits/wormaudit/PostgresAcceptanceTest.java b/src/test/java/com/kingsrook/qbits/wormaudit/PostgresAcceptanceTest.java index a089bba..3a94d45 100644 --- a/src/test/java/com/kingsrook/qbits/wormaudit/PostgresAcceptanceTest.java +++ b/src/test/java/com/kingsrook/qbits/wormaudit/PostgresAcceptanceTest.java @@ -229,11 +229,21 @@ class PostgresAcceptanceTest assertThat(((SQLException) e).getSQLState()) .isEqualTo("42501")); sql(c, "GRANT UPDATE, DELETE, TRUNCATE ON " + table + " TO app"); - for(String operation : - List.of( - "UPDATE " + table + " SET id=id", - "DELETE FROM " + table, - "TRUNCATE " + table)) + // A table a foreign key references (audit, checkpoint, export) refuses TRUNCATE before + // its guard runs; TRUNCATE ... CASCADE (owner) reaches the guards, which refuse it. + if(count(c, "SELECT count(*) FROM pg_constraint WHERE contype='f' AND confrelid='" + table + "'::regclass") > 0) + { + assertThatThrownBy(() -> sql(runtime, "TRUNCATE " + table)) + .isInstanceOf(SQLException.class) + .hasMessageContaining("referenced in a foreign key constraint"); + assertThatThrownBy(() -> sql(c, "TRUNCATE " + table + " CASCADE")) + .isInstanceOf(SQLException.class) + .hasMessageContaining("worm_audit:"); + } + boolean referenced = count(c, "SELECT count(*) FROM pg_constraint WHERE contype='f' AND confrelid='" + table + "'::regclass") > 0; + for(String operation : referenced + ? List.of("UPDATE " + table + " SET id=id", "DELETE FROM " + table) + : List.of("UPDATE " + table + " SET id=id", "DELETE FROM " + table, "TRUNCATE " + table)) { assertThatThrownBy(() -> sql(runtime, operation)) .isInstanceOf(SQLException.class) @@ -583,6 +593,91 @@ class PostgresAcceptanceTest } + /******************************************************************************* + ** Referential integrity guardrail: every table reference of the WORM tables + ** (possible-value fields, and the chain's audit header) has a RESTRICT + ** foreign key, or an exemption with its reason. Nothing may cascade into or + ** out of audit evidence. + *******************************************************************************/ + @Test + void everyReferenceHasARestrictForeignKeyOrAReason() throws Exception + { + String links = "the retention purge deletes the sealed prefix of chain links; this evidence row outlives the links it names"; + java.util.Map exempt = java.util.Map.of( + "worm_audit_checkpoint.from_link_id", links, + "worm_audit_checkpoint.to_link_id", links, + "worm_audit_export.from_link_id", links, + "worm_audit_export.to_link_id", links, + "worm_audit_verification.from_link_id", links, + "worm_audit_verification.to_link_id", links, + "worm_audit_verification.first_break_link_id", links + "; a break may name a link that is missing", + "worm_audit_purge.anchor_link_id", "the anchor is the last link the same purge deleted"); + + java.util.Set references = new java.util.TreeSet<>(List.of("worm_audit_chain.audit_id")); + for(var table : instance.getTables().values()) + { + if(!table.getName().startsWith("worm_audit_")) + { + continue; + } + for(var field : table.getFields().values()) + { + if(field.getPossibleValueSourceName() != null && instance.getTable(field.getPossibleValueSourceName()) != null) + { + references.add(table.getName() + "." + field.getBackendName()); + } + } + } + + java.util.Map foreignKeys = new java.util.HashMap<>(); + try(var c = owner(); var s = c.createStatement(); var r = s.executeQuery(""" + SELECT c.conrelid::regclass::text || '.' || a.attname, c.confdeltype::text || c.convalidated::text + FROM pg_constraint c JOIN pg_attribute a ON a.attrelid = c.conrelid AND a.attnum = c.conkey[1] + WHERE c.contype = 'f' AND c.conrelid::regclass::text LIKE 'worm_audit_%'""")) + { + while(r.next()) + { + foreignKeys.put(r.getString(1), r.getString(2)); + } + } + assertThat(references.stream().filter(ref -> !foreignKeys.containsKey(ref) && !exempt.containsKey(ref)).toList()) + .as("references without a foreign key (add one to worm-audit.sql, or an exemption with a reason)").isEmpty(); + assertThat(foreignKeys).as("RESTRICT and validated").containsOnlyKeys("worm_audit_chain.audit_id", "worm_audit_export.from_checkpoint_id", + "worm_audit_export.to_checkpoint_id", "worm_audit_verification.export_id").allSatisfy((k, v) -> assertThat(v).isEqualTo("rtrue")); + } + + + + /******************************************************************************* + ** Upgrading a schema whose chain names a missing audit header: the link is + ** evidence, so it is copied to qqq_fk_quarantine (KEPT), never changed, and + ** the constraint is added NOT VALID (new links are still checked). A re-run + ** changes nothing. + *******************************************************************************/ + @Test + void orphanEvidenceIsKeptAndTheConstraintIsNotValidated() throws Exception + { + audit(1); + new WormAuditChainer(config).chainOnce(); + try(var c = owner()) + { + sql(c, "ALTER TABLE worm_audit_chain DROP CONSTRAINT fk_worm_audit_chain_audit_id"); + sql(c, "ALTER TABLE audit DISABLE TRIGGER worm_audit_row_guard; DELETE FROM audit WHERE id = 1; ALTER TABLE audit ENABLE TRIGGER worm_audit_row_guard"); + sql(c, WormAuditDdl.postgres(config)); + sql(c, WormAuditDdl.postgres(config)); + assertThat(count(c, "SELECT count(*) FROM qqq_fk_quarantine WHERE table_name = 'worm_audit_chain' AND column_name = 'audit_id'" + + " AND row_id = '1' AND missing_parent_id = '1' AND action = 'KEPT' AND row_data LIKE '%\"link_hash\"%'")).isEqualTo(1); + assertThat(count(c, "SELECT count(*) FROM qqq_fk_quarantine")).isEqualTo(1); + assertThat(count(c, "SELECT count(*) FROM worm_audit_chain WHERE id = 1 AND audit_id = 1")).isEqualTo(1); + assertThat(count(c, "SELECT count(*) FROM pg_constraint WHERE conname = 'fk_worm_audit_chain_audit_id' AND NOT convalidated")).isEqualTo(1); + assertThatThrownBy(() -> sql(c, "INSERT INTO worm_audit_chain(id,kind,audit_id,detail_ids,prev_hash,link_hash,chained_at)" + + " VALUES(99,'AUDIT',404,'',repeat('0',64),repeat('0',64),now())")) + .hasMessageContaining("fk_worm_audit_chain_audit_id"); + } + } + + + /******************************************************************************* ** The settings row (seeded by the DDL) overrides config: change it there. *******************************************************************************/