test(store): pin the PHI-at-rest assertion FORM with a negative control - #171
Closed
wshallwshall wants to merge 1 commit into
Closed
test(store): pin the PHI-at-rest assertion FORM with a negative control#171wshallwshall wants to merge 1 commit into
wshallwshall wants to merge 1 commit into
Conversation
…tive control Closes a review gap the coordinator raised on PR #168: the "against a simulated leaking store the new assertion fails" check was the single most important claim in that change -- it is what separates strengthening an assertion from silencing one -- and it existed only as a throwaway rig. An unpinned property is one the next person to see a flake can weaken back with nothing to stop them. Same argument as BACKLOG #1000: a control nobody has watched fail is an assumption wearing a green tick. Four tests, over the assertion FORM rather than the store: 1. the marker check alone does NOT detect a leak. Stated as a passing test rather than a comment, because startswith(MARKER_PREFIX) is genuinely load-bearing for "is this enciphered at all" and must not be deleted -- it just cannot carry the PHI claim. 2. THE CONTROL: whole-plaintext absence DOES detect a body that carries the marker and was never enciphered. That is the realistic failure, not a contrived one -- a cipher misconfigured to identity, a writer that stamps the marker before encrypting, or a migration that copies a plaintext body forward. 3. it does not flake on real ciphertext (200 real tokens, real key). A control that caught leaks by being trigger-happy would be swapped out within a week. 4. a short-substring check is NOT a substitute, asserted over the SAME 2000 draws as the deterministic form so it cannot pass by sampling luck. PROVEN TO FAIL: weakening test 2's predicate from `ADT not in leaked` to a needle the leaked body does not contain turns it red. Restored, 4 pass, ruff clean. Deliberately a SEPARATE FILE from test_store_encryption.py. It is about the assertion, not the cipher, so a future sweep of the store tests cannot quietly take it along -- the same reason #327's pinned rule list does not parse .gitignore. Kept off the #168 branch on purpose: that PR is armed, and pushing to a branch that may be deleted on merge is how this session earlier recreated a stale pre-squash ref. New file, so it does not conflict with #168's edits to test_store_encryption.py and can land in either order.
wshallwshall
enabled auto-merge (squash)
August 4, 2026 03:10
Collaborator
Author
|
Superseded by #177, which batches this with the other two small code changes into one CI cycle. Nothing is dropped — this commit is carried across verbatim in #177. Closing rather than leaving it open because 🤖 Batched by Claude Code |
auto-merge was automatically disabled
August 4, 2026 14:09
Pull request was closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One new file, +106. Closes a review gap on the PHI-at-rest work: the check that justified changing those assertions existed only as a throwaway rig, so the property it established was unpinned.
That matters because of what the change looks like from the outside. The old assertion was flaky — roughly 1 red CI leg in 2,222 per assertion on correct encryption. The obvious response to a flaky assertion is to weaken it until it stops failing, and from a diff alone that is indistinguishable from strengthening it. The only thing that separates the two is a control proving the assertion still detects the leak it exists to catch. Without one committed, the next person to see a red leg can weaken it back and nothing stops them.
Four tests over the assertion FORM
mfenc:while sitting in cleartext satisfies the prefix assertion completely.Proven to fail: weakening the predicate back to a short-substring check turns the last test red.
Why it is a separate branch rather than a commit on the PHI PR
That PR is armed. Pushing to an armed branch that may be deleted on merge is how a stale pre-squash ref got recreated earlier today — a branch carrying six already-merged commits whose three-dot diff read as 6,937 insertions. New file, no overlap with the other branch's edits, so the two land in either order.
No ledger token: this is a follow-up to an item, not an item.