From b9c50da0f735ad447fe3a175005bf5000861db90 Mon Sep 17 00:00:00 2001 From: Peter Permenter <41281403+TusanHomichi@users.noreply.github.com> Date: Sat, 5 Sep 2026 02:44:37 +0200 Subject: [PATCH] storage(schema): enforce enrollment event reference shape --- .../0014_enrollment_event_shape.sql | 35 ++ .../tests/enrollment_event_schema.rs | 411 ++++++++++++++++++ ...008-session-draft-and-attribution-model.md | 2 + .../0018-enrollment-event-reference-shape.md | 89 ++++ docs/development.md | 6 +- 5 files changed, 542 insertions(+), 1 deletion(-) create mode 100644 crates/consolebook-server/migrations/0014_enrollment_event_shape.sql create mode 100644 crates/consolebook-server/tests/enrollment_event_schema.rs create mode 100644 docs/decisions/0018-enrollment-event-reference-shape.md diff --git a/crates/consolebook-server/migrations/0014_enrollment_event_shape.sql b/crates/consolebook-server/migrations/0014_enrollment_event_shape.sql new file mode 100644 index 0000000..390ac3e --- /dev/null +++ b/crates/consolebook-server/migrations/0014_enrollment_event_shape.sql @@ -0,0 +1,35 @@ +-- ADR 0018; #51: only a version change may name program versions. +-- 0006's CHECK rejects two references on other kinds but admits one. +-- Preserve that migration's checksum and the referenced, append-only table. +-- Its existing no-update trigger and this insert trigger enforce the full +-- shape without rebuilding history or disabling foreign-key enforcement. + +-- Refuse an upgrade over malformed retained history. Do not infer which +-- reference was intended or silently discard it. SQLx runs this migration +-- and its ledger entry in one transaction; failure rolls back this guard. +CREATE TEMP TABLE enrollment_event_shape_upgrade_guard ( + valid INTEGER NOT NULL, + CONSTRAINT enrollment_event_legacy_version_references_invalid CHECK (valid = 1) +) STRICT; + +INSERT INTO enrollment_event_shape_upgrade_guard (valid) +SELECT 0 FROM enrollment_event +WHERE NOT CASE kind + WHEN 'version_change' + THEN from_program_version_id IS NOT NULL AND to_program_version_id IS NOT NULL + ELSE from_program_version_id IS NULL AND to_program_version_id IS NULL +END +LIMIT 1; + +DROP TABLE enrollment_event_shape_upgrade_guard; + +CREATE TRIGGER enrollment_event_version_reference_shape +BEFORE INSERT ON enrollment_event +WHEN NOT CASE NEW.kind + WHEN 'version_change' + THEN NEW.from_program_version_id IS NOT NULL AND NEW.to_program_version_id IS NOT NULL + ELSE NEW.from_program_version_id IS NULL AND NEW.to_program_version_id IS NULL +END +BEGIN + SELECT RAISE(ABORT, 'enrollment events name both versions for a version change and neither otherwise'); +END; diff --git a/crates/consolebook-server/tests/enrollment_event_schema.rs b/crates/consolebook-server/tests/enrollment_event_schema.rs new file mode 100644 index 0000000..ad9a391 --- /dev/null +++ b/crates/consolebook-server/tests/enrollment_event_schema.rs @@ -0,0 +1,411 @@ +//! Enrollment-event shape is enforced by migrations, including upgrades from +//! the schema that admitted a single version reference on other event kinds. +//! All rows and names are invented; no retained history is rewritten to seed tests. + +use std::borrow::Cow; +use std::path::Path; + +use consolebook_server::{export_verify, storage, trainee_packet}; +use sqlx::sqlite::{SqliteConnectOptions, SqliteConnection, SqliteJournalMode, SqlitePoolOptions}; +use sqlx::{Connection, SqlitePool}; + +const LEGACY_VERSION: i64 = 13; +const SHAPE_VERSION: i64 = 14; +const EXPORTED_AT: i64 = 1_788_289_200; + +async fn connect(path: &Path) -> SqliteConnection { + SqliteConnection::connect_with( + &SqliteConnectOptions::new() + .filename(path) + .create_if_missing(true) + .foreign_keys(true) + .journal_mode(SqliteJournalMode::Wal), + ) + .await + .expect("fixture connection") +} + +async fn legacy_database(path: &Path) -> SqliteConnection { + let mut connection = connect(path).await; + let legacy = sqlx::migrate::Migrator { + migrations: Cow::Owned( + storage::MIGRATOR + .iter() + .filter(|migration| migration.version <= LEGACY_VERSION) + .cloned() + .collect(), + ), + ..sqlx::migrate::Migrator::DEFAULT + }; + legacy + .run(&mut connection) + .await + .expect("legacy migrations"); + seed(&mut connection).await; + connection +} + +async fn seed(connection: &mut SqliteConnection) { + sqlx::raw_sql( + "INSERT INTO instance (id, installation_id, created_at_utc) + VALUES (1, 'invented-schema-installation', '2026-09-01T00:00:00Z'); + INSERT INTO user (id, username, display_name, password_hash, created_at) + VALUES (1, 'casey.schema', 'Casey Example', 'unused-invented-fixture', 1); + INSERT INTO capability_grant (user_id, capability, granted_at) + VALUES (1, 'export_records', 1); + INSERT INTO program (id, name, created_at) VALUES (1, 'Invented Schema Program', 1); + INSERT INTO program_version + (id, program_id, version_number, label, name, description, created_at) + VALUES (1, 1, 1, 'rev A', 'Invented Schema Program', '', 1), + (2, 1, 2, 'rev B', 'Invented Schema Program', '', 1), + (3, 1, 3, 'draft C', 'Invented Schema Program', '', 1); + INSERT INTO phase (id, program_version_id, name, description, presentation_number) + VALUES (1, 1, 'Phase One', '', 1), (2, 2, 'Phase Two', '', 1); + UPDATE program_version SET published_at = 1 WHERE id IN (1, 2); + INSERT INTO enrollment (id, user_id, program_version_id, enrolled_at) + VALUES (1, 1, 1, 1);", + ) + .execute(connection) + .await + .expect("invented base rows"); +} + +async fn insert_event( + connection: &mut SqliteConnection, + kind: &str, + from: Option, + to: Option, +) -> sqlx::Result { + sqlx::query( + "INSERT INTO enrollment_event + (enrollment_id, kind, occurred_at, actor_user_id, reason, + from_program_version_id, to_program_version_id) + VALUES (1, ?1, 20, 1, 'Invented lifecycle event.', ?2, ?3)", + ) + .bind(kind) + .bind(from) + .bind(to) + .execute(connection) + .await +} + +async fn reference_matrix(connection: &mut SqliteConnection) { + for kind in ["version_change", "withdraw", "complete", "reinstate"] { + for (from, to) in [ + (None, None), + (Some(1), None), + (None, Some(2)), + (Some(1), Some(2)), + ] { + let valid = if kind == "version_change" { + from.is_some() && to.is_some() + } else { + from.is_none() && to.is_none() + }; + let result = insert_event(connection, kind, from, to).await; + assert_eq!( + result.is_ok(), + valid, + "{kind} from={from:?} to={to:?}: {result:?}" + ); + if kind != "version_change" && !valid { + let message = result.expect_err("shape refused").to_string(); + assert!( + message.contains("enrollment events name both versions"), + "{message}" + ); + } + } + } +} + +#[tokio::test] +async fn fresh_schema_enforces_the_complete_reference_matrix() { + let tmp = tempfile::tempdir().expect("scratch"); + let mut connection = connect(&tmp.path().join("consolebook.db")).await; + storage::MIGRATOR + .run(&mut connection) + .await + .expect("migrate"); + seed(&mut connection).await; + reference_matrix(&mut connection).await; + let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM enrollment_event") + .fetch_one(&mut connection) + .await + .expect("count accepted rows"); + assert_eq!(count, 4, "only the four valid shapes append rows"); + connection.close().await.expect("close"); +} + +type EventRow = ( + i64, + i64, + String, + i64, + Option, + String, + Option, + Option, +); +type PhaseRow = ( + i64, + i64, + String, + Option, + Option, + i64, + i64, + Option, + String, + Option, +); +type SchemaRow = (String, String, String, String); + +#[derive(Debug, PartialEq, Eq)] +struct History { + events: Vec, + phases: Vec, + pin: i64, +} + +async fn history(connection: &mut SqliteConnection) -> History { + History { + events: sqlx::query_as( + "SELECT id, enrollment_id, kind, occurred_at, actor_user_id, reason, + from_program_version_id, to_program_version_id + FROM enrollment_event ORDER BY id", + ) + .fetch_all(&mut *connection) + .await + .expect("event history"), + phases: sqlx::query_as( + "SELECT id, enrollment_id, kind, from_phase_id, to_phase_id, effective_at, + recorded_at, actor_user_id, reason, version_change_event_id + FROM phase_event ORDER BY id", + ) + .fetch_all(&mut *connection) + .await + .expect("phase history"), + pin: sqlx::query_scalar("SELECT program_version_id FROM enrollment WHERE id = 1") + .fetch_one(connection) + .await + .expect("pin"), + } +} + +async fn schema(connection: &mut SqliteConnection) -> Vec { + sqlx::query_as( + "SELECT type, name, tbl_name, sql FROM sqlite_schema + WHERE sql IS NOT NULL AND name NOT LIKE 'sqlite_%' ORDER BY type, name", + ) + .fetch_all(connection) + .await + .expect("schema objects") +} + +async fn seed_history(connection: &mut SqliteConnection) { + sqlx::raw_sql( + "INSERT INTO phase_event + (id, enrollment_id, kind, to_phase_id, effective_at, recorded_at, actor_user_id, reason) + VALUES (100, 1, 'advance', 1, 10, 10, 1, 'Invented original phase.'); + INSERT INTO enrollment_event (id, enrollment_id, kind, occurred_at, actor_user_id, reason) + VALUES (200, 1, 'withdraw', 11, 1, 'Invented withdrawal.'), + (201, 1, 'reinstate', 12, 1, 'Invented reinstatement.'); + INSERT INTO enrollment_event + (id, enrollment_id, kind, occurred_at, actor_user_id, reason, + from_program_version_id, to_program_version_id) + VALUES (202, 1, 'version_change', 20, 1, 'Invented revision.', 1, 2); + UPDATE enrollment SET program_version_id = 2 WHERE id = 1; + INSERT INTO phase_event + (id, enrollment_id, kind, to_phase_id, effective_at, recorded_at, + actor_user_id, reason, version_change_event_id) + VALUES (101, 1, 'advance', 2, 20, 21, 1, 'Invented new phase.', 202); + INSERT INTO enrollment_event (id, enrollment_id, kind, occurred_at, actor_user_id, reason) + VALUES (203, 1, 'complete', 30, 1, 'Invented completion.');", + ) + .execute(connection) + .await + .expect("append legacy history"); +} + +async fn packet(pool: &SqlitePool) -> Vec { + let packet = trainee_packet::export_at(pool, 1, 1, EXPORTED_AT) + .await + .expect("export") + .expect("authorized") + .bytes; + let report = export_verify::verify_archive(&packet); + assert!(report.verified(), "{report:?}"); + packet +} + +#[tokio::test] +async fn upgrade_preserves_history_references_schema_objects_and_packet_bytes() { + let tmp = tempfile::tempdir().expect("scratch"); + let path = tmp.path().join("consolebook.db"); + let mut connection = legacy_database(&path).await; + seed_history(&mut connection).await; + let before = history(&mut connection).await; + let before_schema = schema(&mut connection).await; + connection.close().await.expect("close legacy fixture"); + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect_with( + SqliteConnectOptions::new() + .filename(&path) + .foreign_keys(true), + ) + .await + .expect("legacy export connection"); + let before_packet = packet(&pool).await; + pool.close().await; + + // Exercise the real startup path, including migration checksum validation. + let pool = storage::open(&path).await.expect("upgrade at startup"); + let after_packet = packet(&pool).await; + assert_eq!( + before_packet, after_packet, + "fixed-instant packets are byte-identical" + ); + let mut connection = pool.acquire().await.expect("inspect upgraded storage"); + assert_eq!(before, history(&mut connection).await); + let after_schema = schema(&mut connection).await; + let retained_schema: Vec<_> = after_schema + .into_iter() + .filter(|(_, name, _, _)| name != "enrollment_event_version_reference_shape") + .collect(); + assert_eq!( + before_schema, retained_schema, + "existing tables, indexes, and triggers stay intact" + ); + let foreign_keys: i64 = sqlx::query_scalar("PRAGMA foreign_keys") + .fetch_one(&mut *connection) + .await + .expect("foreign keys"); + assert_eq!(foreign_keys, 1); + assert!( + sqlx::query("PRAGMA foreign_key_check") + .fetch_all(&mut *connection) + .await + .expect("foreign key check") + .is_empty() + ); + assert_eq!( + sqlx::query_scalar::<_, String>("PRAGMA integrity_check") + .fetch_one(&mut *connection) + .await + .expect("integrity"), + "ok" + ); + for statement in [ + "UPDATE enrollment_event SET from_program_version_id = 1 WHERE id = 200", + "DELETE FROM enrollment_event WHERE id = 200", + "UPDATE enrollment SET program_version_id = 1 WHERE id = 1", + "INSERT INTO phase_event (enrollment_id, kind, to_phase_id, effective_at, recorded_at, reason) + VALUES (1, 'advance', 2, 31, 31, '')", + "INSERT INTO enrollment_event + (enrollment_id, kind, occurred_at, reason, from_program_version_id, to_program_version_id) + VALUES (1, 'version_change', 31, '', 2, 1)", + ] { + assert!(sqlx::query(statement).execute(&mut *connection).await.is_err(), "{statement}"); + } + // Existing target-publication, different-version, and reason rules survive. + assert!( + insert_event(&mut connection, "version_change", Some(2), Some(2)) + .await + .is_err() + ); + assert!( + insert_event(&mut connection, "version_change", Some(2), Some(999)) + .await + .is_err() + ); + let draft_target = insert_event(&mut connection, "version_change", Some(2), Some(3)) + .await + .expect_err("draft targets remain refused"); + assert!( + draft_target + .to_string() + .contains("published program versions"), + "{draft_target}" + ); + reference_matrix(&mut connection).await; + drop(connection); + storage::MIGRATOR + .run(&pool) + .await + .expect("already upgraded is idempotent"); + pool.close().await; +} + +#[tokio::test] +async fn malformed_legacy_rows_refuse_upgrade_without_rewriting_history() { + for kind in ["withdraw", "complete", "reinstate"] { + for (from, to) in [(Some(1), None), (None, Some(2))] { + let tmp = tempfile::tempdir().expect("scratch"); + let mut connection = legacy_database(&tmp.path().join("consolebook.db")).await; + seed_history(&mut connection).await; + // This is the exact #51 reproduction: the old schema accepts it. + insert_event(&mut connection, kind, from, to) + .await + .expect("legacy loophole"); + let before = history(&mut connection).await; + let before_schema = schema(&mut connection).await; + for _ in 0..2 { + let error = storage::MIGRATOR + .run(&mut connection) + .await + .expect_err("refuse malformed history"); + assert!( + error + .to_string() + .contains("enrollment_event_legacy_version_references_invalid"), + "{error}" + ); + assert_eq!(before, history(&mut connection).await); + assert_eq!(before_schema, schema(&mut connection).await); + let guard_count: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM sqlite_temp_schema WHERE name = 'enrollment_event_shape_upgrade_guard'", + ).fetch_one(&mut connection).await.expect("no temporary guard remains"); + assert_eq!(guard_count, 0); + let latest: i64 = sqlx::query_scalar("SELECT MAX(version) FROM _sqlx_migrations") + .fetch_one(&mut connection) + .await + .expect("migration ledger"); + assert_eq!(latest, LEGACY_VERSION); + } + connection.close().await.expect("close"); + } + } +} + +#[tokio::test] +async fn startup_fails_closed_on_malformed_legacy_history() { + let tmp = tempfile::tempdir().expect("scratch"); + let path = tmp.path().join("consolebook.db"); + let mut connection = legacy_database(&path).await; + insert_event(&mut connection, "withdraw", Some(1), None) + .await + .expect("legacy loophole"); + let before = history(&mut connection).await; + connection.close().await.expect("stop fixture"); + let error = storage::open(&path).await.expect_err("startup must stop"); + assert!( + format!("{error:#}").contains("enrollment_event_legacy_version_references_invalid"), + "{error:#}" + ); + let pool = storage::open_diagnostic(&path) + .await + .expect("read-only inspection"); + let mut connection = pool.acquire().await.expect("inspect"); + assert_eq!(before, history(&mut connection).await); + let applied: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM _sqlx_migrations WHERE version = ?1") + .bind(SHAPE_VERSION) + .fetch_one(&mut *connection) + .await + .expect("ledger"); + assert_eq!(applied, 0); + drop(connection); + pool.close().await; +} diff --git a/docs/decisions/0008-session-draft-and-attribution-model.md b/docs/decisions/0008-session-draft-and-attribution-model.md index 938d327..ac2011a 100644 --- a/docs/decisions/0008-session-draft-and-attribution-model.md +++ b/docs/decisions/0008-session-draft-and-attribution-model.md @@ -2,6 +2,8 @@ - **Status:** Accepted - **Date:** 2026-08-28 +- **Amended by:** [ADR 0018](0018-enrollment-event-reference-shape.md), which + completes database enforcement of enrollment-event version-reference shape. ## Context diff --git a/docs/decisions/0018-enrollment-event-reference-shape.md b/docs/decisions/0018-enrollment-event-reference-shape.md new file mode 100644 index 0000000..71ce8e4 --- /dev/null +++ b/docs/decisions/0018-enrollment-event-reference-shape.md @@ -0,0 +1,89 @@ +# ADR 0018: Enrollment-event version-reference shape + +- **Status:** Accepted +- **Date:** 2026-09-05 +- **Issue:** [#51](https://github.com/FieldmouseWorks/consolebook/issues/51) +- **Amends:** [ADR 0008](0008-session-draft-and-attribution-model.md) + +## Context + +Only a version-change event names program versions. Migration 0006 compared +`kind = 'version_change'` with both references being non-null. For other kinds, +that comparison requires at least one null reference, allowing a withdrawal, +completion, or reinstatement with exactly one version reference. The domain +service already writes both references for a version change and neither for +other kinds, and the packet verifier already refuses the malformed shape. +The database boundary must enforce the same rule. + +## Decision + +Forward migration 0014 adds `enrollment_event_version_reference_shape`, a +`BEFORE INSERT` trigger enforcing: + +```sql +CASE kind + WHEN 'version_change' + THEN from_program_version_id IS NOT NULL AND to_program_version_id IS NOT NULL + ELSE from_program_version_id IS NULL AND to_program_version_id IS NULL +END +``` + +The existing unconditional no-update trigger covers later changes. The +original CHECK constraints still enforce different version identities and a +reason for a version change; target-publication and append-only triggers +remain intact. No service, packet shape, packet format version, or persisted +event value changes. Migration 0006 and its checksum remain unchanged. + +### Upgrade over existing history + +Before installing the new trigger, migration 0014 evaluates the full shape +against existing events. A malformed row fails a temporary guard's named +constraint, `enrollment_event_legacy_version_references_invalid`. SQLx applies +the script and its migration-ledger entry in one transaction, so failure +leaves the history, existing schema, and ledger at their prior state; the +temporary guard rolls back too. Startup reports the migration failure and +does not serve the installation. + +The migration does not clear the reference, infer a new event kind, delete a +row, or mark the migration applied despite malformed history. The operator +must preserve the source installation and resolve its history through a +separately authorized repair decision. This migration provides no automatic +repair command. Retrying without resolving the malformed history fails again. +Valid installations, including those with phase events referencing recorded +version-change IDs, upgrade without copying their event tables. + +### Why a trigger instead of rebuilding the table + +The issue proposed replacing the CHECK by rebuilding `enrollment_event`. +This is unnecessary for an append-only table whose database contract already +uses triggers. A trigger adds the missing enforcement while preserving table +identity, incoming `phase_event` foreign keys, indexes, and existing triggers. +The temporary guard supplies the legacy-row validation that adding a trigger +alone would omit. + +SQLite's [generalized table-rebuild procedure](https://www.sqlite.org/lang_altertable.html#making_other_kinds_of_table_schema_changes) +requires care with incoming references and foreign-key enforcement. SQLx's +SQLite migration implementation wraps each script in a transaction, and +SQLite [cannot toggle foreign-key enforcement inside that transaction](https://www.sqlite.org/foreignkeys.html#fk_enable). +We retain the normal migration runner and keep foreign keys enabled throughout. + +## Proof and consequences + +`tests/enrollment_event_schema.rs` owns direct storage and upgrade proof: + +- all four event kinds against all four null/non-null reference combinations, + on both fresh and upgraded schemas; +- an installation migrated only through 0013 reproduces each of the six + one-reference loopholes, then refuses 0014 without changing event or phase + rows, schema objects, or the applied-migration version; +- repeated failure leaves no temporary guard behind, and the real startup + path fails closed on malformed legacy history; +- a valid upgrade preserves event IDs, phase epoch references, enrollment + pins, existing tables/indexes/triggers, and fixed-instant packet bytes; +- foreign-key and integrity checks pass, prior write restrictions still + refuse invalid operations, and rerunning the migrator is idempotent. + +The upgrade scans the event stream until it finds an invalid row or reaches +the end. This is validation cost, not a table rewrite. The predicate appears +in the historical CHECK, migration guard, insert trigger, and packet verifier; +the schema matrix and packet tests keep their intended contract aligned. diff --git a/docs/development.md b/docs/development.md index 6ed5e46..adea896 100644 --- a/docs/development.md +++ b/docs/development.md @@ -17,7 +17,7 @@ tests show what is implemented. [Roadmap](roadmap.md) owns milestone status. | Setup, login, recovery | `setup.rs`, `users.rs`, `sessions.rs`, `secrets.rs` | [ADR 0004](decisions/0004-local-authentication.md) | | Capabilities and assignments | `capabilities.rs`, `assignments.rs`, `draft_access.rs` | [ADR 0010](decisions/0010-service-owned-authorization-boundary.md), [Domain model](domain-model.md) | | Program configuration | `programs.rs`, `program_export.rs` | [ADR 0007](decisions/0007-program-version-configuration-model.md), [Program format](formats/program-version-export.md) | -| Enrollment and training sessions | `enrollments.rs`, `lifecycle.rs`, `training_sessions.rs`, `session_membership.rs`, `session_time.rs` | [ADR 0008](decisions/0008-session-draft-and-attribution-model.md), [ADR 0009](decisions/0009-session-local-time-resolution.md) | +| Enrollment and training sessions | `enrollments.rs`, `lifecycle.rs`, `training_sessions.rs`, `session_membership.rs`, `session_time.rs` | [ADR 0008](decisions/0008-session-draft-and-attribution-model.md), [ADR 0009](decisions/0009-session-local-time-resolution.md), [ADR 0018](decisions/0018-enrollment-event-reference-shape.md) | | Drafts and review | `evaluation_drafts.rs`, `draft_content.rs`, `draft_review.rs` | [ADR 0008](decisions/0008-session-draft-and-attribution-model.md), [ADR 0010](decisions/0010-service-owned-authorization-boundary.md) | | Finalization and canonical bytes | `finalization.rs`, `canonical.rs`, `record_envelope.rs` | [Integrity](records-integrity.md), [ADR 0011](decisions/0011-canonical-record-format-and-finalization.md) | | Acknowledgments and amendments | `acknowledgments.rs`, `amendments.rs` | [Domain model](domain-model.md), [ADR 0012](decisions/0012-amendment-reopening-state-machine.md) | @@ -37,6 +37,10 @@ Packet membership and timeline verification tests live in `tests/trainee_packet/pin_history.rs`; the parent packet test module owns shared fixtures and archive-editing helpers. +`tests/enrollment_event_schema.rs` covers fresh and upgraded lifecycle-event +storage, retained-history preservation, and fail-closed migration of malformed +legacy rows. ADR 0018 explains the migration 0014 diagnostic and repair boundary. + ## Runtime flow ```text