From fec4eaeb1b0e58d932943d340a0c926eb9da9192 Mon Sep 17 00:00:00 2001 From: tegwick Date: Wed, 9 Sep 2026 14:38:21 +0200 Subject: [PATCH] Hold the pinned image against the repository's schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Schema v4 gave the deployment a silent drift surface: the pinned pair is self-consistent, migrating to 3 and serving 3, while this repository has moved to 4. That reads as healthy, which makes it worse than an error — the failure is the assumption that the deployment records approver principal type. Document v4 in storage-operations, state in the deploy runbook that the pin predates it, and add a test that requires the statement whenever the release record's schema version differs from this repository's. Verified to fail when the acknowledgement is removed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HyybaE7DUXrWYrhbnESCTe Assistant: claude-code Assistant-Model: opus Assistant-Process: 1275879@bnt-lap001 Assistant-Session: eb464208-f821-41b2-bc5a-a6c33d92a8ad --- deploy/README.md | 11 +++++ docs/storage-operations.md | 18 ++++++++ tests/test_deploy_manifest.py | 43 +++++++++++++++++++ ...duction-readiness-and-consumer-adoption.md | 16 +++++++ 4 files changed, 88 insertions(+) diff --git a/deploy/README.md b/deploy/README.md index 3e535e9..12a3d17 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -13,6 +13,17 @@ pinned at `b51d174`. There is no placeholder left to replace. Both references MUST stay identical and MUST stay digests — a tag here would let the init container and the server run different code against one database. +**The pinned image predates schema v4.** The published artifact recorded in +[`docs/image-scan-2026-09-06.md`](../docs/image-scan-2026-09-06.md) carries +`LATEST_SCHEMA_VERSION = 3`; this repository is now at 4 +(`entries.principal_type`, see [storage-operations.md](../docs/storage-operations.md)). +The pinned pair is self-consistent — that image migrates to 3 and serves 3 — so +nothing is broken by leaving it pinned, but a rollout that must carry approver +principal-type evidence requires cutting a new image at step 3 below. The +`migrate` init container then performs the additive upgrade on the existing +volume; `tests/test_deploy_manifest.py` holds this acknowledgement so the drift +cannot go quiet. + Gates 1 and 2 below are the outstanding ones; nothing is deployed today. Before applying: diff --git a/docs/storage-operations.md b/docs/storage-operations.md index 775ed41..d3de405 100644 --- a/docs/storage-operations.md +++ b/docs/storage-operations.md @@ -42,3 +42,21 @@ PDP path, and inferring the declaration from an incidental digest would manufacture a statement nobody made. Such approvals stay usable by consumers in this engine's own vocabulary and are simply not usable on the PDP path — which is the ruling's intended cost, not a migration defect. + +## Schema v4 — recorded approver principal type + +`entries.principal_type` (TEXT, nullable) records what kind of principal bound +an approval, taken only from the verified token. Migration is the usual additive +`ALTER TABLE`; run `approval-engine migrate` before a production start, which +refuses an unmigrated store. + +Legacy entries stay `NULL` and are **not** back-filled — the same reasoning as +`pdp_path` in v3. An entry written before this column existed carries no +verified statement about the principal that made it, and reading `user:`-shaped +`subject_id` values as `human` would manufacture approver evidence nobody +presented. A `NULL` here means *unclassified*, never *human*. + +Downgrade is not supported: an older server refuses the store on the version +check rather than reading the column, which is the intended direction. Restore +from a v4 backup onto a v3 release requires re-pinning forward, not editing +`user_version`. diff --git a/tests/test_deploy_manifest.py b/tests/test_deploy_manifest.py index f4f2300..a8c7982 100644 --- a/tests/test_deploy_manifest.py +++ b/tests/test_deploy_manifest.py @@ -11,8 +11,12 @@ from __future__ import annotations import re from pathlib import Path +from approval_engine.store import LATEST_SCHEMA_VERSION + ROOT = Path(__file__).resolve().parents[1] MANIFEST = ROOT / "deploy" / "approval-engine.yaml" +RELEASE_RECORD = ROOT / "docs" / "image-scan-2026-09-06.md" +DEPLOY_README = ROOT / "deploy" / "README.md" IMAGE_LINE = re.compile(r"^\s*image:\s*(\S+)\s*$", re.MULTILINE) DIGEST_PIN = re.compile(r"^[^\s@]+@sha256:[0-9a-f]{64}$") @@ -67,3 +71,42 @@ def test_deploy_readme_does_not_instruct_replacing_a_pinned_placeholder(): readme = (ROOT / "deploy" / "README.md").read_text(encoding="utf-8") if "REPLACE_WITH_RELEASE_DIGEST" not in manifest: assert "replace `REPLACE_WITH_RELEASE_DIGEST`" not in readme + + +def test_pinned_image_schema_drift_is_acknowledged(): + """The pin can silently fall behind the repository's schema. + + The checks above hold that migrate and serve run the *same* code. They + cannot see that both run *old* code: a v3 image migrating to 3 and serving + 3 is perfectly self-consistent while this repository has moved to 4. That + reads as healthy and is the more dangerous shape, because the failure is an + assumption ("the deployment records principal_type") rather than an error. + + So when the manifest still pins the artifact the release record describes, + the record's schema version must either match this repository or the + runbook must say plainly that the pin is behind. Prose alone loses that; + the test makes stating it the cheaper option. + """ + refs = image_refs() + assert refs, "no image references found" + pinned = refs[0].split("@", 1)[-1] + + record = RELEASE_RECORD.read_text(encoding="utf-8") + if pinned not in record: + return # a newer artifact is pinned; this record no longer describes it + + versions = { + int(v) for v in re.findall(r"LATEST_SCHEMA_VERSION\s*=\s*(\d+)", record) + } + assert len(versions) == 1, f"release record states {versions or 'no'} schema versions" + recorded = versions.pop() + if recorded == LATEST_SCHEMA_VERSION: + return + + readme = DEPLOY_README.read_text(encoding="utf-8") + assert f"predates schema v{LATEST_SCHEMA_VERSION}" in readme, ( + f"pinned image records schema v{recorded} while this repository is at " + f"v{LATEST_SCHEMA_VERSION}; deploy/README.md must say so explicitly " + f"(the phrase 'predates schema v{LATEST_SCHEMA_VERSION}') or the " + "manifest must be re-pinned to a newer artifact" + ) diff --git a/workplans/APPROVAL-WP-0002-production-readiness-and-consumer-adoption.md b/workplans/APPROVAL-WP-0002-production-readiness-and-consumer-adoption.md index f7127d5..6d29310 100644 --- a/workplans/APPROVAL-WP-0002-production-readiness-and-consumer-adoption.md +++ b/workplans/APPROVAL-WP-0002-production-readiness-and-consumer-adoption.md @@ -351,6 +351,22 @@ delivery, so a rollout attempted today would fail closed by design and would prove nothing. **T03 stays `wait`** — on T01's registrations and the audit credential, not on anything this repo owns. +2026-09-09: schema v4 (`entries.principal_type`, T01) gave this task a new +drift surface, now closed in the only way that survives review. The manifest's +pinned pair is self-consistent — the published artifact migrates to 3 and serves +3 — so nothing fails, and that is exactly the dangerous shape: the deployment +reads healthy while the assumption "the deployment records approver principal +type" is false. `deploy/README.md` now states that the pin predates schema v4 +and that carrying the new evidence requires cutting a new image at gate 3, and +`tests/test_deploy_manifest.py::test_pinned_image_schema_drift_is_acknowledged` +holds the statement: while the manifest pins the artifact +`docs/image-scan-2026-09-06.md` describes, that record's schema version must +match this repository or the runbook must say plainly that it does not. Verified +to fail when the acknowledgement is removed. `docs/storage-operations.md` gains +the v4 section (additive `ALTER TABLE`, legacy entries stay `NULL` and are never +back-filled to `human`, downgrade unsupported). T03 stays `wait`: gates 1 and 2 +are unchanged and nothing is deployed. 125 tests pass. + ## Wire outbox delivery and reconciliation ```task