backlog(#347): a PHI-at-rest assertion that can pass for the wrong reason - #147
Conversation
…st base64 ciphertext `tests/test_store_encryption.py:95` asserts `"DOE" not in raw` against a value encrypted under a fresh random key, so the base64 body is fresh random text every run. Measured p ~ 5e-4 per run per leg; it fired on PR #142's windows-2022 py3.14 leg with encryption working correctly. Filed rather than fixed: the fix direction is the maintainer's call (decoded-bytes assertion vs. non-recoverability vs. full-plaintext absence), and simply widening or deleting the substring check would drop the PHI-at-rest property it reaches for. Includes the sibling audit: the identical 3-char shape survives at :303, a 4-char instance at test_content_search.py:123, and ~13 >=6-char instances whose rate is immaterial but whose shape is the same. `test_off_by_default_stores_plaintext` does NOT share the shape (deterministic equality) and needs no change.
…per-CI-run figure Reframed after the session that owns PR #142 reproduced the diagnosis independently. Three substantive corrections, none of them cosmetic: 1. FRAMING. The banner led with the flake; it now leads with what is actually wrong. The substring check fails when encryption worked AND would pass on a weak encoding that happened to avoid those three characters. The second half is the PHI defect; the flake is only what made someone look. 2. RATE. 5.5e-4 is per assertion per leg. There are two such assertions and three OS legs (ubuntu + windows-2022 + windows-2025, verified against ci.yml), so the figure an operator experiences is 1 in 303 full CI runs, not 1 in 1,820. Both over-estimate caveats kept: L is from one measured value, and the hex-fingerprint prefix is not base64. 3. SCOPE. Reversed my own recommendation to sweep the >=6-char sites for shape. Rewriting a dozen correct assertions costs review attention for no risk reduction; the pattern-propagation concern is answered by writing the ">=6 characters" rule into the :49-58 convention comment instead. Fix list is now :95, :303, and test_content_search.py:123 (4 chars, unsafe under that same rule -- an addition to the reviewing session's list, in its own framing). Also records the confirmation-by-prediction: #142's re-run came back 25 passed with the prediction written beforehand, so a green re-run confirms a chance collision rather than resetting the question.
…at hid in the analysis Three sessions reviewed this; each correction below is theirs, verified here rather than adopted. RATES. Recomputed exactly: 1 in 1,821 per assertion per leg, 1 in 911 per leg, 1 in 304 per full CI run (I had floored two of them). JANE at test_content_search.py:123 is 8.58e-6 = 1 in 116,509. THE ZERO. The 14-char row was written as "p=0 (unreachable)". It is 7.44e-24. Cause reproduced: 1-(1-64**-14)**144 UNDERFLOWS to exactly 0.0 in float64, silently, with no warning, in a column of plausible values -- inside an analysis arguing that token length is the discriminator, at the one row where length breaks the arithmetic. The item now carries the trap, the exact value, and the idiom that does not underflow (-expm1(N*log1p(-x))). Every figure recomputed both ways and agreeing. THE COUNT. Three different numbers were quoted before anyone checked (7, then 5, then ~16 for a different denominator). Re-derived from the tree: 7 lines / 8 clauses against ciphertext in test_store_encryption.py, 2 of them unsafe; ~16 repo-wide. The item now states the basis AND the exclusions (:905-908 and :927-928 are caplog assertions against log text, not ciphertext; :56/:532/:625 are full-plaintext; :512 is deterministic twice over) so the count is not re-litigated a fourth time. #344 CITATION. Kept -- its owner confirmed the framing and supplied the discriminator that stops a reader folding the two: this one would fire at exactly the same rate on an infinitely fast machine. Related line now says what the citation is NOT. #346 / ADR 0158. Added as the closer sibling: an assertion that passes for a reason unrelated to the property it tests. ADR 0158 cited WITHOUT a link -- I guessed its filename, checked, and was wrong; the file is on PR #145's branch, not on main. VERIFICATION. backlog_status_check.py falsified against this item: a deliberately doubled banner makes it fail at BACKLOG.md:8429 naming #347. The first probe attempt silently no-op'd on a cp1252 decode and "passed" -- the same shape this item is about.
Revised after review by three sessions (
|
| Scope | Rate |
|---|---|
| one assertion, one leg | 1 in 1,821 |
| either assertion, one leg | 1 in 911 |
| either assertion, one full CI run | 1 in 304 |
A float64 trap now recorded in the item, because it occurred inside the analysis of this very defect. The 14-char row was first written down as p = 0 (unreachable). It is 7.44e-24. The naive expression 1-(1-64**-14)**144 underflows to exactly 0.0 — 1 - 64**-14 is not representable in float64 and rounds to 1.0 — so it reports certainty, silently, with no warning, in a column of otherwise plausible values. In an item arguing that token length is the discriminator, at the one row where length was extreme enough to break the arithmetic. Whoever fixes these tests will re-derive these rates, so the trap and the idiom that survives it are written down.
The count, now stated with its basis. Three different numbers were quoted before anyone checked. Re-derived from the working tree: 7 lines / 8 clauses against at-rest ciphertext in test_store_encryption.py (:95, :232, :233, :251, :303×2, :304, :1062), of which 2 are unsafe — both DOE. Explicit exclusions are listed so it is not re-litigated: :905–908 / :927–928 assert against caplog/exception text, not ciphertext; :56 / :532 / :625 are full-plaintext; :512 is a fixed-nonce blob and : is not in the base64 alphabet.
#344 citation kept, with the discriminator its owner supplied, which is the sentence that stops the two being folded:
#344 is a fixed bound meeting variable latency; #347 is a deterministic property tested by a probabilistic proxy. This one would fire at exactly the same rate on an infinitely fast machine.
#346 added as the closer sibling — an assertion that passes for a reason unrelated to the property it claims to test. This one passes because random base64 usually lacks a 3-character run; #346's would have passed because nothing walks the imports.
Verification of this filing's own instruments
- Every probability computed by two independent methods that agree.
- Audit counts re-derived from the tree rather than quoted.
backlog_status_check.pyfalsified against this item — a deliberately doubled banner makes it fail atBACKLOG.md:8429naming #347, so its green is evidence it can see the item. The first probe attempt silently no-op'd on acp1252decode and "passed" — the same shape this item is about.- ADR 0158 is referenced without a link: I guessed its filename from its title, checked, and was wrong; the real file exists only on PR docs(adr): land ADR 0158 — the silent-controls taxonomy the ledger already pointed at #145's branch, not on
main.
Rebase order
PR #148 (#346) is armed and will land first. Both items append at EOF after #343 and are independent — take both, in either order; backlog_status_check.py should then report 270 items.
…just adopted The #142-owning session pointed out that :512 is safe for a reason unlike the others -- ':' is not in the base64 alphabet -- and suggested it as a third safety category. It is a real category and it is now ground (1), stated more usefully than either of us first had it: a token containing a character the value cannot contain is a PROOF at any length, and it is the same principle that makes the recommended fix (assert ADT not in raw) deterministic. Rule and remedy are now one idea. But the argument as given does not survive, and I checked before adopting it. The haystack is <marker>:<base64>, NOT base64 alone, and the marker carries colons -- so ':' IS representable and ground (1) does not cover :512. It is deterministic for a different reason: fixed marker layout with the version field reading v1, plus a body with no ':' for the run to straddle. Structure, not alphabet. The item now says so explicitly, because a rule that is right about the conclusion and wrong about the mechanism is the thing this whole item is about. Verified mechanically rather than argued: over the haystack's actual character set, the ADT fixture carries \r & . \ ^ | (unrepresentable -> ground 1 genuinely holds), while DOE, JANE, SECRET, WESTWING, SECRETSTATEMRN and ':v2:' are ALL fully representable -- so ground (1) applies to none of them and length is their only defense. That is what the table already claimed; now it is checked. Also records why the float64 trap survives review, from #344's owner: the naive expression is correct everywhere you would sanity-check it and silently wrong only in the tail.
…mber The session landing ADR 0158 (PR #145) confirmed #347 is a genuine instance and supplied the precise anchor instead of a general pointer. Verified against the ADR on its branch before citing -- all three lines read verbatim as quoted: :246 "An equality check satisfiable by coincidence is not an equality check." :60 Class 2 -- a control that cannot OBSERVE or ACT ON its own failure :61 Test: "if this control were broken, what would tell me?" :56 / :439 the taxonomy explicitly disclaims completeness Citing the RULE is what makes the reference survive renumbering, and the Class 2 test is the sharper statement of this defect than anything I had written: if the encryption were replaced tomorrow with a weak encoding, "DOE" not in raw would still go green. The answer to "what would tell me" is the control itself. Still deliberately unlinked -- 0158 is on #145's branch, absent from main, so a relative link renders broken. The follow-up (file this against 0158 once it is on main) is recorded as NOT done here, with the reason: that session declined to add instances its author did not choose, and padding a rescued document at merge time is its own defect. Their call, recorded so it does not read as an oversight.
PR #141 landed BACKLOG #345 at the file's EOF, the same append point as #347. Resolved by keeping BOTH items, main's #345 first (it is already on main) then #347, which is what the sandbox-codec session proposed when we compared append points: two independent OPEN items at EOF carry no ordering meaning. Merged rather than rebased deliberately. Each of the five #347 commits rewrites the same block, so a rebase re-raises the same conflict five times and risks a mid-stack resolution silently keeping an earlier draft; a merge resolves the seam once against the FINAL text. The branch squashes on land, so the merge commit does not reach main. Verified rather than assumed: backlog_status_check.py reports 270 items each with exactly one banner -- the count that session predicted for both items landing -- and #347's post-merge text still carries all three of its late revisions (the infinitely-fast-machine discriminator, the ADR 0158 rule citation, the float64 underflow callout), so the resolution kept the final block and not an early draft.
… was not mine Three fixes, all of the same defect the item is about -- an observational claim carrying more confidence than its sourcing supports. 1. THE 200k SIMULATION IS NOT MINE. It arrived with the originating defect report and I never ran it; the sentence read as though this filing corroborated the rate that way. Every word was accurate, which is the shape: an unsourced observational claim inside a sentence whose whole job is telling the reader how much to trust the number beside it. Now attributed, with this filing's actual contribution (exact Fraction derivation, cross-checked against expm1/log1p) stated separately. 2. "PRODUCED INDEPENDENTLY BY THREE SESSIONS" was an aggregate confidence claim. Two sessions derived rates; the third contributed process discipline. Replaced with itemised attribution -- who supplied the framing, the >=6 rule, the discriminator, the demand to falsify the gate -- and an explicit statement that no claim rests on a count of who agreed. A session count is not evidence. 3. "#344 IS a fixed bound meeting variable latency" -> "#344's THESIS is". That item's instance 2 has since been re-diagnosed as a swallowed lock-timeout (SET LOCK_TIMEOUT 0 -> native 1222 caught and returned as a normal empty, with the dispatcher then parking in a terminal IDLE) -- not a bound at all. The wholesale characterisation was over-broad, and the item now says not to lean on "#344 = timeouts" as a premise. The chain that prompted this went two sessions -> one -> none -> mechanism-only -> mechanism-only-labelled-as-deduction, on a separate claim, every step a good-faith correction, and the conclusion correct throughout. Only the stated mechanism was hollow, and the stated mechanism is what the next reader carries.
…re it is trusted The item told an implementer HOW to fix the assertion but not how to know the fix works. Shipping the replacement on an unfalsified green would reproduce the defect inside the remedy -- a green taken as evidence for a property it cannot see is the whole item. So the fix direction now closes by requiring the deliberate break: hand the store an IdentityCipher or plant a plaintext body, watch the rewritten test go RED, then restore. With the trap that makes it more than a formality, from the session that settled #344's instance 2 today: proving the INSTRUMENT can fire is only half -- the WORKLOAD must also be able to produce the failure class. Their 800-iteration repro loop returned 800/800 green against a live SQL Server while hunting a lock- contention bug, because running the two tests in isolation was the one configuration that could not generate contention. They had falsified the probe and not the rig, which felt like all of it. A rig that excludes the condition it hunts reports silence, and silence reads like evidence. Merged origin/main first (PR #148 / BACKLOG #346 landed): clean auto-merge, no conflict this time, verified by CONTENT and not only by count -- 271 items, #345, #346 and #347 all present, and all five of #347's late revisions still resolving in the merged file.
One file, docs only:
docs/BACKLOG.md+1 item. No code, no test, no CI change.The defect
tests/test_store_encryption.py:95assertsraw.startswith(MARKER_PREFIX) and "DOE" not in raw— three characters of a 76-character body — as the proof that a patient surname is unreadable at rest. The instrument is wrong in both directions:make_cipher(generate_key()), a fresh random key every run, so the base64 body is fresh random text and base64's alphabet containsD,OandE; andThe second half is the PHI defect. The flake is what made someone look; it is not what is wrong.
The rate — per CI run, not per assertion
Surfaced on PR #142, job
91502517146, legtest (windows-2022, py3.14).(L-k+1)/64^kat the observed ~146-char ciphertext gives p ≈ 5.5e-4 for a 3-char literal. But there are two such assertions (:95,:303) and this repo runs three OS legs (ubuntu+windows-2022+windows-2025, verified againstci.yml):Both over-estimate caveats are stated in the item rather than buried: L comes from one measured value, and the
mfenc:v1:<hex-fingerprint>:prefix is not base64, so the effective window count is lower.Fix scope — neither "just one" nor "all of them"
Fix:
:95,:303(3 chars, 5.5e-4 each) andtest_content_search.py:123(JANE, 4 chars, ~9e-6). Leave alone: ~13 sites at ≥6 chars (≤2e-9) — same pattern, but not a defect at any rate that will ever be observed, and churning correct assertions costs review attention for no risk reduction. The pattern-propagation risk is answered instead by writing the rule into the existing convention comment at:49–58:test_off_by_default_stores_plaintextdoes not share the shape (deterministic equality), and neither do the many"DOE" not in …assertions elsewhere that checksafe_text-scrubbed plaintext rather than ciphertext.Why file rather than fix
The fix direction is the maintainer's call — decoded-bytes assertion, non-recoverability, or full-plaintext absence (
assert ADT not in raw, deterministic because the fixture contains|and\r). Widening or deleting the substring check would drop the PHI-at-rest property it reaches for, which is the failure mode this repo already tracks.Provenance
Diagnosed independently by two sessions and reconciled. The instrument-first framing, the ≥6 rule and the leave-the-rest-alone scoping come from the review by the session that owns PR #142; the fourth site (
content_search.py:123) and the per-CI-run multiplier came back the other way. The item also records a confirmation by prediction: #142's re-run returned 25 passed with the prediction written beforehand, so a green re-run confirms a chance collision rather than resetting the question.Coordination
alloc.ps1; claimed FILED ONLY, NOT BUILT — the fix is unclaimed and free for any session to take.backlog_status_check.pypasses: 269 items, each declaring exactly one status.#345/#346also land. One file, so it rebases trivially in either direction.🤖 Generated with Claude Code