feat(retention): ASVS 14.2.7 — retention classification, automatic deletion, and the legacy outbox PHI leak - #71
Conversation
…hip half-done
Commit 1 of ASVS 14.2.7. No production code. Three guards that run on the plain
leg with NO database, deliberately landed BEFORE the changes they police.
TWO independent nine-agent workflows analysed this change (two Claude processes
under one session id, neither aware of the other). They produced DIFFERENT plans
-- 12 traps and 13 traps, 20 distinct. Both are preserved with their union in
vault PR #1258. Two of the three guards here exist because of traps only ONE pass
found, which is the argument for having run both.
--- 1. tests/test_sqlserver_encrypt_pass_tables.py (AST, no DB) ---------------
Every (table, column) literal driving a ciphered-cell sweep must name a table the
module actually CREATEs.
This is the ONLY mechanism in the repo that can catch the defect. Commit 6 drops
the legacy `outbox` table; the pair ("outbox","payload") sits ~1200 lines away in
_encrypt_existing_rows. That method early-returns on `if not self._cipher.encrypts`
-- and EVERY SQL Server CI step runs KEYLESS, so the SELECT below that return
never executes on any leg. Plan A's own stated mutation proof for this trap
("the keyed open must red") reds on no CI leg that exists. A keyed production
open would crash-loop on Invalid object name; CI would stay green.
The naive version over-matched on its first run and that was useful: both sweeps
also build composite AAD tuples like ("body","detail") and ("event_type",
"connection") whose first element is a COLUMN. Flagging those would have cried
wolf on ~14 legitimate pairs, and a guard that always fails gets xfailed or
deleted. So it anchors on the LOOP (`for table, <col> in (...)`), not on "any
2-string tuple in the function".
Four mutations against the REAL source, not the fixture:
M1 unknown table in _encrypt_existing_rows -> RED
M2 unknown table in reencrypt_to_active -> RED (both sweeps walked)
M3 ("outbox","payload") REMOVED -> GREEN (must not block Commit 6)
M4 CREATE TABLE regex blinded -> RED (liveness receipt fires
rather than passing vacuously)
M3 and M4 matter as much as M1: a guard that blocks its own fix, or that passes
when blinded, is worse than none.
--- 2. tests/test_fixture_outbox_reset.py (AST, no DB) ------------------------
No fixture may issue an UNGUARDED `DELETE FROM outbox` once the table is retired.
The existence-guarded form
f"IF OBJECT_ID(N'{table}', N'U') IS NOT NULL DELETE FROM {table}"
is a no-op afterwards and is accepted; the bare form is not. That distinction is
structural, so the scanner reads the loop BODY rather than grepping for the word.
MEASURED, independently reproducing plan B's trap 8: 538 test files scanned, 21
unguarded reset sites across 18 files, and NINE of those 18 run in no SQL Server
CI step at all -- test_adr0071_dispatch_wiring_sqlserver, ..._fused_callables_...,
test_adr0075_batch_sqlserver, test_adr0114_claim_proc_live, test_batch_completion,
test_metadata_bag, test_outbound_batch, test_shard_recovery_sqlserver,
test_sqlserver_sync_handoff. A mutation planted in any of those passes VACUOUSLY,
so "the SS leg is green" was never evidence the reset lists were updated.
Marked xfail(strict=True), not left red: the 21 sites are CORRECT today (the
table exists). strict makes it SELF-CLEARING -- the moment the last reset list is
fixed the test XPASSes, which strict turns into a failure, forcing the marker out
in the same commit. A non-strict xfail would rot silently green forever.
--- 3. _RETIRED_CLAIMS registrations (tests/test_phi_at_rest_inventory.py) ----
Three strings that are TRUE TODAY and become false in Commits 3 and 6, registered
FIRST so the guard reds against the UNEDITED docs. Confirmed RED at registration:
docs/PHI.md:123 '**No purge path at all**'
docs/PHI.md:401 'is purged by nothing on any backend'
docs/PHI.md:1050 'touched by no purge on any backend'
That output is the evidence, and it is only obtainable BEFORE the edits. A
registration made afterwards passes on day one whether or not the guard works.
A FOURTH candidate was STRUCK, and this is the trap only plan B found. The
obvious string "no retention purge" collides with a still-TRUE sentence at
docs/security/ASVS-292-289-HANDOFF-2026-07-25.md:173 about
pending_approvals.params. _retired_claim_hits() globs docs/*.md PLUS
docs/security/*.md -- and docs/security/ is GIT-IGNORED. Registering it would red
on a laptop and pass in CI, where the file does not exist. A guard strictly
weaker in CI than locally is not a guard. Plan A instructed adding it, with no
warning; I was minutes from doing so.
Grep evidence recorded in-file (docs/*.md | docs/security/*.md):
"no retention purge" 1 | 1 <- STRUCK
"**No purge path at all**" 1 | 0
"touched by no purge on any backend" 1 | 0
"is purged by nothing on any backend" 1 | 0
DELIBERATELY RED IN THIS COMMIT: test_retired_false_claims_do_not_reappear fails
until Commit 3 edits those three PHI.md lines. That is the forcing function, and
CI gates the PR head, not each commit.
`reference.value` (ADR 0006 versioned lookup snapshots) is PL-2 in PHI.md §2 and
can hold patient-keyed rows. It had NO purge path at all: the only thing that
ever replaced a snapshot was the next sync's build-new-then-flip, which never
comes for a set nobody declares any more. `grep purge_reference` returned zero
production hits.
Adds purge_reference_snapshots to the Store protocol and all three backends, the
[retention].reference_snapshot_days window, and the runner phase.
ORPHAN-SCOPED, and the limit is stated rather than glossed: a set that IS still
declared is never touched however old its synced_at, because its snapshot is live
data the engine serves. So the normal case -- a wired set holding live PHI -- is
still purged by nothing. That is an honest residual, not a closed cell, and the
classification table must not describe this window as covering reference.value
generally (which would machine-bless a false claim).
Three defences, each from an adversarial pass and each mutation-proven:
1. EMPTY `declared` RAISES, in all three stores AND at the runner call site.
`declared` is the keep-set, so an empty one reads as "every set is abandoned"
and would purge the whole store. `registry is None` does not cover it: a
registry that LOADS FINE while declaring zero reference sets -- a subset
--config, a per-team split, a harness redirect aimed at the real DB -- yields
references == {}. Absence-based guards fail open, so this is positive-signal.
Belt AND braces because one refactor dropping the runner check costs every
snapshot in the store. Worse without it: ReferenceSyncRunner deliberately does
NOT advance synced_at when a source fetch fails, so the rows that look most
stale belong to a still-wired set whose source is merely DOWN.
2. ELIGIBILITY RE-ASSERTED INSIDE THE DELETE, on all three backends including
SQLite. One analysis said SQLite "happens to be safe" via self._lock; its own
adjudication corrected that and is right -- `declared` is computed in the
RUNNER, outside any store lock, so the config-reload race is at the caller
level everywhere. Do not simplify the EXISTS away because the SELECT already
filtered.
3. THE POINTER SURVIVES BUT ITS VERSION IS BUMPED. Both analyses proposed bolting
a removal arm onto converge_reference_cache in each server backend. Reading it
gives a better fix: converge only reloads a set whose active version DIFFERS
from the one a handle reflects. So leaving the version alone means a follower's
populated cache is never revisited -- it serves purged PHI from RAM until
restart -- and deleting the pointer is WORSE, because converge only ever
adds/updates names present in a fresh read and would never notice. Bumping to
'purged:<version>' with row_count=0 routes the deletion through the mechanism
that already exists, on both server backends, with no second removal path to
keep in sync. Idempotent via `WHERE version NOT LIKE 'purged:%'`.
Four mutations killed:
M1 drop the `not in declared` filter -> a DECLARED set loses its rows
M2 empty-declared raise -> return 0 -> the wipe path becomes a silent no-op
M3 drop the version bump -> followers never converge
M4 drop the EXISTS re-assert -> a concurrent re-sync is destroyed
M4 SURVIVED THE FIRST ATTEMPT, and the reason is worth recording because it is
the exact defect class this change set exists to prevent. The original test
re-synced BEFORE calling the purge, so the internal SELECT already filtered the
set out and the DELETE never ran -- it exercised the candidate filter, not the
re-assert, and passed whether or not the control worked. The race only exists
BETWEEN the SELECT and the DELETE, so the reload is now injected mid-method by
wrapping _db.execute, with `assert fired` so a wrapper that stops matching can
never let the test pass vacuously.
SQLite keeps no per-set version map (single-node, sole writer, converge is a
genuine no-op there) so it evicts its read-through cache instead; the server
backends evict AFTER the commit, so a rolled-back purge cannot leave the cache
claiming rows the store still holds.
Expected reds until the docs commit: test_retired_false_claims_do_not_reappear
and test_purge_surface_is_defined_on_every_backend_and_documented_per_backend.
The second is NEWLY ARMED by this commit -- adding the method to the protocol
makes the tree demand its PHI.md §8 row, which is the forcing function working.
ruff + mypy(strict, 257 modules) clean; 219 passed across the touched suites.
…is stated (14.2.7)
Clears two of the four forcing functions landed in Commits 1-2.
`test_purge_surface_is_defined_on_every_backend_and_documented_per_backend` was
armed by Commit 2 the moment purge_reference_snapshots joined the Store protocol:
the tree DEMANDED a §8 row rather than trusting anyone to remember one. That is
the guards-first ordering doing its job. Added, naming the mechanism, the
re-assert, and the pointer-survives-with-a-bumped-version behaviour that is what
actually makes a cluster follower converge.
Also retires the registered false claim at §2's reference.name/version/key row
("**No purge path at all**") and rewrites §2's reference.value row.
THE ORPHAN-ONLY LIMIT IS WRITTEN INTO EVERY PLACE THE WINDOW IS NAMED, on
purpose. purge_reference_snapshots bounds a set config has DROPPED; a set still
declared is never purged whatever its age, because its snapshot is live data the
engine serves. So the normal case -- a wired set holding live PHI -- remains
bounded by nothing. An adversarial pass flagged that describing this as a plain
`rides <window>` entry would machine-bless a FALSE claim and, worse, make the
true statement unwritable once the retired-claim string was registered. So §8's
honest-gaps table KEEPS a reference.value row rather than losing it: the row now
scopes the gap to the declared-set case instead of deleting it.
Two registered claims deliberately survive this commit, both about
`outbox.payload` (docs/PHI.md:401 and :1051). They are Commit 6's to clear when
the legacy SQL Server table is migrated and dropped. Registering all three in
Commit 1 -- before any edit -- is what made the guard's RED output the work list;
the count going 3 -> 2 here is that mechanism reporting progress.
…tention-classification
…tention-classification
… from the CLI
Two more corrections from the sibling session's hostile review, both verified
here before acting. The first sharpens a rewrite I made an hour ago and did not
push far enough.
1. I WROTE 'only an out-of-band audit_anchor detects that' -- which implies an
operator can reach it. They cannot. Verified:
__main__.py:3418 return await store.verify_audit_chain() <- no anchor
parser zero --expect-anchor / audit-anchor flags
audit_anchor() implemented on all three backends; its ONLY callers are
pipeline/dr.py:569,585 and rekey_audit_chain
So the machinery exists and the shipped operator command cannot use it: an
operator or compliance job running OK: verified 0 audit row(s) gets a CLEAN
result after someone truncates the newest audit rows. As shipped, truncation
detection depends on the off-box tee (sec-offbox-log #361/#363), not on
anything runnable locally.
My previous wording was true about the mechanism and misleading about the
posture -- the same shape as the inverted argument it was correcting. Now
states the gap plainly, with an explicit instruction not to upgrade it to 'the
anchor detects it' without also shipping a way to pass one.
2. §5 AND §7 CONTRADICTED EACH OTHER, and §7 was right. §5 claimed 'no PHI is
placed in a URL (so it cannot leak into history, bookmarks, or Referer)' while
§7 documented, correctly, that '?content=1234567 or ?field_value=MRN12345
survives the whole filter chain unredacted'. The console's own search route
takes both as query parameters. Narrowed §5 to what actually holds: no message
BODY in a URL, plus Referrer-Policy: no-referrer.
Also flagged in the same bullet: messagefoundry_webconsole/_security.py
justifies degrading to no-referrer on the grounds that '/ui URLs carry opaque
ids only (never PHI)'. The header is still the right call, but its stated
REASON is false -- and a compensating control resting on a false premise is
worse than an uncommented one, because the next person reasons from the
comment. The route fix is filed separately by the sibling session; the comment
would have outlived it.
Third instance today of two live documents disagreeing while the stale/wrong half
is the one a reviewer picks up (after harden_kex_groups across five documents, and
the pre-#301 Transit claim). The pattern is worth naming: a repo that states a
fact twice will eventually state it two ways, and the wrong copy is the one that
gets cited.
…NDED (14.2.7)
Adds §2's seventh column: a retention verdict for EVERY at-rest tier, in a
七-form vocabulary the serve gate's tier list and the §2/§8 drift test are
GENERATED from. A wider hand-typed tuple is the same defect with a longer
literal; this is the artifact that replaces it.
Produced by a nine-agent workflow (classify -> adversarially attack -> reconcile)
because a wrong cell here does not fail a test, it gets MACHINE-BLESSED: a false
'rides <window>' asserts coverage that lives somewhere else, and the drift test
would then defend it on every run. This project has retracted an ASVS score twice
for that shape.
DISTRIBUTION: 7 own-window, 13 rides, 2 orphan-only, 5 keep-forever, 6 not-PHI,
1 keep-N, and 4 UNBOUNDED. The F7 count being non-zero was a stated pass
condition -- a classification of this codebase with no gaps in it would be
flattering rather than complete.
TWO UNBOUNDED PL-1 TIERS WERE IN NO GAP TABLE ANYWHERE, and both are now in §8:
* at stage ingress/routed, when DEAD. This is a REAL DEFECT, not
a documentation gap, and it is filed separately. dead_letter_now issues
and never
touches ; a router/handler content fault calls it on the claimed
ingress/routed row (wiring_runner.py:4415, :4449); and BOTH purges are scoped
Stage.OUTBOUND (store.py:8384, :8659). So blanks on its window
and the message READS AS PURGED while a full raw PHI body survives here
indefinitely. It was missed because these rows are documented as transient --
true on the happy path, and the dead-letter path is the exception nobody
classified.
* the backup staging dirs. A TemporaryDirectory unlinks on exit but not on a
crash or SIGKILL, and verify_after_backup (default true) decrypts a full
archive back out on EVERY run.
ONE ATTACKER OVERTURNED A CLASSIFIER, and the correction matters beyond
bookkeeping: at stage outbound was filed as riding the body
window, but purge_dead_letters is a purge dedicated to that exact table+column on
its OWN window. Filing it F2 would have dropped --
one of exactly two windows __main__.py enumerates for the auto-bound and the
refusal -- out of a list generated from this column.
MY OWN GUARD CAUGHT MY OWN WORDING while writing this: the new staging-dir row
said 'point TMP/TMPDIR at an owner-only path', which
test_owner_only_file_acl_is_always_qualified_to_the_sqlite_store correctly flagged
as an unqualified ACL claim -- the engine applies no file ACL on a server-DB
store, the deployed posture. Re-scoped to state the engine's no-ACL plainly and
put the cover where it belongs, on the operator.
WORKFLOW DEFECT WORTH RECORDING: was passed as a JSON array and arrived as
a STRING, so the four intended row-batches sliced CHARACTERS. The agents recovered
by reading the file themselves -- 68 classifications came back for 38 rows -- and
the reconciler deduplicated. It worked by redundancy, not by design. Completeness
was therefore verified independently (1..38 exactly once, no duplicates, every row
7 cells) rather than taken from the reconciler's own 'checked' claim.
|
{"body":"## Commit 6 (SQL Server legacy- |
… a literal (14.2.7) The startup retention gate checked a two-element literal. That literal is why this cell broke once: a new PHI tier landed and nobody widened it. Widening the literal is the same defect with more characters, so the tier list is now GENERATED from config/retention_classification.py, and tests/test_retention_classification_drift.py holds that constant equal -- in BOTH directions -- to docs/PHI.md 2's Retention column. Add a tier to the doc without adding it here, or here without the doc, and the suite reds. Nine windows across FIVE settings sections, not one. The first drift test checked every window against RetentionSettings and exempted [store]/[security] by prefix; adding [backup].retention_keep reded it and the tempting fix was to widen the exemption, which would have stopped verifying three of the five sections. Routing each window to its own model instead makes the check stronger than it was before it failed. MIN_PHI_RETENTION_WINDOWS is a FLOOR, not an emptiness check: `if not TUPLE` passes for a one-element tuple, so a bad merge would leave the gate checking one window while reporting success. The floor was itself corrected by its own drift test -- written as 7 by hand, the two-way equality immediately reported [backup].retention_keep and [retention].connection_event_retention_hours as documented-but-absent, before either had ever run. Behaviour change, deliberate and owner-approved (2026-07-30): the three PHI-BODY windows now DEFAULT to 30 days when UNSET, on both dials. Previously an unset window on the shipped `enforce` posture REFUSED TO START, which forced an operator to pick a number; it now starts bounded at 30. What survives is the fail-closed path for an EXPLICIT 0 -- typing 0 is a deliberate act and is still refused unless [security].allow_keeping_phi_indefinitely is set. So "unbounded by accident" is still prevented; "unbounded by inattention" becomes "30 days by inattention". test_cli.py carries both halves as a pair. state_max_age_days / search_preset_days / app_log_days / backup.retention_keep are classified and WARNED, never silently bounded -- also a ruling, not an omission. Their timestamps only move on a WRITE (state.set_at is never refreshed by state_get), so an auto-bound would delete a Handler's live MRN->surrogate crosswalk mid-stream, and the next message would transform WRONGLY, post-ACK, with no ERROR disposition. connection_event_retention_hours and uploads_retention_days are classified but never tested for unboundedness: on the first, 0 means INHERIT the body window, so a `days <= 0` predicate would refuse a start over a window that is doing its job; the second carries a ge=1 floor, so the clause is unfireable by construction. Both are recorded as zero_is_unbounded=False rather than dropped, so the generated list still matches 2.
…ld purge (14.2.7)
`outbox` was recreated by `_SCHEMA` on every open and read by NOTHING -- the staged pipeline
and every delivery-side method use `queue`. So on a store upgraded from the
pre-staged-pipeline layout, `outbox.payload` (a full transformed PHI body) was reached by no
purge on any backend: `purge_old_messages` and `purge_dead_letters` both scope to `queue`.
Meanwhile `messages.raw` blanked on its own window, so the message READ as purged while the
body survived indefinitely. It also sat outside `reencrypt_to_active`, so a rotation that
retired the old key left those bodies undecryptable.
Both close the same way: a guarded `_SCHEMA` statement folds surviving rows into `queue` as
stage='outbound' -- payload carried over VERBATIM, so encryption at rest is preserved -- and
then DROPs the table. Migrated rows become ordinary outbound queue rows: bounded by
[security].delete_message_bodies_after_days / [retention].dead_letter_days, swept by the
on-open cipher migration, rotated by reencrypt_to_active. SQLite already did the equivalent
(`_migrate_outbox_to_queue`); Postgres never had the table. The three `outbox` DDL statements
are gone, so a fresh store never creates it again.
THREE PLACEMENT DECISIONS, each load-bearing rather than stylistic:
* It lives in `_SCHEMA`, not an open-path method like SQLite's. `_schema_hash()` is the ONLY
thing deciding whether the DDL batch runs -- a current marker skips it entirely -- so
migration code outside `_SCHEMA` would be invisible to that decision and would never fire on
an upgrade. It also inherits the batch's `raw.timeout = 0`, so a large legacy backlog cannot
be killed mid-INSERT into a rollback-and-re-fail crash-loop.
* It sits AFTER the `queue` DDL, not where the `outbox` DDL was. SQL Server defers name
resolution, so an earlier placement would parse fine and then fail at RUN time on a legacy DB
old enough to have `outbox` but not `queue`.
* Two filters guard the INSERT. EXISTS(messages) skips FK orphans -- unreplayable anyway, but
one would abort the batch with an opaque FK error. NOT EXISTS(queue) makes a PK collision a
no-op instead of a 2627 that rolls back and re-fails on every restart, and makes a
partially-applied migration safely re-runnable.
VERIFIED AGAINST A REAL SQL SERVER (2022 CU25 in Docker), not just asserted. The new live suite
simulates an UPGRADE rather than a fresh open, because a fresh open never executes the
migration at all (the existence guard is false) and a re-open with a current `schema_meta`
marker skips the whole batch -- both would have passed while measuring nothing. Each test
seeds a real legacy table and clears the marker, which is the state a genuine upgrade presents.
All three filters were then mutation-proved against the live database: dropping the DROP reds 3
tests, removing EXISTS(messages) reds the orphan test with an IntegrityError, removing
NOT EXISTS(queue) reds the collision test with an IntegrityError.
FIFO on a migrated backlog was MEASURED, not assumed: rows INSERTed into `outbox` in the exact
reverse of their created_at order came out with `seq` ascending by created_at. It is still not
a documented SQL Server guarantee, so the comment says so and no test asserts it -- a flaky
gate on a best-effort property is worse than none.
`("outbox", "payload")` is removed from `_encrypt_existing_rows`; leaving it would have failed
every KEYED open with *Invalid object name*. No CI leg would have caught that -- every SQL
Server leg runs keyless and returns before the sweep -- which is why the AST guard from the
previous commit exists. Mutation-proved: re-adding the entry reds it, naming the line.
27 `"outbox"` entries removed from reset lists across 24 files. Nine of those files run on NO
SQL Server CI step, so "the SS leg is green" would not have been evidence; the source scanner
is what covers them. Its `xfail(strict=True)` self-cleared on the last fix -- XPASS, which
strict reports as a failure -- so removing the marker here was forced rather than remembered.
The `test_no_engine_code_still_references_the_retired_table` sweep was rewritten mid-change
after it correctly fired on BOTH migrations. A flat ban is unsatisfiable (a migration must name
the table it migrates), but a plain file exemption would hide a stray read inside those same
files forever. It now pins the reference COUNT per migration module and asserts each still
contains the DROP, so a new `FROM outbox` in `store/sqlserver.py` reds on the count. Path keys
are POSIX-normalised: native separators would have keyed on backslashes and missed on ubuntu --
a guard red on one OS and green on another.
docs/PHI.md: the section 2 inventory row and the section 8 gap row are removed rather than
reworded, because the at-rest location no longer exists. The per-backend cipher counts drop to
18/17/17 in both places they are stated -- and those are DERIVED from live code by
test_phi_at_rest_inventory.py, which is what reported the change rather than my counting. The
two registered retired claims ("touched by no purge on any backend", "is purged by nothing on
any backend") now go green, which is what they were registered a commit early to prove.
The bare "no purge on any backend" is still NOT registered, and the reason CHANGED without
changing the answer: it used to collide with the legacy row, and now collides with this
commit's own past-tense account of what the retirement fixed. A phrase whose collision merely
MOVED is not a phrase that became safe to register.
CI WIRING, without which the live suite above would have been decorative. CI's SQL Server steps
run an explicit FILE LIST, not the whole suite, so a new MEFOR_TEST_SQLSERVER-gated file that
nobody appends runs in NO environment: it skips on the plain legs, and a skip reads exactly
like a pass. The new file is appended to the catch-all SQL Server step, and that step's comment
now says it is the catch-all for ANY gated file rather than only throughput levers.
Measuring that gap turned up a larger one, filed rather than fixed here: 17 test files are
wholly skipped without a live SQL Server, and ELEVEN of them are named in no CI step at all.
That is why several of the pre-existing failures below had rotted unnoticed -- nothing runs
them anywhere.
Unrelated pre-existing failures found and filed, NOT introduced here. A full local suite showed
29 failures; 25 were proven pre-existing by stashing this change out and re-running (the
identical set failed). Of those, a large share are LOCAL-ONLY artifacts of exporting the SQL
Server env vars across the WHOLE suite, which CI never does -- it sets them per step for a
named file list. MEFOR_ALLOW_INSECURE_TLS=1 in particular defeats refusals that
test_tls_trust_anchor and test_hop_refusal_rawtcp assert; test_audit_integrity.py goes 29/29
green on a clean env. Separately, 5 of 6 tests in test_adr0114_claim_proc_live.py fail against
a live SQL Server (claim_proc_effective is False; the deployed proc body does not match the
shipped definition, and the warning's own DELETE FROM schema_meta remedy does not fix it) --
also confirmed pre-existing the same way, and one of the eleven files that run nowhere.
…tention-classification
…an path origin/main moved docs/throughput-build-plan.md under docs/archive/throughput/ (#69). The rename updated docs/AOAG-DEPLOYMENT.md but not these three ci.yml comments, and nothing catches it: no guard scans workflow COMMENTS for doc paths, so the drift is invisible. Comment-only, no behaviour change. Fixed here rather than left because one of the three sits directly above the file list this branch appends to -- shipping a stale path in a block I just rewrote is my drift, not inherited drift.
wshallwshall
changed the title
Commit 6 landed — the legacy SQL Server
|
| Mutation | Result |
|---|---|
Delete DROP TABLE outbox |
3 tests red |
Remove EXISTS (SELECT 1 FROM messages ...) |
orphan test red — IntegrityError |
Remove NOT EXISTS (SELECT 1 FROM queue ...) |
collision test red — IntegrityError |
FIFO on a migrated backlog was measured: rows inserted in the exact reverse of their created_at order came out with seq ascending by created_at. It is still not a documented SQL Server guarantee, so the comment says so and no test asserts it — a flaky gate on a best-effort property is worse than none.
CI wiring — without it the live suite would have been decorative
CI's SQL Server steps run an explicit file list, not the whole suite. A new MEFOR_TEST_SQLSERVER-gated file that nobody appends runs in no environment: it skips on the plain legs, and a skip reads exactly like a pass. The new file is appended to the catch-all step, whose comment now says it is the catch-all for any gated file rather than only throughput levers.
Measuring that gap turned up a larger one, filed rather than fixed here: 17 test files are wholly skipped without a live SQL Server, and eleven are named in no CI step at all.
Failure triage — 0 caused by this change
A full local suite showed 29 failures. Accounting closes completely:
- 26 consistent — proven pre-existing by stashing this change out and re-running: the identical set failed. Two causes: (a) exporting the SQL Server env vars across the whole suite, which CI never does —
MEFOR_ALLOW_INSECURE_TLS=1defeats refusals thattest_tls_trust_anchorandtest_hop_refusal_rawtcpassert, andtest_audit_integrity.pygoes 29/29 green on a clean env; (b) files among the eleven that run nowhere and have rotted. - 4 varying — self-inflicted: I edited
docs/PHI.mdand ran a second pytest concurrently while that suite was in flight. They pass in every run on a frozen tree.
Separately filed: 5 of 6 tests in tests/test_adr0114_claim_proc_live.py fail against a live SQL Server (claim_proc_effective is False; the deployed proc body does not match the shipped definition, and the warning's own DELETE FROM schema_meta remedy does not fix it). Also confirmed pre-existing the same way, and one of the eleven files that run nowhere — so the engine has been silently degrading off the ADR 0114 proc path with nothing to catch it.
Note for review
docs/PHI.md §2 loses a row (38 → 37) and §8 loses a gap row, because the at-rest location no longer exists. The stated per-backend cipher counts drop to 18/17/17 in both places — and those are derived from live code by test_phi_at_rest_inventory.py, which is what reported the change rather than any hand count.
This branch needs main integrated before it can merge (strict is on).
Handoff — ASVS 14.2.7 packet + what it uncovered (2026-07-30, ~14:45)Session Written because the account hit 94% weekly usage. The work is finished and pushed — this is a 1. State at handoff — nothing is half-donePR #71 is pushed, marked ready, retitled, and armed for auto-merge (squash).
The one thing to watch. Do not hand-merge past a red check; Commits on the branch
Verification actually performed (not "should be fine")
2. What the change actually fixed (so a reviewer does not under-rate it)
Three placement decisions in
3. Follow-ups — two already have live sessionsBoth were spawned as background tasks by the owner at ~14:30 and are running independently.
⚠ Hazard to relay to both, if they are still running. They share the single 4. Local-red / CI-green — verified, do not "fix" it
The whole module skips when Worth someone's judgment later, not mine to decide: these ASVS 12.1.1 attestation guards run on 5. Method notes that cost me real time today
6. Not done, deliberately
|
PR #80 appended eight Steps-view items to docs/BACKLOG.md without the status banner every numbered item must carry, so tests/test_backlog_status_check.py::test_the_real_backlog_satisfies_the_invariant began failing on main itself (8 errors, first at line 6905). Because GitHub tests each PR merged into main, every open PR inherited the failure: #81, #74, #71, #66 and #60 were all blocked, three of them with auto-merge armed and unable to fire. Adds exactly one leading banner per item. Seven use the open/prioritized form; #239 uses the partial form, because its measurement ran and is recorded on PR #81 while the re-runnable scripts/quality/lens_coverage.py is still unmerged -- the number is not yet reproducible from main. The banners are deliberately minimal and do not re-litigate any item's verdict. #234 in particular is left explicitly unsettled rather than entrenched: it was filed as "revisit, not a bug" after the owner asked for a fix, and that framing is still open. Verified: scripts/docs/backlog_status_check.py exits 0 (237 items) and tests/test_backlog_status_check.py is 15 passed, was 14 passed 1 failed. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Draft — commits landing incrementally. Opened early so CI runs alongside the work rather than after it.
Provenance
Two independent nine-agent workflows analysed this change (two Claude processes under one session ID, neither aware of the other). They produced different plans — 12 traps and 13 traps, 20 distinct. Both plus their union are preserved in vault PR #1258. Several guards here exist because of traps only one pass found.
Commit 1 — no-DB guards, landed first (
5e2e1a32)Guards before the changes they police, so the retirement cannot ship half-done.
test_sqlserver_encrypt_pass_tables.py— AST-walkssqlserver.py; every(table, column)literal driving a ciphered-cell sweep must name a table the module CREATEs. This is the only mechanism that can catch a stale("outbox","payload")after the DDL removal:_encrypt_existing_rowsearly-returns on a keyless handle and every SQL Server CI step is keyless, so that code never executes in CI. Four mutations proven, including outbox pair removed → green (must not block the Commit 6 fix) and detector blinded → red (liveness receipt fires rather than passing vacuously).test_fixture_outbox_reset.py— measured 21 unguardedDELETE FROM outboxreset sites across 18 files, 9 of which run on no SQL Server CI step. A mutation planted in any of those nine passes vacuously.xfail(strict=True)so it is self-clearing: the moment the last list is fixed it XPASSes, which strict turns into a failure, forcing the marker out._RETIRED_CLAIMSregistrations, confirmed RED against the unedited docs — namingdocs/PHI.mdlines 123, 401, 1050 as the edits owed. A fourth candidate ("no retention purge") was struck: it collides with a still-true sentence in a git-ignored file, so it would red locally and pass in CI.Commit 2 —
purge_reference_snapshots(in progress)reference.valueis PL-2 and had no purge path at all. Protocol + all three backends done;ruffandmypy --strictclean.declaredraises, in the stores as well as the runner — an empty collection reads as "every set is abandoned" and would wipe the store; there is no legitimate caller, so it is an error, not a no-op.declaredis computed in the runner outside any store lock, so the config-reload race is at the caller level everywhere.converge_reference_cacheonly reloads a set whose version differs, so an unchanged pointer leaves followers serving purged PHI from RAM, and deleting the pointer is worse (converge only adds names present in a fresh read). The purge bumps the version, routing the deletion through the convergence path that already exists.Expected reds until Commit 3
test_retired_false_claims_do_not_reappearandtest_purge_surface_is_defined_on_every_backend_and_documented_per_backend— both are forcing functions demanding the PHI.md §2/§8 edits, and both clear in the docs commit.