fix(phi-scan): widen the walk to src, test and scripts - #40
Merged
Conversation
The all-mode walk was rooted at `test/__fixtures__` and `src` only, so 89 of this repository's 123 tracked files were scanned by NEITHER of the scanner's two routes: PHI-SCAN-WALK-ROOT-SCOPE, the other half of the item `#38`/`#39` closed. It now roots at `src`, `test` and `scripts`, opening 72 tracked files instead of 34. RE-DERIVED FOR THIS REPOSITORY, NEVER PORTED. Measured on `ba059a2`: 123 tracked, 34 opened (7 fixtures + 27 `src/`), 89 in neither route, 7 of those carrying an inline HL7 `PID|` literal. RED before, GREEN after, back to back on that sha: a dashed SSN and an off-domain address written into `test/planted.test.ts` in this repo's own inline-message shape (a whole HL7 message as one `.ts` string literal with `\r` escapes between segments) exited 0 `OK, no hits` in all mode, while `phi-scan test/planted.test.ts` reported both at exit 1 over the same bytes. A file under `scripts/` behaved the same. 14 of the suite's 68 cases red against `ba059a2`'s scanner. ALL 38 NEWLY OPENED FILES WERE HAND-READ. Every message literal is a placeholder (`DOE^JANE`, `X^^^H^MR`, `SENDER`, `ZZSENTINEL*`) and the only SSN/email shapes anywhere are the scanner's own declared synthetic payload. So the 89 were an ENUMERATION gap, not a live PHI exposure: the defect is that the gate could not see those files, so nothing would have caught a real one. `test` REPLACES `test/__fixtures__` rather than joining it. Roots must stay disjoint: each is walked independently and the results concatenated, so a nested root would enumerate every file beneath it twice and report each hit twice. The fixture dir is still watched, through the observation rule's other condition; only the root a refusal is filed under moved, which is what the four updated assertions are. `scripts` is in because the allow-list, the override log and this scanner live there, so the one directory guaranteed to hold PHI-shaped text was the one nothing enumerated. All nine files were measured against the floor before the root was declared: zero hits. Not roots, each measured: `vendor/` (DEFLATE streams), `docs-content`/`documentation`/`.changeset` (all `.md`, which the walk skips), `.github` and the repo root. A NARROWING CAME WITH IT: `test/__fixtures__` is no longer a root, so a live or dangling link there is now an enumerated entry and is refused outright, whatever it points at. Only the three top-level roots are still followable. The two residual pins were retargeted one level up and a new case pins the narrowing. `test/scripts/phi-scan.test.ts` is the ONE exempt path. Applied at the SCAN, never the enumeration (still read, still observed, still reconciled; unreadable still exits 2); SCOPED to the all-mode sweep (naming it still reports every hit, because an unscoped exemption DELETES a detection the base had); and PER PATH, never a pattern. Each property has a control that reds when it is removed. Allow-listing the values was refused: `EMAILDOMAIN` is global. WHAT THIS BOUGHT AND WHAT IT DID NOT. The SSN/email floor over 38 more files and nothing else; structured field-level detection is still the unimplemented TODO, and a test pins what is still missed. The recogniser was NOT widened, on measurement: this floor is anchor-free, so it never had the "the file IS the document" defect, and an escape-decoded second view finds nothing the raw view does not over the whole newly-opened corpus. That measurement ships as a TRIPWIRE with a negative control, not a one-time claim. `--staged` is unchanged, deliberately: widening it is a hook decision about what a commit is BLOCKED on. The path-SET escape is unchanged and still disclosed. Both named PRE-EXISTING minors were re-measured NOT open here. verify.sh cli: 12 steps ran, 10 green. `pnpm audit --prod --audit-level high` and the licenses gate fail, both reproduced byte-identically on a base tree restored by file copy; `package.json` and `pnpm-lock.yaml` are untouched here and no CI job runs either command. 424 tests, 97.7% statements.
…at the scripts root costs Gate pass 1 (conformance-refuter) returned REFUTED with two majors and three minors. No blocker: no detection the base had is deleted, the detector is byte-identical, and no PHI escapes. Every finding was reproduced by hand before being acted on. MAJOR 1, the scripts root costs the allow-list its own bytes. phi-allow-list.txt documents `ID <value>` as a synthetic id "matching an SSN / MRN / member-id shape", and the dashed-SSN check consults no allow-list, so an `ID` entry in that dashed shape now reds the gate on the allow-list itself. Measured: exit 0 on ba059a2, exit 1 here. Latent today (the shipped file declares its only id in the `MRN-` form) and it stops being latent the moment someone acts on scanTarget's TODO to add the structured id detector that consumes allow.ids. Disclosed, not "fixed": making the SSN pass read allow.ids would DELETE a detection the base had, and exempting the allow-list would leave the likeliest place for a real value unswept. Neither remedy the hit message prints works either, which is now written down: "declare it in the allow-list" is circular, and --allow-fixture routes the invocation down paths mode where the one named target is filtered out, so it opens zero files and prints OK (PRE-EXISTING, identical on ba059a2). MAJOR 2, two of the three "deliberately NOT a root" measurements were false. "every tracked file under docs-content/, documentation/ and .changeset/ is .md, so declaring them would open not one new byte" is wrong: documentation/ is 6 of 6, but docs-content/ is 9 of 10 (sidebars.json) and .changeset/ is 3 of 4 (config.json). ".github/ and the repository root: measured clean" is half wrong: .github/ is clean over 8 files, and the repository root is NOT, because package.json's author field carries a real off-domain contact address. All four are now checks in the suite rather than sentences a reader has to trust. This is the third time this repository has written down a measurement it did not take. MINOR 3, the vacuity guard on the exemption's most safety-relevant case did not guard: the skip branch asserted `typeof readable === "boolean"`, true of every value, so under a uid that ignores mode bits the case went green having asserted nothing while hasAssertions was satisfied by the tautology. Each branch now asserts its own premise. The two PRE-EXISTING cases carrying the same shape are left alone. MINOR 4, the fixture directory's remaining cover is CONDITIONAL and was stated flat. It holds where git tracks files under test/__fixtures__ (here, seven). Where git tracks nothing under it, an empty one no longer refuses: ba059a2 exits 2, this exits 0. A real loss, now pinned and disclosed rather than implied away. MINOR 5, walkedCorpus() claimed to be "exactly what the widened walk opens". It is the TRACKED files; the walk also opens an untracked, non-gitignored file under a root. One-directional, so the tripwire can under-cover but never over-claim. Corrected, and given -z so a C-quoted path cannot become a name no file has. The gate caught this remediation red-handed on its own rule: a first draft of the new banner SPELLED an example SSN and a real address, scripts/ is now under its own scan, and phi-scan reported a hit on itself. The banner now names the shapes without spelling them, the literals stay in the exempt test file, and CLAUDE.md carries the one-line imperative. Re-derived rather than carried: 16 of the suite's 74 cases red against ba059a2's scanner (a first draft read 14 of 68, before these cases existed), measured by swapping that scanner under the head suite. CLAUDE.md is 31,449 of its 31,451 budget. Room was made by relocating two rationale clauses already verbatim in documentation/agent-notes.md and by cutting meta-text. No trap was deleted. verify: twelve steps ran, 430 tests pass, ten green. The two red steps are pre-existing and their inputs cannot have moved: pnpm audit (transitive advisories under @modelcontextprotocol/sdk) and pnpm licenses (ERR_PNPM_MISSING_PACKAGE_INDEX_FILE on the vendored @cosyte/fhir tarball), with package.json and pnpm-lock.yaml byte-identical to base.
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
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.
What
The all-mode walk of
scripts/phi-scan.tswas rooted attest/__fixtures__andsrconly, so 89of this repository's 123 tracked files were scanned by NEITHER of the scanner's two routes. It now
roots at
src,testandscripts, opening 72 tracked files instead of 34.This is
PHI-SCAN-WALK-ROOT-SCOPE, the other half of the item#38/#39closed: the observationrule made the gate refuse a root it never opened, and this moves the roots so they cover the files
that actually exist.
The measurement, re-derived on
ba059a2rather than ported123 tracked files; 34 opened by the walk (7
test/__fixtures__+ 27src/); 89 in neitherroute; 7 of those carrying an inline HL7
PID|literal.RED before, GREEN after, back to back on that sha. A dashed SSN and an off-domain address written
into
test/planted.test.ts, in this repository's own inline-message shape (a whole HL7 message as one.tsstring literal with\rescapes between its segments), exited 0 withOK, no hitsin allmode, while
phi-scan test/planted.test.tsreported both at exit 1 over the same bytes. A filewritten under
scripts/behaved identically. Both routes now report both. 14 of the suite's 68cases red against
ba059a2's scanner.All 38 newly opened files were hand-read. Every message literal is a placeholder (
DOE^JANE,X^^^H^MR,SENDER,ZZSENTINEL*) and the only SSN/email shapes anywhere are the scanner's owndeclared synthetic payload. So the 89 were an ENUMERATION gap, not a live PHI exposure - the
defect is that the gate could not see those files, so nothing would have caught a real value.
The roots
testreplacestest/__fixtures__rather than joining it. Roots must stay disjoint: each iswalked independently and the results concatenated, so a nested root would enumerate every file
beneath it twice and report each hit twice. The fixture directory is still watched, through the
observation rule's other condition; only the root a refusal is filed under moved, which is what four
updated assertions are.
scriptsis in because the allow-list, the override log and the scanner itself live there, so the onedirectory guaranteed to hold PHI-shaped text was the one nothing enumerated. All nine files were
measured against the detector before the root was declared: zero hits. Deliberately not roots,
each measured:
vendor/(DEFLATE streams),docs-content/documentation/.changeset(everytracked file is
.md, which the walk skips),.githuband the repository root.A narrowing came with it:
test/__fixtures__is no longer a root, so a live or dangling symboliclink at that path is now an enumerated entry and is refused outright, whatever it points at. Only
the three top-level roots are still followable.
The one exempt path
test/scripts/phi-scan.test.tscarries violator literals on purpose, as the positive half of thescanner's own tests. Three properties, each with a control that reds when it is removed:
an unreadable one still exits 2 rather than passing as exempt;
exemption would DELETE a detection the base had;
Allow-listing the values instead was refused:
EMAILDOMAINis global, and the dashed-SSN checkconsults no allow-list at all.
What this bought, and what it did not
It bought the SSN/email floor over 38 more files and nothing else. Structured, field-level
detection is still the unimplemented TODO in
scanTarget, and a test pins what is still missed (anundashed nine-digit id, a name, a DOB and an address in the same
PIDsegment, all at exit 0).The recogniser was NOT widened, on measurement. The companion defect this class carries is that a
recogniser assumes the file IS the document, so enumerating a
.tssource whose message is aninline literal buys nothing. That needs an anchored detector, and this scanner has none:
scanCommonShapesis two unanchored passes over the whole text. Pinned by an anchor-free probe thatputs one token in three placements and requires all three to red. The one widening a sibling shipped
here, an escape-decoded second view, finds nothing the raw view does not over the whole
newly-opened corpus, so it ships as a tripwire with a negative control rather than as a guard with
no measurement behind it: if a source ever hides a token behind an escape, the suite reds and says to
widen in addition to the raw pass.
Out of scope, stated rather than implied
--stagedis unchanged. Widening it changes what a commit is BLOCKED on, which is a hookdecision. The two routes therefore differ widely and the module header says by how much.
compares path sets, not the bytes git carries at those paths, so a root swapped for a directory
mirroring the tracked names still exits 0 over decoy contents. The widening does not make it
worse; it only moves which paths a decoy has to mirror.
PRE-EXISTINGminors were re-measured NOT open here..github/workflows/ci.yml's banner says the ruleset requires four contexts while it requiresseven. Flagged, not fixed: it is its own item.
Verify
verify.sh cli: 12 steps ran, 10 green, 424 tests, 97.7% statements. The two failures arepnpm audit --prod --audit-level highand the licenses gate(
ERR_PNPM_MISSING_PACKAGE_INDEX_FILEfor the vendored@cosyte/fhirtarball). Both reproducebyte-identically on a base tree restored by file copy,
package.jsonandpnpm-lock.yamlareuntouched by this branch, and no CI job runs either command. The licenses one is newly visible
rather than newly broken: the umbrella's ladder used to print green on that step without running it.