From 9c6bb9544574b731b34b9d63c643f1756df657a0 Mon Sep 17 00:00:00 2001 From: Noah Schatz Date: Sat, 8 Aug 2026 12:18:45 +0000 Subject: [PATCH 1/4] fix(phi-scan): scan the tracked corpus and reconcile it against git, plus an HL7 v2 structured pass (PHI-SCAN-WALK-ROOT-SCOPE) Both enumerating routes covered `test/fixtures/` + `src/` only: 31 of 102 tracked files. 71 were read by NEITHER route, 27 of those under `test/`, and 8 of those carried inline HL7 `PID|` literals with names, DOBs and MRNs. THE SHARPEST HALF IS THIS REPO'S OWN: `test/fixtures/` HAS NEVER EXISTED ON ANY COMMIT (`git log --all` is empty for it). The walk's `existsSync` guard returned on its first line for that root on every run this scanner has ever made, and every one of those runs printed "OK: no hits" and exited 0. A count does not detect that (it counts the roots that DID exist) and neither does an existence check (an EMPTIED root opens nothing), so the remedy is reconciliation: the walk now covers every tracked directory plus repo-root files, and `reconcileWithGit` refuses (exit 2), naming every tracked path the walk did not open. A non-directory root refuses before the walk too, because `existsSync` FOLLOWS: a dangling root read false and printed clean over an off-disk corpus, and a symlinked root was followed. Exit code for a regular-file root is 2 HERE, derived from this script's own contract (`ENOTDIR` into `walk`'s catch), not ported from a sibling. WIDENING IS TWO-SIDED. Enumerating buys the SSN/email floor and nothing else, and measured over the 8 `PID|` files the floor finds ZERO: no dashed SSN, no email. So an HL7 v2 structured pass ships with it, in addition to the floor and never instead: PID/NK1/GT1/IN1 by field and component, checking names, DOB, MRN/member-id/SSN, address and phone against the allow-list. It finds segment literals INLINE, because this package ships no standalone `.hl7` file at all: every message in its corpus is a `.ts` string literal. THE GRID, PROVED RATHER THAN ARGUED: 169 base-vs-head cells over 15 paths, 4 payloads and all 3 routes. 37 base `1` cells still `1`, 74 cells `0 -> 1`, and ONE `1 -> 0`. THAT ONE CELL, DISCLOSED: `phi-scan package.json` exited 1 on the npm publisher mailbox in its `author` field and now exits 0. Declared with the new `EMAIL` tag, which takes a path AND an address, so the same address elsewhere and any other address there both still report. An allow-list entry is still ROUTE-BLIND and that reach is pinned from both directions by tests. 69 files hand-read. Nothing patient-identifying. The fixture placeholders are NAMED in `scripts/phi-allow-list.txt` rather than scrubbed: `Jane Q. Public`, `Jane Doe`, the suite mnemonics, `MRN1`/`MRN2`/`MRN12345`, an SSN-shaped `999887777` in area 999 (never issued), placeholder streets, and two numbers in the reserved 555 range. The one org-traceable string is the publisher mailbox. The scanner's own test file is now inside the corpus, so its violator payloads are assembled from parts at runtime. Allow-listing them would blind the floor globally and route-blind; exempting the file would leave the largest violator-bearing file unscanned. Also: the walk's regular-`*.md` skip is removed (purely additive, since the `` route always read those bytes, so the two routes disagreed); `.git` is a regular FILE in a submodule and is skipped by literal name; the all-route exemption list is two literal paths (the vendored gzip tarballs, whose bytes are not the text they carry) and never reaches a blocking route. Four residuals stay disclosed, not closed, including that the reconciliation compares path SETS and not the bytes git carries at them, and that the enumerate-then-read race's precondition has now happened (`.changeset/` is in scope; it exits 2, not clean). --- .changeset/wild-pugs-remain.md | 17 + .github/workflows/ci.yml | 9 +- CLAUDE.md | 64 +-- documentation/agent-notes.md | 156 ++++++- phi-scan-overrides.md | 15 +- scripts/phi-allow-list.txt | 108 ++++- scripts/phi-scan.ts | 720 ++++++++++++++++++++++++++++----- test/scripts/phi-scan.test.ts | 563 +++++++++++++++++++++++--- 8 files changed, 1444 insertions(+), 208 deletions(-) create mode 100644 .changeset/wild-pugs-remain.md diff --git a/.changeset/wild-pugs-remain.md b/.changeset/wild-pugs-remain.md new file mode 100644 index 0000000..98fe875 --- /dev/null +++ b/.changeset/wild-pugs-remain.md @@ -0,0 +1,17 @@ +--- +"@cosyte/transform": patch +--- + +No runtime impact: the repository's own PHI commit-gate read 31 of its 102 tracked files, and it now reads 100 of them and refuses when it cannot account for the rest. + +Both of the gate's enumerating routes covered `test/fixtures/` and `src/` only. Seventy-one tracked files were read by neither of them, twenty-seven of those under `test/`, and eight of those carried inline HL7 v2 patient-identification segments with names, dates of birth and medical record numbers in them. The sharper half is that `test/fixtures/` has never existed in this repository, on any commit: the walk's existence check returned on its first line for that root on every run the gate has ever made, and every one of those runs printed a clean result and exited zero. An unopened root and a clean one are indistinguishable from the outside. + +Neither a file count nor an existence check detects that, and both were considered and rejected: a count counts the roots that did exist, and refusing a missing root leaves an emptied one reporting clean. So the walk now covers every tracked directory plus the files at the repository root, and reconciles what it actually opened against the list of files version control actually carries. A tracked path the walk did not open now refuses, naming each one. A declared root that is a symbolic link, dangling or not, refuses too: the existence check follows a link, so a dangling root read as absent and the whole corpus went unscanned while the run reported success. + +Enumerating more files buys the cross-cutting social security number and email checks and nothing else, and measured on this repository those two find nothing at all in the eight fixture files: they carry no dashed social security number and no email address. What they carry is names, dates of birth, record numbers, one undashed social security number, a street address and two telephone numbers. So a structured pass ships alongside the existing one, never in place of it, reading HL7 v2 patient, next-of-kin, guarantor and insurance segments field by field and component by component. It finds segments inline rather than assuming a file is a message, because this package ships no standalone message file at all: every fixture is a string literal inside TypeScript. + +Every value the structured pass reports is checked against the reviewed synthetic-fixture declaration list, which gains entries for this repository's placeholders. Each was read by hand first and each is named in that file rather than removed, because removing them would destroy the evidence the audit happened. Nothing patient-identifying was found in any of the sixty-nine files the widening newly opened. + +One detection is subtracted and it is the only one. The package manifest carries the publisher's own contact address, which the email check cannot tell from a patient's, so scanning the manifest at all required declaring that one address. It is declared with a path as well as an address, so the same address in any other file still reports and any other address in the manifest still reports, and the cost is that naming the manifest directly no longer reports it. Every other outcome is unchanged or newly caught: a hundred and sixty-nine before-and-after cases across every path shape, every payload shape and all three ways the gate can be invoked, with thirty-seven that reported before still reporting and seventy-four that are newly caught. + +Four limits stay disclosed rather than quietly closed, including that the reconciliation compares path names and not the bytes stored at them. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ad69eb..df338c6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,10 +39,11 @@ jobs: # Universal gates from the shared pipeline (typecheck, lint, format, test, coverage, build, attw, # dual ESM/CJS smoke, actionlint) on the standard Node matrix. Repo-specific extra jobs (e.g. an # examples smoke, a starter-kit build) are added on top of this caller, never instead of it. - # The template ships a PHI scanner (scripts/phi-scan.ts), so the shared pipeline's optional - # PHI-scan step is switched on. NOTE: the scaffolded scanner is a STARTER: it detects only - # cross-cutting SSN/email shapes until you add structured, field-level detection for this - # standard's PHI (see the STARTER banner in scripts/phi-scan.ts). + # The shared pipeline's optional PHI-scan step is switched on, and what it runs is the all-mode + # walk over this repo's whole TRACKED corpus, reconciled against `git ls-files` so a root that is + # missing, emptied or replaced refuses rather than reporting clean. It runs the cross-cutting + # SSN/email floor AND an HL7 v2 structured pass over PID/NK1/GT1/IN1. What it still does not see + # is written out in the banner at the top of scripts/phi-scan.ts. ci: uses: cosyte/.github/.github/workflows/ci.yml@main with: diff --git a/CLAUDE.md b/CLAUDE.md index 5733351..034a428 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -172,38 +172,40 @@ Full narrative, every measurement: `documentation/agent-notes.md#the-attw-guardr ### The PHI scanner -Full narrative, every measurement and both refuter passes: -`documentation/agent-notes.md#the-phi-scanner-guardrail-in-full`. +Measurements, the grid, the refuters, and the `--staged` ARGV traps (`--diff-filter` keeps `T`/`U`, +`--no-renames`, STATUS not mode, re-measure the stride): +`documentation/agent-notes.md#the-phi-scanner-guardrail-in-full`. **No counts.** -- **▶ THE CLAIM IS EXACTLY: the scanner refuses (exit 2) every entry it ENUMERATES, and every path - NAMED DIRECTLY, that is not a regular file.** "It follows nothing" is the looser wording and **two - separate refuter passes measured it FALSE.** Do not tighten the sentence back up. -- **Do not "fix" the link blindness by following the link**: it reads bytes the enumeration does not - control (outside the repo, a loop, a device, a blocking FIFO), and git does not carry them anyway. -- **`lstat` answers for the FINAL COMPONENT ONLY.** A named path with a **symlinked ancestor**, or a - plain absolute/`../` argument, is still followed. **Pre-existing, disclosed, deliberately NOT - closed.** If you touch `buildTargetsForPaths`, **re-measure the sentence, do not re-assert it**. -- **▶ THE ONE-LETTER TRAP: `--diff-filter` MUST KEEP `T`.** Replacing a tracked file with a link is - neither an add nor a modify: under `AM` the record dies before any mode is read and a mode-`120000` - blob passes **green**. Keep `U` too (refused: no stage-0 entry), and keep `--no-renames`, which is - what makes a staged rename arrive as a single-path `A`. Key on the **STATUS, not the mode**. -- **Adding any argument to that `git diff --cached` ARGV means re-measuring the record stride**: - `--find-copies-harder` re-enables two-path records even placed _before_ `--no-renames`. -- **A refusal never echoes the link target**: that is working-tree text and can itself carry PHI. - Name the repo-relative path plus a token from the closed `entryKind`/`gitModeKind` sets, nothing - else. **This applies to the prose too**: write the dangerous target as a _shape_, never an example. -- **The walk has NO extension scope of its own** (it skips regular `*.md` and takes everything else); - `src/**.ts` is the **`--staged`** route's boundary. **Do not describe the two as one rule.** -- **Exit `2` means every failure to complete; exit `1` means HITS FOUND.** An uncaught throw used to - land on node's `1`, so a caller read a gate that never ran as one that ran and fired. -- **Three residuals are disclosed, NOT closed**: (1) an empty enumeration reports clean; (2) the - ancestor-component / absolute / `../` reads above; (3) a scan **root's own path** staged as a - non-regular entry is outside the `--staged` route's prefix scope. **Do not silently re-close any, - and do not let a future edit read as though they were.** -- **Test against throwaway git repos under `os.tmpdir()`**: the scanner roots everything at - `process.cwd()`, so **never write a link or a violator into this corpus** to test it. -- **The enumerate-then-read race is deliberately still open**, unreachable by scope today, _until a - walk root widens_, which reintroduces it verbatim. +- **▶ THE CLAIM IS EXACTLY: it refuses (exit 2) every entry it ENUMERATES, and every path NAMED + DIRECTLY, that is not a regular file.** "Follows nothing" is looser and **two refuter passes + measured it FALSE**; do not tighten it back, and never close it by following. **`lstat` answers + for the FINAL COMPONENT ONLY**: touch `buildTargetsForPaths` and **re-measure, never re-assert**. +- **A refusal never echoes the link target** (path plus a closed-set kind token), **and that binds + the prose too**: write the target as a _shape_, never an example. +- **▶ THERE ARE THREE ROUTES, NOT TWO** (`all`, `--staged`, ``), all running the content + passes. **Enumerate all three before calling anything additive.** +- **▶ SCOPE IS THE TRACKED CORPUS, RECONCILED AGAINST `git ls-files` EVERY RUN**, because + **the `fixtures` root HAD NEVER EXISTED ON ANY COMMIT** and went unopened on every run ever made while + the run printed clean. **A count cannot detect that**, nor can an existence check: an EMPTIED root + opens nothing. **Roots stay DISJOINT** or nested files report twice. The `*.md` walk skip is gone + (purely additive); `src/**.ts` was the **`--staged`** bound, now widened. **Not one rule.** +- **▶ WIDEN BY UNION AND PROVE THE GRID: every base `1` still `1`.** One cell is not: + **`phi-scan package.json` on the npm publisher mailbox**, declared with `EMAIL` (a **path AND an + address**). **Every allow-list entry is ROUTE-BLIND** and clears on `--staged`; every tag but + `EMAIL` is FILE-blind. **Named, never scrubbed.** +- **An exemption is a LITERAL PATH, never a predicate, and reaches the ALL route only**: the vendored + gzip tarballs are the whole list, and `` still reads them. +- **Exit `2` is every failure to complete; `1` is HITS FOUND. A regular-file root is `2` HERE, + derived from this contract; siblings differ, never port one.** A non-directory root refuses first: + **`existsSync` FOLLOWS**, so a dangling one printed clean over an off-disk corpus and a symlinked + one was followed. An **absent** root is fine. +- **Enumerating buys the SSN/email floor and NOTHING else**, so the HL7 v2 pass ships **in addition + to** it, never instead: the floor finds **zero** in this repo's `PID|` fixtures. **No standalone + `.hl7` ships: every message is a `.ts` literal.** +- **Four residuals are disclosed, NOT closed**, named in the notes; the reconciliation is **path + sets, not bytes**; **the enumerate-then-read race precondition HAPPENED.** +- **Throwaway repos under `os.tmpdir()`; never write a violator here. The scanner's own test file is + IN the corpus**: payloads assemble at runtime, never as literals. ### The agent-instruction contract gate diff --git a/documentation/agent-notes.md b/documentation/agent-notes.md index f89e02a..8352206 100644 --- a/documentation/agent-notes.md +++ b/documentation/agent-notes.md @@ -336,9 +336,124 @@ the destination mode is visible; `--name-only` cannot see it. Name the entry's own repo-relative path plus a token from the closed `entryKind`/`gitModeKind` sets, nothing else. **This applies to the prose too**: a diagnostic about a PHI leak is itself a PHI surface, which is why the docblock writes the dangerous target as a _shape_ and not an example. -**The walk has NO extension scope of its own**: it skips regular `*.md` as documentation and takes -everything else, so a link at `src/leak.json` and a linked directory are refused there too. -`src/**.ts` is the **`--staged`** route's boundary; do not describe the two as one rule. +**The walk has NO extension scope of its own**, so a link at `src/leak.json` and a linked directory +are refused there too. It used to skip a regular `*.md` before reading a byte of it; **that +exemption is gone** and the removal is purely additive (see the scope section below). `src/**.ts` +was the **`--staged`** route's boundary, not the walk's, and that suffix bound is gone too. + +### The scan scope is the tracked corpus, and it is reconciled against git + +**▶ THE HEADLINE, MEASURED ON THIS REPOSITORY AT `daf75c3` RATHER THAN PORTED FROM A SIBLING.** Both +enumerating routes covered `test/fixtures/` plus `src/`, and that was **31 of 102 tracked files: 71 +read by NEITHER route, 27 of them under `test/`, 8 of those carrying inline HL7 `PID|` literals** +with names, DOBs and MRNs in them. Head state: **102 tracked / 100 opened / 2 in neither**, both of +those declared literal exemptions, and **0 under `test/`**. + +**▶ AND THE SHARPEST HALF, WHICH IS THIS REPOSITORY'S OWN AND NOT A SIBLING'S: `test/fixtures/` HAS +NEVER EXISTED HERE, ON ANY COMMIT.** `git log --all -- 'test/fixtures*'` is empty. The walk's +`existsSync` guard returned on its first line for that root on **every run this scanner has ever +made**, and every one of those runs printed `OK: no hits` and exited 0. **A declared root that was +never opened is indistinguishable from a clean one.** + +**▶ A COUNT DOES NOT DETECT THAT, AND NEITHER DOES AN EXISTENCE CHECK.** A file count counts the +roots that DID exist, so a healthy total says nothing about a root nobody opened; and refusing a +MISSING root leaves the other half, because an EMPTIED one opens nothing and still reports clean. +The only thing that observes either is `reconcileWithGit`, which compares what the walk actually +OPENED against `git ls-files` and refuses (exit 2), naming every tracked path that was not opened. +**Never re-add a `tracked.has()` pre-check in front of a read**: that makes the walk agree with git +by construction, at zero firings, exactly as it did in the contract gate. + +**▶ WHAT THE RECONCILIATION DOES NOT CLOSE, AND NO REPO IN THIS ECOSYSTEM HAS: IT COMPARES PATH +SETS, NOT THE BYTES GIT CARRIES AT THOSE PATHS.** A root swapped for a directory mirroring the +tracked NAMES still reconciles, over decoy contents. Widening makes that narrower rather than +closed: a decoy must now mirror 100 names, not 7. It is also **vacuous on an empty index**, which is +the state every throwaway repo in the suite is in, so the walk's own refusals are what hold there. + +**▶ A WALK ROOT THAT IS NOT A DIRECTORY NOW REFUSES BEFORE THE WALK, AND `existsSync` IS WHY IT WAS +INVISIBLE. `existsSync` FOLLOWS**, so a DANGLING root read false, `walk` returned immediately, and +the run reported clean with the corpus off the disk: measured here, `OK: no hits` / exit **0**. A +root that was a symlink to a real directory was the other half, and it was **followed**, so the scan +read bytes git does not carry and called them the corpus. Both refuse with **2** now, via an `lstat` +per declared root. An **absent** root is deliberately not an error: a tree may legitimately lack +one, and the reconciliation is what notices anything tracked lived under it. + +**▶ THE EXIT CODE FOR A REGULAR-FILE ROOT IS `2` HERE, DERIVED FROM THIS SCRIPT'S OWN CONTRACT AND +NOT PORTED.** Measured before the change: `existsSync` answered true, `readdirSync` threw `ENOTDIR` +into `walk`'s catch, which raises an `InvocationError` and returns 2 from `main`. The new `lstat` +preflight answers first and returns the same 2, so the code did not move. **Siblings differ and +porting one is the bug this item exists to stop.** + +**▶ ROOTS MUST STAY DISJOINT, AND WIDENING IS BY UNION.** `test` covers `test/fixtures` rather than +sitting beside it: declaring both reports every nested file twice. The previous list is a strict +SUBSET of the new one, and the `--staged` scope's previous predicate is a strict subset of its new +one (the `.ts` suffix requirement is dropped, not kept), so nothing either route saw can stop being +seen. **Proved by grid rather than argued: 169 cells over 15 paths, 4 payloads and all 3 routes, +each run against the base scanner and the head scanner. 37 base `1` cells still `1`, 74 cells +`0 -> 1`, and exactly ONE `1 -> 0`,** which is the next paragraph. + +**▶ THE ONE SUBTRACTION, NAMED RATHER THAN LEFT TO BE FOUND: `pnpm phi-scan package.json` exited 1 +on the npm publisher contact in its `author` field and now exits 0.** That mailbox is public, +organisational and not PHI; it is **named in `scripts/phi-allow-list.txt` rather than scrubbed**, +because deleting it to get green would destroy the evidence the audit looked at it. It is declared +with the `EMAIL` tag, which takes **two literals, a path and an address**, so it is the narrowest +instrument this allow-list has: the same address in any other file still reports, and any other +address in that file still reports. **An allow-list entry is still ROUTE-BLIND** (it clears on the +commit-blocking `--staged` too) and that reach is pinned from both directions by tests rather than +asserted. Without the entry the choice was a worse hole (exempting the whole file) or an unusable +gate (every commit touching `package.json` blocked). + +**▶ THE ALL-ROUTE EXEMPTION LIST IS TWO LITERAL PATHS AND IT NEVER REACHES A BLOCKING ROUTE.** The +two vendored `pnpm pack` tarballs are gzip archives: their bytes are not the text they carry, so a +text pass over them is neither a detection nor a clearance, and both are gated at their own source +repositories. Measured before the change, the fhir tarball produced exactly one hit, seven bytes of +DEFLATE output matching the email shape, which changes with every repack. **A literal path, never a +predicate; the all route only; `` still reads them.** If `pnpm vendor:refresh` renames one the +gate refuses naming the new path, which is the safe direction and is deliberate. + +**▶ `.git` IS A REGULAR FILE IN A SUBMODULE WORKING TREE**, not a directory, and this repository is +consumed as one. It is skipped by literal name: it is git's own metadata, never tracked, and in a +plain clone it is a directory the walk already skipped, so admitting it made the scan behave +differently in a clone and a submodule for nothing. + +### The HL7 v2 structured pass, and why enumeration alone would have been false confidence + +**▶ ENUMERATING MORE FILES BUYS THE SSN/EMAIL FLOOR AND NOTHING ELSE.** Measured over the 8 tracked +files carrying `PID|` here: **zero dashed SSNs and zero emails between them.** Widening the walk +without a recogniser would have opened 8 files full of names, DOBs, MRNs, one undashed SSN, a street +address and two phone numbers, and reported every one of them **clean**. The two halves ship +together and each is **"in addition to"**, never "instead of": both passes run on every target on +all three routes. + +**▶ AND THE SHAPE THAT MAKES THIS PACKAGE DIFFER FROM ITS PARSER SIBLINGS: THERE IS NO STANDALONE +`.hl7` FIXTURE IN THIS REPOSITORY AT ALL.** Every message in the corpus is a `.ts` **string +literal**, usually one segment per array element. A recogniser written the usual way, treating the +file as the document, would have found nothing in any of them. So the pass locates segment literals +**anywhere in a target's text** and reads each from its segment id to the first CR, LF, double quote +or backtick. A single quote is deliberately NOT a terminator: it occurs in real family names, and +stopping there would scan less. + +It parses **PID / NK1 / GT1 / IN1** by field and component and checks names (XPN 1/2/3), DOB (the +leading 8 digits of a TS), ids (CX-1 across `~` repetitions, with an `SS` type code or a bare +9-digit value named as an SSN), addresses (XAD 1/2/3/5) and phones (XTN components with 4+ digits). +**Four things it does NOT do, stated rather than implied:** a value injected by template +interpolation is skipped rather than guessed at; PROVIDER names in PV1/ORC/OBR XCN fields are out of +scope (a clinician is not the patient, and XCN is a different layout); a single-character middle +initial is below the name-token floor; and a segment written with a non-default field separator is +not recognised. + +**Nothing patient-identifying was found in the 69 files the widening newly opened**, every one of +them hand-read. The fixture values are placeholders and are **named in the allow-list rather than +scrubbed**: `Jane Q. Public` and `Jane Doe`, the mnemonics keyed to their suites (`Appt^Amy`, +`Doc^Dana`, `Imm^Ian`, `Kin^Next`), `MRN1`/`MRN2`/`MRN12345`, an SSN-shaped `999887777` in area +number 999 which the Social Security Administration has never issued, placeholder street lines, and +two numbers in the reserved 555 range. The one org-traceable string is the publisher mailbox above. + +**▶ THE SCANNER'S OWN TEST FILE IS NOW INSIDE THE CORPUS, AND ITS VIOLATOR PAYLOADS ARE ASSEMBLED +FROM PARTS AT RUNTIME.** A live literal there would red the repository's own gate on every run, and +both alternatives are worse: allow-listing it blinds the floor globally and route-blind, and +exempting the file by path leaves the largest violator-bearing file in the tree unscanned. +Assembling keeps every runtime value byte-identical, so no assertion lost bite. **The residual: +nothing gates the convention itself** beyond the gate reddening if someone writes a literal back. **▶ THE RENAME RESIDUAL IS CLOSED, AND THE FRAMING IT WAS FILED UNDER WAS FALSE.** It was disclosed here as "admitting `R`/`C` needs the two-path record shape, a scope decision". **There is no scope decision and no record shape work**: the remedy is `--no-renames` on the `git diff @@ -367,16 +482,19 @@ down as canonical**: across both-modified, add/add, modify/delete, delete/modify symlink/symlink the status is always `U` and the dst mode always `000000`, while the SRC mode (`100644`/`120000`/`000000`) and the set of stages present (1/2/3, 1/2, 2/3, 1/3) both vary. **Three residuals remain disclosed, not closed.** Do not silently re-close any, and do not let a -future edit read as though they were. (1) This scanner has **no refuse-a-scan-that-observed-nothing -rule**, so an empty enumeration reports clean. (2) The ancestor-component and absolute/`../` reads -above, on the named-path mode only. (3) **A scan ROOT'S OWN PATH staged as a non-regular entry is -outside the `--staged` route's scope**, because that scope tests `test/fixtures/` and `src/` as -path PREFIXES and an index entry at exactly `src` matches neither. Measured identically on both +future edit read as though they were. (1) This scanner still has **no refuse-a-scan-that-observed- +nothing rule** on the routes themselves: an empty enumeration reports clean. The all route now has +a partial answer, `reconcileWithGit`, which refuses when the walk opened less than git tracks; that +does **not** cover `--staged` or ``, and it is **vacuous on an empty index**, so the sentence +is narrowed rather than retired. (2) The ancestor-component and absolute/`../` reads above, on the +named-path mode only. (3) **A scan ROOT'S OWN PATH staged as a non-regular entry is outside the +`--staged` route's scope**, because that scope tests the root NAMES as path prefixes and an index +entry at exactly `src` matches neither prefix nor a repo-root file. Measured identically on both trees: `ln -s elsewhere src && git add -A` stages `:000000 120000 0000000 A src` and -`--staged` reports clean / exit **0**, while the all-mode walk over the same tree exits 1 on the -payload behind it. `dicom`'s copy of this function carries exactly that guard -(`s.path === "test/fixtures" || s.path.startsWith("test/fixtures/")`) and **it did not come across -in the port**. Found by this slice's refuter, pre-existing, and its own item. +`--staged` reports clean / exit **0**. The all-mode walk now answers that tree two ways rather than +one: the `lstat` preflight refuses the root outright, and the reconciliation refuses over whatever +was tracked beneath it. `dicom`'s copy of this function carries a guard for the staged half and +**it did not come across in the port**. Pre-existing, and its own item. **Exit **2** now means every failure to complete, not just a bad invocation.** A throw raised before or outside `main`'s inner `try` blocks (`loadAllowList()` on a missing `scripts/phi-allow-list.txt`, `readdirSync` on an unreadable directory under a walk root) left the @@ -386,10 +504,16 @@ of the file is the outermost net and `walk` names an unreadable directory itself throw still prints its stack, because a gate that swallows its own bugs is harder to fix. Pinned in `test/scripts/phi-scan.test.ts` against **throwaway git repos under `os.tmpdir()`**: the scanner roots everything at `process.cwd()`, so never write a link or a violator into this corpus -to test it. **The enumerate-then-read race is deliberately still open here** and is a separate -item: measured on this tree, a real `pnpm build` puts **no** transient under either walk root, and -both temp-using suites `mkdtemp` into `os.tmpdir()`, so it is unreachable by scope, _until a walk -root widens_, which reintroduces it verbatim. +to test it. **The enumerate-then-read race is deliberately still open here**, and the condition that +sentence named as hypothetical has now happened: it said the race was unreachable by scope _until a +walk root widens_, and the roots have widened to the whole tracked corpus. **Re-measured rather than +re-asserted**: `pnpm build` writes only to `dist/` and `coverage/`, both gitignored and neither a +root; the temp-using suites still `mkdtemp` into `os.tmpdir()`; and `dist-artifacts/`, which +`pack:docs` writes, is gitignored too. What DID come into scope is `.changeset/`, where +`pnpm changeset` and `changeset version` create and delete files, so a scan racing a release step +can now enumerate an entry that is gone before it is read. That path exits **2** through +`scanTarget`'s read guard rather than reporting clean, which is the safe direction, but it is a real +new reachability and is recorded as one rather than left to be discovered. ## No internal project bookkeeping on a public surface, in full diff --git a/phi-scan-overrides.md b/phi-scan-overrides.md index 5b5804d..7fe64a8 100644 --- a/phi-scan-overrides.md +++ b/phi-scan-overrides.md @@ -8,12 +8,15 @@ creates an audit trail. Prefer extending `scripts/phi-allow-list.txt` (a token-level, reviewed declaration) over a whole-file bypass, which silences _every_ check for that file. -> **This is the STARTER template.** `scripts/phi-scan.ts` ships with the shared -> machinery and a cross-cutting SSN/email floor ONLY. Before you rely on -> `pnpm phi-scan` as a real PHI gate for this standard, add structured, -> field-level detection (names, DOB, MRN / member id, address, phone) in the -> fenced TODO section of `scripts/phi-scan.ts`: see the sibling parsers -> (`hl7` / `dicom` / `x12` / `ccda` / `ncpdp`) for worked examples. +> **What the scanner detects, so a bypass is judged against the real gate rather +> than a template.** Two passes run on every target, on all three routes: the +> cross-cutting SSN/email floor, and an HL7 v2 structured pass over PID / NK1 / +> GT1 / IN1 checking names, date of birth, MRN / member id / SSN, address and +> phone against `scripts/phi-allow-list.txt`. Segment literals are found inline, +> because this package ships no standalone `.hl7` file: every message in its +> corpus is a `.ts` string literal. What it still does NOT see is written out in +> the banner at the top of `scripts/phi-scan.ts`, and a bypass here silences all +> of it for that path. ## Format diff --git a/scripts/phi-allow-list.txt b/scripts/phi-allow-list.txt index ef52348..95f318f 100644 --- a/scripts/phi-allow-list.txt +++ b/scripts/phi-allow-list.txt @@ -9,14 +9,29 @@ # # Format: ` ` per line. `#` comments and blank lines are ignored. # NAME synthetic person-name word, matched case-insensitively -# DOB synthetic date of birth (normalize as your detector does) +# DOB synthetic date of birth, the leading 8 digits of a v2 TS # ID synthetic id matching an SSN / MRN / member-id shape +# ADDRESS synthetic address component (street, city, postal code) +# PHONE synthetic telephone number, compared on digits only # EMAILDOMAIN reserved / test email domain that is allowed to appear +# EMAIL one allowed mailbox in one file: the narrowest tag here # -# NOTE: the STARTER scanner only actively consumes EMAILDOMAIN (its floor is -# SSN + email shapes). NAME / DOB / ID entries are loaded and ready for the -# structured, field-level detectors you MUST add to scripts/phi-scan.ts before -# relying on this as a real PHI gate: see the STARTER banner in that file. +# ▶ EVERY ENTRY HERE IS ROUTE-BLIND. It clears its literal on the all-mode walk, +# on `pnpm phi-scan `, AND on `--staged`, which is the pre-commit gate. +# So adding one is a subtraction from the commit-blocking route as well, +# whatever the reason for adding it was. There is no route-scoped tag and +# there should not be: a value that is safe on one route and not another is a +# claim nobody can hold in their head. +# +# ▶ AND EVERY TAG BUT ONE IS ALSO FILE-BLIND. `NAME`, `DOB`, `ID`, `ADDRESS`, +# `PHONE` and `EMAILDOMAIN` clear their value in EVERY file the scan reads. +# `EMAIL` is the exception: it takes a literal path as well, so it clears one +# address in one file. Prefer the narrowest tag that covers the case. +# +# ▶ NEVER ADD A VALUE THIS REPOSITORY'S OWN GATE TESTS USE AS A TRIPWIRE. The +# violator payloads in `test/scripts/phi-scan.test.ts` are assembled from +# parts at runtime precisely so they never need an entry here: allow-listing +# one would blind the floor for every corpus at once. # # To add a new synthetic fixture: reuse the tokens below, or append new ones here # and commit. Adding a token is a reviewed, intentional act, never silent. @@ -27,12 +42,81 @@ EMAILDOMAIN example.org EMAILDOMAIN example.net EMAILDOMAIN example.test -# --- Synthetic person-name tokens (example: extend for your fixtures) --- -# NAME DOE -# NAME JOHN +# --- Synthetic person-name tokens --- +# +# Every one is a placeholder in this repository's HL7 v2 fixtures, hand-read +# before it was declared. `Jane Q. Public` and `Jane Doe` are the standard +# anonymous-party placeholders; the rest are mnemonics keyed to the test file +# they appear in (`Appt^Amy` in the appointment suite, `Doc^Dana` in the +# document suite, `Imm^Ian` in the immunization suite, `Kin^Next` for a +# next-of-kin). None names a real person. +NAME PUBLIC +NAME JANE +NAME JOHN +NAME DOE +NAME APPT +NAME AMY +NAME DOC +NAME DANA +NAME IMM +NAME IAN +NAME KIN +NAME NEXT + +# --- Synthetic dates of birth (leading 8 digits of the v2 TS) --- +# +# Round placeholder dates, none of them a real person's. +DOB 19800115 +DOB 19850101 +DOB 19900101 +DOB 20180101 + +# --- Synthetic ids: SSN / MRN / member-id shapes --- +# +# `999887777` is an SSN-shaped value in area number 999, which the Social +# Security Administration has never issued and never will, so it cannot collide +# with a real number. The MRN values are sequence placeholders. +ID MRN1 +ID MRN2 +ID MRN12345 +ID 999887777 + +# --- Synthetic address components --- +# +# Placeholder street lines against a real city and postal code. The combination +# does not identify anyone: no such patient, and no such address record. +ADDRESS 123 MAIN ST +ADDRESS 456 OAK AVE +ADDRESS 1 ST +ADDRESS APT 4 +ADDRESS BOSTON +ADDRESS 02101 -# --- Synthetic dates of birth (example: normalize to your detector's format) --- -# DOB 19800115 +# --- Synthetic telephone numbers --- +# +# The 555 exchange is the reserved fictional range. +PHONE 555-1234 +PHONE 555-9999 -# --- Synthetic ids: SSN / MRN / member-id shapes (example) --- -# ID MRN12345 +# --- Allowed mailboxes: one address, in one file --- +# +# ▶ THIS IS NOT A SCRUB, AND THE ADDRESS IS NAMED HERE ON PURPOSE. The one +# mailbox this repository carries is the npm publisher contact in +# `package.json`'s `author` field. It is public (it is on the package's +# registry page), it is an organisation address rather than a person's, and it +# is not PHI. Deleting it to get the gate green would destroy the evidence +# that this audit looked at it, which is the opposite of what an audit is for. +# +# ▶ AND ITS COST, STATED RATHER THAN LEFT TO BE FOUND. Two literals, so this is +# as narrow as this mechanism goes: every OTHER address at cosyte.com still +# reports, and this SAME address in any other file still reports. What it does +# cost is one measured cell: before this change `pnpm phi-scan package.json` +# exited 1 on that mailbox, and now it exits 0. That is a real subtraction, it +# is the only one in this change, and it is the price of the walk covering +# package.json at all rather than exempting the whole file. +# `test/scripts/phi-scan.test.ts` pins the cell and both narrowness halves. +# The declaration itself carries the literal, so the file holding it needs its +# own entry. Stated rather than special-cased in the scanner: an allow-list that +# silently excused itself would be the one file nobody checks. +EMAIL package.json hello@cosyte.com +EMAIL scripts/phi-allow-list.txt hello@cosyte.com diff --git a/scripts/phi-scan.ts b/scripts/phi-scan.ts index aea3ba4..654b92d 100644 --- a/scripts/phi-scan.ts +++ b/scripts/phi-scan.ts @@ -9,29 +9,47 @@ * accident. * * =========================================================================== - * ██ STARTER: READ BEFORE YOU RELY ON THIS ███████████████████████████████ + * ██ WHAT THIS DETECTS, AND WHAT IT STILL DOES NOT ███████████████████████ * =========================================================================== * - * This file is the SHARED MACHINERY only. As shipped it detects EXACTLY TWO - * cross-cutting PHI shapes that apply to ANY format: + * TWO passes run on EVERY target, on ALL THREE ROUTES. The second is "in + * addition to" the first, never "instead of" it: * - * (1) a dashed Social Security Number (\d{3}-\d{2}-\d{4}) - * (2) an email at a non-test domain + * (1) THE CROSS-CUTTING FLOOR, which applies to any format: + * - a dashed Social Security Number (\d{3}-\d{2}-\d{4}) + * - an email at a non-allow-listed domain or address * - * That is a FLOOR, not a gate. It does NOT understand Transform. It will NOT - * catch a patient name, a date of birth, an MRN / member id, an address, or a - * phone number sitting in a structured Transform field: the PHI that a real - * Transform message actually carries. + * (2) THE HL7 v2 STRUCTURED PASS, which is what this package's corpus + * actually carries. `@cosyte/transform` ships NO standalone `.hl7` + * fixture files at all: every message in the corpus is an inline `.ts` + * STRING LITERAL, so this pass finds segment literals ANYWHERE in a + * target's text rather than assuming the file IS the message. It parses + * PID / NK1 / GT1 / IN1 by field and component and checks each + * PHI-bearing field against the allow-list: + * - person NAMES (XPN family / given / middle) + * - DATE OF BIRTH (TS, leading 8 digits) + * - MRN / member id / SSN (CX repetitions and bare id fields) + * - ADDRESS (XAD street / other designation / city / postal code) + * - PHONE (XTN components carrying 4 or more digits) * - * ⚠ A scanner that silently ships SSN/email-only detection is a FALSE- - * CONFIDENCE RISK: it reports green on fixtures stuffed with real names and - * DOBs. Before you trust `pnpm phi-scan` as a safety gate for Transform, - * YOU MUST add structured, field-level detection for THIS standard's PHI - * (names, DOB, MRN / member id, address, phone) in the clearly-fenced - * TODO section inside `scanTarget` below. + * ⚠ ENUMERATING MORE FILES BUYS THE FLOOR AND NOTHING ELSE. The floor finds + * ZERO in this repository's HL7 fixtures: they carry no dashed SSN and no + * email. Widening the walk without (2) would have opened 8 files full of + * names, DOBs and MRNs and reported every one of them clean. That is the + * false confidence this banner exists to refuse. + * + * ⚠ STILL NOT DETECTED, stated rather than implied: + * - a value injected by TEMPLATE INTERPOLATION (`${…}`) into a segment + * literal. A static text scan cannot see what a placeholder resolves + * to; such a component is skipped rather than guessed at. + * - PROVIDER names in PV1 / ORC / OBR XCN fields. Those identify a + * clinician, not a patient, and the XCN layout differs from XPN; + * declared out of scope rather than half-implemented. + * - a segment written with a NON-DEFAULT field separator. The pass keys + * on `SEG|`, the encoding this corpus and the v2 default both use. * * Worked examples of structured, format-aware detection live in the sibling - * parsers. Read one before you start: + * parsers: * ../hl7/scripts/phi-scan.ts (segment → field → component aware) * ../x12/scripts/phi-scan.ts (ISA-delimited NM1 / DMG / PER aware) * ../dicom/scripts/phi-scan.ts (binary tag-aware) @@ -44,6 +62,12 @@ * `# synthetic: true` header, so the allow-list is the proven substitute * (same approach every sibling uses). A whole-file bypass needs * `--allow-fixture ` AND a logged entry in `phi-scan-overrides.md`. + * + * ▶ AN ALLOW-LIST ENTRY IS GLOBAL AND ROUTE-BLIND. It clears that literal on + * the commit-blocking `--staged` route too, and on ``. Adding one is + * therefore a subtraction from every route at once, and the one entry here + * that subtracts a detection this scanner had BEFORE this change is called + * out by name in `scripts/phi-allow-list.txt`. * =========================================================================== * * Modes: @@ -100,17 +124,55 @@ * the only true thing available: there is an entry here the scan cannot account * for, so the scan is not clean. * - * "In scope" is each route's own existing boundary, not a new one: the walk - * still excludes a gitignored entry (the same rule that already excludes a - * gitignored file, so links do not get a second, stricter boundary of their - * own), and `--staged` still only looks at `test/fixtures/**` and `src/**.ts`. - * This narrows what those scopes ADMIT; it does not widen the scopes. Note that - * `test/fixtures/` does not exist in this repo today, so `src/` is the only - * directory the walk actually descends. The walk has NO extension scope of its - * own (it skips regular `*.md` as documentation and takes everything else), so - * a link at `src/leak.json`, and a linked directory, are refused there too. The - * `.ts` suffix is the `--staged` route's boundary, not the walk's; do not - * describe them as one rule. + * "In scope" is each route's own boundary: the walk still excludes a gitignored + * entry (the same rule that already excludes a gitignored file, so links do not + * get a second, stricter boundary of their own). The refusal narrows what those + * scopes ADMIT; it does not widen the scopes. The walk has NO extension scope of + * its own, so a link at `src/leak.json`, and a linked directory, are refused + * there too. + * + * --------------------------------------------------------------------------- + * THE SCAN SCOPE IS THE TRACKED CORPUS, AND IT IS RECONCILED AGAINST `git + * ls-files` ON EVERY RUN. Both enumerating routes used to cover `test/fixtures/` + * plus `src/`, and MEASURED ON THIS REPOSITORY AT `daf75c3`, THAT WAS 31 OF 102 + * TRACKED FILES: 71 were read by NEITHER ROUTE, 27 of them under `test/`, + * 8 of those carrying inline HL7 `PID|` literals with names, DOBs and MRNs. + * + * ▶ AND THE SHARPEST HALF: `test/fixtures/` HAS NEVER EXISTED IN THIS + * REPOSITORY, ON ANY COMMIT (`git log --all -- 'test/fixtures*'` is empty). The + * walk's `existsSync` guard returned immediately for it on every run this + * scanner has ever made, and the run still printed "OK: no hits" and exited 0. + * A DECLARED ROOT THAT WAS NEVER OPENED IS INDISTINGUISHABLE FROM A CLEAN ONE. + * + * ▶ A COUNT DOES NOT DETECT THAT, AND NEITHER DOES AN EXISTENCE CHECK. "145 + * files scanned" counts the roots that DID exist, and refusing a MISSING root + * still leaves an EMPTIED one reporting clean. The only thing that observes it + * is reconciling what the walk actually OPENED against what git actually + * TRACKS, which `reconcileWithGit` does in all-mode. Every tracked path the walk + * did not open REFUSES (exit 2) and is named. + * + * ▶ WHAT THE RECONCILIATION DOES NOT CLOSE, AND NO REPO IN THIS ECOSYSTEM HAS: + * IT COMPARES PATH SETS, NOT THE BYTES GIT CARRIES AT THOSE PATHS. A root + * swapped for a directory that mirrors the tracked NAMES still reconciles, over + * decoy contents. Widening the roots makes that narrower rather than closed: a + * decoy must now mirror 100 names, not 7. It is also VACUOUS ON AN EMPTY INDEX, + * which is why the suite's throwaway repos still exercise the other guards. + * + * ▶ A WALK ROOT THAT IS NOT A DIRECTORY REFUSES BEFORE THE WALK. `existsSync` + * FOLLOWS a link, so a DANGLING root read false and `walk` returned without + * enumerating anything: measured here, `ln -s /nowhere test/fixtures` printed + * "OK: no hits" and exited 0. A root that is a symlink to a real directory was + * FOLLOWED, reading bytes git does not carry. Both now refuse with 2 via an + * `lstat` per declared root. A root that is simply ABSENT is not an error (a + * tree may legitimately not have one); the reconciliation is what notices if + * anything tracked lived under it. + * + * EXIT CODE FOR A REGULAR-FILE ROOT, DERIVED FROM THIS SCRIPT'S OWN CONTRACT AND + * NOT PORTED FROM A SIBLING: **2**. Measured before this change, `existsSync` + * answered true and `readdirSync` threw `ENOTDIR` into the `walk` catch, which + * raises an `InvocationError` and returns 2 from `main`. It is 2 for the new + * `lstat` preflight too, so the code did not move. + * --------------------------------------------------------------------------- * * THE STAGED ROUTE READS `--raw`, AND ITS `--diff-filter` ADMITS `T`. Replacing * a TRACKED regular file with a link is neither an add nor a modify: measured @@ -171,11 +233,66 @@ const REPO_ROOT = process.cwd(); const ALLOW_LIST_PATH = join(REPO_ROOT, "scripts", "phi-allow-list.txt"); const OVERRIDE_LOG_PATH = join(REPO_ROOT, "phi-scan-overrides.md"); -// Roots walked in "all" mode. test/fixtures gets the full scan; src gets the -// same conservative shape pass because it is hand-written code, not data: -// JSDoc `@example` snippets must not carry real PHI either. -const FIXTURE_ROOT = join(REPO_ROOT, "test", "fixtures"); -const SRC_ROOT = join(REPO_ROOT, "src"); +/** + * Directories walked in "all" mode, plus the repo-root regular files, which are + * enumerated separately by `walkTopLevel` (a root file has no directory to + * declare). Together these cover the whole tracked corpus, which is what + * `reconcileWithGit` then proves on every run. + * + * ▶ ROOTS MUST STAY DISJOINT. `test` covers `test/fixtures` rather than sitting + * beside it: declaring both would report every nested file twice. + * + * ▶ WIDEN BY UNION, NEVER BY REPLACEMENT. Each entry here is "in addition to"; + * the previous list (`test/fixtures` + `src`) is a strict SUBSET of this one, so + * nothing the walk opened before can stop being opened. + * + * ▶ `vendor/` IS DELIBERATELY ABSENT. It holds two `pnpm pack` gzip tarballs of + * sibling packages, and see `RECONCILE_EXEMPT` for why a text scan over gzip + * bytes is neither a detection nor a clearance. + */ +const WALK_ROOT_NAMES = [ + ".changeset", + ".github", + "docs-content", + "documentation", + "scripts", + "src", + "test", +] as const; + +const WALK_ROOTS = WALK_ROOT_NAMES.map((name) => join(REPO_ROOT, name)); + +/** + * The tracked paths `reconcileWithGit` excuses, as LITERAL PATHS. + * + * ▶ THREE RULES, AND EACH WAS PAID FOR ELSEWHERE IN THIS ECOSYSTEM: + * 1. A literal path, NEVER a predicate. A predicate reads as a tidy rule and + * then applies to files nobody enumerated when they wrote it. + * 2. It reaches the ALL route only. `--staged` is the commit-blocking + * pre-commit gate and exempts nothing; `` scans exactly what it is + * handed, so `pnpm phi-scan vendor/` still reads those bytes and + * still reports what it finds. NO DETECTION EITHER ROUTE HAD IS + * SUBTRACTED. + * 3. It is enumerated here in source, so adding one is a reviewed act and a + * diff, never a silently-widening glob. + * + * WHY THESE TWO: they are gzip archives. Their bytes are not the text they + * carry, so scanning them is neither a detection nor a clearance: a name inside + * one is compressed and unreadable to any text pass, and a clean result over + * them would be evidence of nothing. Both are `pnpm pack` outputs of sibling + * `@cosyte/*` repositories, each gated by its own PHI scanner at its own source. + * Measured here before this change, the fhir tarball produced exactly one hit: + * seven bytes of DEFLATE output that happen to match the email shape, and that + * change with every repack. It is written as a shape rather than quoted, + * because this file is inside the scan's own corpus and a quoted violator here + * would red the gate on every run. + * + * ▶ IF `pnpm vendor:refresh` EVER RENAMES A TARBALL, THIS LIST GOES STALE AND + * THE GATE REFUSES (exit 2) NAMING THE NEW PATH. That is the safe direction and + * it is deliberate: the remedy is to update this list, never to loosen it into a + * `vendor/**` pattern. The names are pinned in `scripts/vendor-refresh.sh`. + */ +const RECONCILE_EXEMPT = new Set(["vendor/cosyte-fhir-0.0.0.tgz", "vendor/cosyte-hl7-0.0.0.tgz"]); // --------------------------------------------------------------------------- // Types @@ -189,23 +306,35 @@ interface Hit { } interface AllowList { - /** - * Uppercase synthetic person-name tokens. UNUSED by the starter floor: the - * structured name detector you add in the TODO section consumes these. - */ + /** Uppercase synthetic person-name tokens, consumed by the XPN name check. */ names: Set; - /** - * Synthetic dates of birth (raw, format-normalized as you choose). UNUSED by - * the starter floor: your structured DOB detector consumes these. - */ + /** Synthetic dates of birth, normalized to the leading 8 digits of a v2 TS. */ dobs: Set; - /** - * Synthetic id values (SSN / MRN / member-id shapes). UNUSED by the starter - * floor: your structured id detector consumes these. - */ + /** Uppercase synthetic id values (SSN / MRN / member-id shapes). */ ids: Set; - /** Allowed email domains (anything else is a hit). Used by the starter floor. */ + /** Uppercase synthetic address components (street, city, postal code). */ + addresses: Set; + /** Synthetic phone values, compared on their digits only. */ + phones: Set; + /** Allowed email domains: every address at one of these passes. */ emailDomains: Set; + /** + * Allowed email ADDRESSES, keyed `\0`. + * + * ▶ TWO LITERALS, AND THE PATH HALF IS THE POINT. An `EMAILDOMAIN` entry + * excuses a whole domain everywhere; an unscoped address entry excuses one + * mailbox everywhere. This excuses ONE mailbox in ONE file, which is the + * narrowest instrument this allow-list has. The same address in any other + * file, and any other address in the same file, both still report. + * + * It is still route-blind within that file, which is a property of the + * allow-list rather than of this tag: an entry clears its literal on the walk, + * on ``, and on the commit-blocking `--staged`. That cost is stated in + * `scripts/phi-allow-list.txt` beside the entry and pinned by tests. The + * entries themselves live in that file; quoting one here would put a live + * address in a file the scan reads. + */ + emails: Set; } interface Args { @@ -283,6 +412,15 @@ function parseArgs(argv: string[]): Args { // Allow-list + override log // --------------------------------------------------------------------------- +/** + * The key an `EMAIL` allow-list entry is stored and looked up under. A path and + * an address, joined by a byte neither can contain, so a path ending in the + * address's first characters cannot be confused for a different entry. + */ +function emailKey(path: string, address: string): string { + return `${path}\u0000${address.toLowerCase()}`; +} + function loadAllowList(): AllowList { if (!existsSync(ALLOW_LIST_PATH)) { throw new InvocationError(`allow-list not found at ${ALLOW_LIST_PATH}`); @@ -291,7 +429,10 @@ function loadAllowList(): AllowList { const names = new Set(); const dobs = new Set(); const ids = new Set(); + const addresses = new Set(); + const phones = new Set(); const emailDomains = new Set(); + const emails = new Set(); for (const lineRaw of raw.split(/\r?\n/)) { const line = lineRaw.trim(); if (line.length === 0 || line.startsWith("#")) continue; @@ -310,14 +451,41 @@ function loadAllowList(): AllowList { case "ID": ids.add(value.toUpperCase()); break; + case "ADDRESS": + addresses.add(value.toUpperCase()); + break; + case "PHONE": + phones.add(digitsOf(value)); + break; case "EMAILDOMAIN": emailDomains.add(value.toLowerCase()); break; + case "EMAIL": { + // `EMAIL
`. Both halves are required: an + // entry with no path is REFUSED rather than read as a global clearance, + // because the looser reading is the one that silently excuses a mailbox + // in a file nobody had in mind when they wrote the line. + const gap = value.indexOf(" "); + if (gap < 0) { + throw new InvocationError( + `allow-list: an EMAIL entry needs a path and an address ("EMAIL
"), got: ${value}`, + ); + } + const scope = value.slice(0, gap).trim(); + const address = value.slice(gap + 1).trim(); + if (scope.length === 0 || address.length === 0) { + throw new InvocationError( + `allow-list: an EMAIL entry needs a path and an address ("EMAIL
"), got: ${value}`, + ); + } + emails.add(emailKey(normalizePath(scope), address)); + break; + } default: break; } } - return { names, dobs, ids, emailDomains }; + return { names, dobs, ids, addresses, phones, emailDomains, emails }; } function normalizePath(p: string): string { @@ -421,19 +589,154 @@ function walk(dir: string, out: string[], unscannable: Unscannable[]): void { if (e.isDirectory()) { walk(full, out, unscannable); } else if (e.isFile()) { - // README/markdown docs may legitimately describe violator values; they - // are documentation, not fixtures. - if (e.name.toLowerCase().endsWith(".md")) continue; + // ▶ THE `*.md` SKIP THAT USED TO SIT HERE IS GONE, AND ITS REMOVAL IS + // PURELY ADDITIVE. It dropped 16 tracked markdown files before a byte of + // any of them was read, on the argument that documentation may + // legitimately describe violator values. Two things were wrong with that. + // First, it was an ENUMERATION-time judgement standing in for a + // CONTENT-time one: the allow-list already exists to say "this literal is + // synthetic", by value and under review, which a filename cannot. Second, + // it was never true of the other routes: `pnpm phi-scan notes.md` ran the + // same content passes at base and reported what it found, so the skip made + // the two routes disagree about the same bytes. Measured over this repo's + // tracked corpus, opening all 16 produced ZERO new hits. out.push(full); } else { - // Deliberately NOT subject to the `.md` exemption above. That exemption is - // a judgement about a file whose bytes the walk could have read; a link's - // name is no evidence at all about what is on the other side. unscannable.push({ path: normalizePath(full), kind: entryKind(e) }); } } } +/** + * Enumerate the REGULAR FILES sitting directly at the repository root. They have + * no directory of their own to declare as a walk root, and there are 14 of them + * tracked here (`package.json`, `README.md`, every config file), so leaving them + * out would leave the reconciliation permanently red for no reason. + * + * Directories are skipped rather than descended: the declared roots above own + * that, and descending from here would double-report every file under them. + * Non-regular entries are collected exactly as they are inside a root. + */ +function walkTopLevel(out: string[], unscannable: Unscannable[]): void { + let entries; + try { + entries = readdirSync(REPO_ROOT, { withFileTypes: true }); + } catch (err) { + const code = err instanceof Error && "code" in err ? String(err.code) : "unknown"; + throw new InvocationError( + `refusing the scan: could not read the repository root (${code}). ` + + `The walk cannot vouch for entries it was never able to enumerate.`, + ); + } + for (const e of entries) { + if (e.isDirectory()) continue; + // ▶ `.git` IS A REGULAR FILE IN A SUBMODULE WORKING TREE, not a directory, + // and this repository IS consumed as one. Measured here: without this line + // the walk opened `.git` and read the `gitdir:` pointer inside it. That is + // git's own metadata, not corpus, it is never tracked, and in a plain clone + // it is a directory the branch above already skips, so admitting it made the + // scan's behaviour differ between a clone and a submodule for no gain. A + // LITERAL name, never a dot-file predicate: `.gitignore`, `.npmrc` and + // `.gitattributes` are corpus and stay in scope. + if (e.name === ".git") continue; + const full = join(REPO_ROOT, e.name); + if (e.isFile()) out.push(full); + else unscannable.push({ path: normalizePath(full), kind: entryKind(e) }); + } +} + +/** + * `lstat` each declared root BEFORE walking it, and refuse anything that is not + * a directory. + * + * ▶ THE CASE THIS EXISTS FOR IS A DANGLING LINK, AND `existsSync` IS WHY IT WAS + * INVISIBLE: `existsSync` FOLLOWS, so it answers FALSE for a link pointing at + * nothing, `walk` returned on its first line, and the run printed "OK: no hits" + * and exited 0 with the whole corpus off the disk. Measured on this scanner. + * A root that is a symlink to a REAL directory was the other half: it was + * followed, so the scan read bytes from wherever the link landed and called them + * the corpus. + * + * A root that is simply ABSENT is NOT refused here. A tree may legitimately not + * have one, and refusing existence is not the same as observing content: + * `reconcileWithGit` is what notices that something tracked lived under it. + */ +function refuseNonDirectoryRoots(roots: string[]): void { + const bad: Unscannable[] = []; + for (const root of roots) { + let st; + try { + st = lstatSync(root); + } catch { + continue; // absent, not an error: the reconciliation owns this case + } + if (st.isDirectory()) continue; + bad.push({ + path: normalizePath(root), + kind: st.isFile() ? "a regular file where a directory is declared" : entryKind(st), + }); + } + refuseUnscannable( + bad, + "A declared scan root that is not a directory is a root the walk cannot open, and an " + + "unopened root reads exactly like a clean one.", + "Restore the directory, or remove the root from WALK_ROOT_NAMES in this script.", + ); +} + +/** + * Reconcile what the all-mode walk actually OPENED against what git actually + * TRACKS, and refuse (exit 2) over every tracked path that was not opened. + * + * ▶ EXISTENCE IS NOT OBSERVATION, AND A COUNT CANNOT SUBSTITUTE FOR THIS. A file + * count counts the roots that DID exist, so a healthy-looking total says nothing + * about a root that was never opened. Refusing a MISSING root only covers half + * the failure too, because an EMPTIED one opens nothing and reports clean. + * Comparing the opened set to `git ls-files` is the only check that observes + * either, and it also catches a stale `WALK_ROOT_NAMES` after a directory is + * added. + * + * ▶ WHAT IT DOES NOT DO, STATED RATHER THAN IMPLIED: it compares PATH SETS, not + * the bytes git carries at those paths. A root replaced by a directory that + * mirrors the tracked NAMES reconciles cleanly over decoy contents. It is also + * VACUOUS ON AN EMPTY INDEX: with nothing tracked there is nothing to reconcile + * against, so this proves nothing in a fresh tree, and the walk's own refusals + * are what still hold there. + * + * ▶ NEVER RE-ADD A `tracked.has(...)` PRE-CHECK IN FRONT OF A READ. That inverts + * the direction of the evidence: it would make the walk agree with git by + * construction, at zero firings, while this comment sold it as protection. + */ +function reconcileWithGit(opened: Set): void { + let out: string; + try { + // SECURITY: array-form execFileSync, no shell. + out = execFileSync("git", ["ls-files", "-z"], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + } catch (err) { + throw new InvocationError( + `refusing the scan: could not list tracked files (${err instanceof Error ? err.message : String(err)}). ` + + `Without git's index there is nothing to reconcile the walk against.`, + ); + } + const missing = out + .split("\0") + .filter((p) => p.length > 0) + .filter((p) => !opened.has(p) && !RECONCILE_EXEMPT.has(p)); + if (missing.length === 0) return; + const lines = missing.map((p) => ` - ${p}`).join("\n"); + const noun = missing.length === 1 ? "tracked file was" : "tracked files were"; + throw new InvocationError( + `refusing the scan: ${String(missing.length)} ${noun} never opened by the walk:\n${lines}\n` + + `A declared root that is missing, emptied or replaced looks exactly like a clean one, so ` + + `the walk is reconciled against git's index rather than trusted. ` + + `Add the directory to WALK_ROOT_NAMES in this script, or (if the path genuinely cannot be ` + + `scanned) add that literal path to RECONCILE_EXEMPT with the reason written down.`, + ); +} + /** * Refuse (exit 2) over entries the enumeration reached and cannot scan. EVERY * offender is named, not just the first: a developer who has to re-run the gate @@ -473,8 +776,10 @@ function gitIgnored(paths: string[]): Set { function buildTargetsForAll(): Target[] { const files: string[] = []; const unscannable: Unscannable[] = []; - walk(FIXTURE_ROOT, files, unscannable); - walk(SRC_ROOT, files, unscannable); + + refuseNonDirectoryRoots(WALK_ROOTS); + for (const root of WALK_ROOTS) walk(root, files, unscannable); + walkTopLevel(files, unscannable); // One `git check-ignore` over both lists. An ignored entry is already out of // scope for the file route, so applying the same rule to a link keeps a single @@ -488,9 +793,16 @@ function buildTargetsForAll(): Target[] { "corpus) untrack it and add it to .gitignore.", ); - return files - .filter((abs) => !ignored.has(normalizePath(abs))) - .map((abs) => ({ path: normalizePath(abs), read: () => readFileSync(abs) })); + const opened = files.map(normalizePath).filter((p) => !ignored.has(p)); + + // The reconciliation runs on the OPENED set, after every filter above, so a + // tracked file dropped by any of them is named rather than assumed. + reconcileWithGit(new Set(opened)); + + return opened.map((rel) => ({ + path: rel, + read: () => readFileSync(join(REPO_ROOT, rel)), + })); } /** @@ -547,6 +859,30 @@ function buildTargetsForPaths(paths: string[]): Target[] { return targets; } +/** + * The `--staged` route's path scope, and it is a STRICT SUPERSET of the one it + * replaces. + * + * BEFORE: `test/fixtures/**` OR `src/**.ts`. Both are contained here + * (`test/fixtures/x` is under the `test` root; `src/x.ts` is under the `src` + * root, and the `.ts` suffix requirement is DROPPED rather than kept, so + * `src/leak.json` is admitted too), which is what makes this widening additive: + * nothing the pre-commit hook blocked before can stop being blocked. + * + * ▶ THIS ROUTE EXEMPTS NOTHING. `RECONCILE_EXEMPT` is an all-route concept and + * is deliberately not consulted here: `--staged` IS the commit gate, and a + * corpus exemption that reaches it SUBTRACTS a detection at exactly the moment + * the gate is meant to fire. `vendor/` is simply outside this scope, as it was + * before, so no staged detection changes there either. + * + * A repo-root file (no `/` in its path) is admitted, matching `walkTopLevel`. + */ +function stagedRouteAdmits(path: string): boolean { + if (!path.includes("/")) return true; + const top = path.slice(0, path.indexOf("/")); + return (WALK_ROOT_NAMES as readonly string[]).includes(top); +} + /** git's file modes for a regular blob. Every other mode is not a file to read. */ const REGULAR_BLOB_MODES = new Set(["100644", "100755"]); @@ -661,10 +997,7 @@ function buildTargetsForStaged(): Target[] { i += 2; } - const inScope = staged.filter( - (s) => - s.path.startsWith("test/fixtures/") || (s.path.startsWith("src/") && s.path.endsWith(".ts")), - ); + const inScope = staged.filter((s) => stagedRouteAdmits(s.path)); refuseUnscannable( inScope @@ -695,15 +1028,239 @@ function scanCommonShapes(path: string, content: string, allow: AllowList, hits: for (const m of content.matchAll(/\b\d{3}-\d{2}-\d{4}\b/g)) { hits.push({ path, segment: "(ssn)", value: m[0], reason: "dashed SSN pattern" }); } - // Emails whose domain is not an allow-listed reserved / test domain. + // Emails whose domain is not an allow-listed reserved / test domain, and whose + // full address is not itself allow-listed. The address check is the narrower + // of the two by construction and exists so a single known mailbox can be + // declared without excusing its whole domain. for (const m of content.matchAll(/\b[A-Za-z0-9._%+-]+@([A-Za-z0-9.-]+\.[A-Za-z]{2,})\b/g)) { const domain = (m[1] ?? "").toLowerCase(); - if (!allow.emailDomains.has(domain)) { - hits.push({ path, segment: "(email)", value: m[0], reason: "email with non-test domain" }); + if (allow.emailDomains.has(domain)) continue; + if (allow.emails.has(emailKey(path, m[0]))) continue; + hits.push({ path, segment: "(email)", value: m[0], reason: "email with non-test domain" }); + } +} + +// --------------------------------------------------------------------------- +// The HL7 v2 structured pass: field- and component-level PHI +// --------------------------------------------------------------------------- +// +// ▶ THIS PASS DOES NOT ASSUME THE FILE IS THE MESSAGE, AND THAT IS THE WHOLE +// POINT HERE. `@cosyte/transform` ships no standalone `.hl7` fixture at all: +// every message in its corpus is a `.ts` STRING LITERAL, usually one segment per +// array element. A recogniser written the usual way, parsing a target as a +// document, would find nothing in any of them. So segment literals are located +// ANYWHERE in the text and each is read from its segment id to the end of the +// line or to the closing quote of the literal it sits in, whichever comes first. +// +// ▶ AND THE FLOOR FINDS NOTHING IN THAT CORPUS, WHICH IS WHY THIS PASS SHIPS +// WITH THE WIDER WALK RATHER THAN AFTER IT. Measured over the 8 tracked files +// carrying `PID|`: zero dashed SSNs and zero emails between them. What they +// carry is names, DOBs, MRNs, one undashed SSN in an `SS`-typed identifier, one +// street address and two phone numbers, and the floor is blind to every one. + +/** PHI-bearing fields per segment, by v2 field number (`PID-5` is index 5). */ +const NAME_FIELDS: Record = { + PID: [5, 6, 9], + NK1: [2, 30], + GT1: [3], + IN1: [16], +}; +const DOB_FIELDS: Record = { + PID: [7], + NK1: [16], + GT1: [8], + IN1: [18], +}; +const ID_FIELDS: Record = { + PID: [3, 19, 20], + NK1: [33], + GT1: [12, 19], + IN1: [36], +}; +const ADDRESS_FIELDS: Record = { + PID: [11], + NK1: [4], + GT1: [5], + IN1: [19], +}; +const PHONE_FIELDS: Record = { + PID: [13, 14], + NK1: [5, 6], + GT1: [6, 7], + IN1: [17], +}; + +const PHI_SEGMENTS = Object.keys(NAME_FIELDS); + +/** + * Locate a segment literal by its `SEG|` opening. The leading boundary keeps + * `PID-3` in prose and `xPID|` in an identifier from matching; only a real + * segment id immediately followed by the default field separator qualifies. + */ +const SEGMENT_OPENING = new RegExp(`(?:^|[^A-Za-z0-9])(${PHI_SEGMENTS.join("|")})\\|`, "g"); + +/** Digits only, so `555-1234`, `(555) 1234` and `5551234` compare equal. */ +function digitsOf(value: string): string { + return value.replace(/\D+/g, ""); +} + +/** + * A component the pass declines to judge because its value is not in the text. + * A template placeholder resolves at runtime, and a static scan that guessed at + * one would be fabricating either a hit or a clearance. + */ +function isInterpolated(component: string): boolean { + return component.includes("${"); +} + +/** XPN / XCN family-name components carry `&`-separated subcomponents. */ +function firstSubcomponent(component: string): string { + const amp = component.indexOf("&"); + return amp < 0 ? component : component.slice(0, amp); +} + +/** + * A component plausible as a written person name. Coded values (`CBC^Complete + * Blood Count`), name-type codes (`L`, `ZZ` in XPN-7) and empty components are + * excluded by shape rather than by position, because the position rules above + * already restrict which components are read at all. + */ +function looksLikeNameToken(component: string): boolean { + return /^[A-Za-z][A-Za-z'\-. ]+$/.test(component) && component.trim().length > 1; +} + +function fieldsOf(segment: string): string[] { + return segment.split("|"); +} + +function componentsOf(field: string): string[] { + return field.split("^"); +} + +/** CX and XPN fields repeat on `~`. */ +function repetitionsOf(field: string): string[] { + return field.split("~"); +} + +function checkNameField(ctx: SegmentContext, index: number, field: string): void { + for (const rep of repetitionsOf(field)) { + const comps = componentsOf(rep); + // XPN-1 family, XPN-2 given, XPN-3 middle. Nothing past component 3 is a + // name: XPN-5 is a prefix, XPN-7 a name-type code. + for (const c of [comps[0], comps[1], comps[2]]) { + if (c === undefined || c.length === 0 || isInterpolated(c)) continue; + const token = firstSubcomponent(c); + if (!looksLikeNameToken(token)) continue; + if (ctx.allow.names.has(token.toUpperCase())) continue; + ctx.hit(index, token, "person name not declared synthetic in the allow-list"); + } + } +} + +function checkDobField(ctx: SegmentContext, index: number, field: string): void { + if (field.length === 0 || isInterpolated(field)) return; + const m = /^(\d{8})/.exec(field.trim()); + if (m?.[1] === undefined) return; + if (ctx.allow.dobs.has(m[1])) return; + ctx.hit(index, m[1], "date of birth not declared synthetic in the allow-list"); +} + +function checkIdField(ctx: SegmentContext, index: number, field: string): void { + for (const rep of repetitionsOf(field)) { + if (rep.length === 0 || isInterpolated(rep)) continue; + const comps = componentsOf(rep); + // CX-1 is the id value; CX-4 is the assigning authority and CX-5 the + // identifier type code, neither of which is an identifier. + const value = (comps[0] ?? rep).trim(); + if (value.length === 0 || isInterpolated(value)) continue; + if (ctx.allow.ids.has(value.toUpperCase())) continue; + const typeCode = (comps[4] ?? "").trim().toUpperCase(); + const reason = + typeCode === "SS" || /^\d{9}$/.test(value) + ? "social security number not declared synthetic in the allow-list" + : "patient / member identifier not declared synthetic in the allow-list"; + ctx.hit(index, value, reason); + } +} + +function checkAddressField(ctx: SegmentContext, index: number, field: string): void { + for (const rep of repetitionsOf(field)) { + const comps = componentsOf(rep); + // XAD-1 street, XAD-2 other designation, XAD-3 city, XAD-5 postal code. + // XAD-4 (state) and XAD-6 (country) are not identifying on their own. + for (const c of [comps[0], comps[1], comps[2], comps[4]]) { + if (c === undefined || c.trim().length === 0 || isInterpolated(c)) continue; + const value = firstSubcomponent(c).trim(); + if (value.length === 0) continue; + if (ctx.allow.addresses.has(value.toUpperCase())) continue; + ctx.hit(index, value, "address component not declared synthetic in the allow-list"); } } } +function checkPhoneField(ctx: SegmentContext, index: number, field: string): void { + for (const rep of repetitionsOf(field)) { + for (const c of componentsOf(rep)) { + if (c.length === 0 || isInterpolated(c)) continue; + const digits = digitsOf(c); + if (digits.length < 4) continue; + if (ctx.allow.phones.has(digits)) continue; + ctx.hit(index, c.trim(), "telephone number not declared synthetic in the allow-list"); + } + } +} + +interface SegmentContext { + allow: AllowList; + hit: (fieldIndex: number, value: string, reason: string) => void; +} + +/** + * Extract every PHI-bearing segment literal from `content` and check it field by + * field. + * + * The literal ends at the first CR, LF, double quote or backtick. Those are the + * segment terminator of a real v2 message and the closing delimiters of the two + * TypeScript literal forms this corpus uses. A single quote is deliberately NOT + * a terminator: it appears inside real family names (`O'Brien`), and ending + * there would scan LESS, which is the wrong direction for a gate. + */ +function scanHl7Segments(path: string, content: string, allow: AllowList, hits: Hit[]): void { + for (const opening of content.matchAll(SEGMENT_OPENING)) { + const id = opening[1]; + if (id === undefined) continue; + const start = (opening.index ?? 0) + opening[0].length - id.length - 1; + const rest = content.slice(start); + const end = rest.search(/[\r\n"`]/); + const segment = end < 0 ? rest : rest.slice(0, end); + const fields = fieldsOf(segment); + + const ctx: SegmentContext = { + allow, + hit: (fieldIndex, value, reason) => { + hits.push({ path, segment: `${id}-${String(fieldIndex)}`, value, reason }); + }, + }; + + const run = ( + table: Record, + check: (c: SegmentContext, i: number, f: string) => void, + ): void => { + for (const index of table[id] ?? []) { + const field = fields[index]; + if (field === undefined || field.length === 0) continue; + check(ctx, index, field); + } + }; + + run(NAME_FIELDS, checkNameField); + run(DOB_FIELDS, checkDobField); + run(ID_FIELDS, checkIdField); + run(ADDRESS_FIELDS, checkAddressField); + run(PHONE_FIELDS, checkPhoneField); + } +} + // --------------------------------------------------------------------------- // Dispatch // --------------------------------------------------------------------------- @@ -719,35 +1276,14 @@ function scanTarget(target: Target, allow: AllowList, hits: Hit[]): void { } const text = buf.toString("utf8"); - // The format-agnostic floor: dashed SSN + non-test email. This runs on every - // target and is all the starter detects. + // ▶ TWO PASSES, AND THE SECOND IS "IN ADDITION TO" THE FIRST, NEVER "INSTEAD + // OF" IT. Every target gets both, on every route. The floor is format-blind + // and catches shapes the structured pass never looks for (an SSN in prose, an + // email in a doc comment); the structured pass catches the field-level PHI a + // real v2 message carries, none of which has an SSN or email shape. Making + // either exclusive of the other would open a leak wider than it closed. scanCommonShapes(target.path, text, allow, hits); - - // ── TODO: add Transform-specific structured field-level PHI detection here ── - // - // The floor above ONLY catches SSN/email shapes. Before you rely on this - // scanner as a real safety gate you MUST add structured, field-level - // detection for Transform's PHI, at minimum: person NAMES, DATE OF BIRTH, - // MRN / MEMBER ID, ADDRESS, and PHONE. That means parsing `text` according to the - // Transform wire format and checking each PHI-bearing field against the - // allow-list (`allow.names` / `allow.dobs` / `allow.ids`), pushing a `Hit` - // for anything not positively declared synthetic. - // - // Parse the format properly (delimiters / segments / elements / tags). Do - // NOT bolt on a blind text regex for names: coded values (`CBC^Complete - // Blood Count`, `Boston^MA`) produce false confidence. See the sibling - // parsers named in the STARTER banner at the top of this file for worked, - // spec-aware examples you can adapt: - // - // const d = detectTransformDelimiters(text); // if applicable - // for (const record of splitTransform(text, d)) { - // // check name / dob / id / address / phone fields against `allow` - // // hits.push({ path: target.path, segment: "", value, reason }); - // } - // - // Until this section is implemented, treat a green `pnpm phi-scan` as - // "no SSN/email shapes found", NOT as "no PHI". - // ─────────────────────────────────────────────────────────────────────────── + scanHl7Segments(target.path, text, allow, hits); } // --------------------------------------------------------------------------- diff --git a/test/scripts/phi-scan.test.ts b/test/scripts/phi-scan.test.ts index 92cde7a..e306460 100644 --- a/test/scripts/phi-scan.test.ts +++ b/test/scripts/phi-scan.test.ts @@ -1,12 +1,9 @@ /** - * Unit tests for scripts/phi-scan.ts: the STARTER PHI commit-gate. + * Unit tests for scripts/phi-scan.ts: the PHI commit-gate. * - * These exercise the SHARED MACHINERY and the cross-cutting SSN/email FLOOR that - * ships with the template. They deliberately do NOT test structured, field-level - * PHI detection, that is format-specific and is the author's obligation to add - * (see the STARTER banner in scripts/phi-scan.ts). When you add structured - * detectors, add positive tests here proving they CATCH real-looking names / - * DOBs / ids for this standard: a weak scanner is worse than none. + * These exercise the SHARED MACHINERY, the cross-cutting SSN/email FLOOR, the + * HL7 v2 STRUCTURED PASS, and the reconciliation that proves the walk opened + * what git carries. * * The scanner is invoked via spawnSync (array args, no shell) so the full CLI * path (argv parse, exit code, stderr) is exercised. Violator/clean files are @@ -14,6 +11,30 @@ * * SECURITY: every subprocess call here uses spawnSync with array args. No exec, * no shell-form. + * + * =========================================================================== + * ▶ EVERY VIOLATOR VALUE IN THIS FILE IS ASSEMBLED FROM PARTS AT RUNTIME, AND + * THAT IS LOAD-BEARING RATHER THAN STYLE. + * + * This file is inside the scan's own corpus: `test/` is a walk root, so + * `pnpm phi-scan` reads these bytes on every run. A live dashed-SSN shape, or + * a live name inside a `PID|` literal here, would red the repository's own + * gate permanently, and both ways out of that are worse than assembling the + * value (the shape is named rather than written, for the same reason): + * + * - allow-listing the literal blinds the floor GLOBALLY and ROUTE-BLIND, + * including the `--staged` pre-commit hook, for every corpus at once; + * - exempting this file by path leaves the largest violator-bearing file in + * the tree unscanned. + * + * Assembling keeps the RUNTIME value byte-identical, so every assertion below + * is exactly as strong as it was when these were literals. What changes is + * only what this file's own bytes spell. + * + * ▶ THE RESIDUAL, STATED: nothing gates the convention itself. An editor who + * writes a live literal back into this file will red `pnpm phi-scan`, which + * is the correct direction and is the only enforcement there is. + * =========================================================================== */ import { describe, it, expect, beforeAll, afterAll } from "vitest"; @@ -35,6 +56,25 @@ const REPO_ROOT = process.cwd(); const SCANNER_PATH = join(REPO_ROOT, "scripts", "phi-scan.ts"); const TSX_BIN = join(REPO_ROOT, "node_modules", ".bin", "tsx"); +// --------------------------------------------------------------------------- +// The assembled violator values. See the banner at the top of this file. +// --------------------------------------------------------------------------- + +/** A dashed SSN shape. Never written as one literal in this file. */ +const SSN = ["123", "45", "6789"].join("-"); +/** An email at a domain no allow-list entry covers. */ +const REAL_EMAIL = ["jane.doe", "hospital.org"].join("@"); +/** A second one, at a different uncovered domain, used inside the payload. */ +const PAYLOAD_EMAIL = ["juanita.rivera", "example-hospital.org"].join("@"); +/** Person-name tokens that are NOT in scripts/phi-allow-list.txt. */ +const PAYLOAD_FAMILY = ["RIVE", "RA"].join(""); +const PAYLOAD_GIVEN = ["JUAN", "ITA"].join(""); +/** A date of birth in ISO shape, and the v2 TS shape of the same day. */ +const PAYLOAD_DOB_ISO = ["1978", "03", "14"].join("-"); +const PAYLOAD_DOB_V2 = ["1978", "03", "14"].join(""); +/** An MRN-shaped id no allow-list entry covers. */ +const PAYLOAD_MRN = ["MRN", "77321"].join(""); + let dir: string; interface RunResult { @@ -69,16 +109,16 @@ afterAll(() => { describe("phi-scan starter: the cross-cutting floor catches SSN + email", () => { it("catches a dashed SSN (exit 1)", () => { - const r = scan("ssn.txt", "patient ssn 123-45-6789 on file\n"); + const r = scan("ssn.txt", `patient ssn ${SSN} on file\n`); expect(r.code, `stderr: ${r.stderr}`).toBe(1); - expect(r.stderr).toMatch(/123-45-6789/); + expect(r.stderr).toContain(SSN); expect(r.stderr).toMatch(/dashed SSN/); }); it("catches an email at a non-test domain (exit 1)", () => { - const r = scan("email.txt", "contact jane.doe@hospital.org for records\n"); + const r = scan("email.txt", `contact ${REAL_EMAIL} for records\n`); expect(r.code, `stderr: ${r.stderr}`).toBe(1); - expect(r.stderr).toMatch(/jane\.doe@hospital\.org/); + expect(r.stderr).toContain(REAL_EMAIL); expect(r.stderr).toMatch(/non-test domain/); }); }); @@ -133,22 +173,22 @@ describe("phi-scan starter: the override-log gate", () => { */ const SYNTHETIC_PHI = [ - "Patient: RIVERA^JUANITA^Q", - "DOB: 1978-03-14", - "SSN: 123-45-6789", - "Contact: juanita.rivera@example-hospital.org", + `Patient: ${PAYLOAD_FAMILY}^${PAYLOAD_GIVEN}^Q`, + `DOB: ${PAYLOAD_DOB_ISO}`, + `SSN: ${SSN}`, + `Contact: ${PAYLOAD_EMAIL}`, ].join("\n") + "\n"; /** The link target's own name carries a synthetic name, so an echo of it is visible. */ -const TARGET_NAME = "RIVERA-JUANITA-1978-03-14.txt"; +const TARGET_NAME = `${PAYLOAD_FAMILY}-${PAYLOAD_GIVEN}-${PAYLOAD_DOB_ISO}.txt`; /** Tokens that must never appear in a refusal message. */ const PHI_TOKENS = [ - "RIVERA", - "JUANITA", - "1978-03-14", - "123-45-6789", - "juanita.rivera@example-hospital.org", + PAYLOAD_FAMILY, + PAYLOAD_GIVEN, + PAYLOAD_DOB_ISO, + SSN, + PAYLOAD_EMAIL, TARGET_NAME, ]; @@ -207,8 +247,8 @@ describe("phi-scan: the synthetic payload is genuinely detectable", () => { writeFileSync(join(root, "src", "violator.ts"), SYNTHETIC_PHI); const r = runIn(root, []); expect(r.code, `stderr: ${r.stderr}`).toBe(1); - expect(r.stderr).toContain("123-45-6789"); - expect(r.stderr).toContain("juanita.rivera@example-hospital.org"); + expect(r.stderr).toContain(SSN); + expect(r.stderr).toContain(PAYLOAD_EMAIL); }); it("a repo with no link and no violator scans clean (exit 0)", () => { @@ -271,23 +311,34 @@ describe("phi-scan: the all-mode walk refuses a non-regular entry", () => { expectNoPhi(r.stderr); }); - it("the .md exemption does not extend to a link that merely ends in .md", () => { - // A regular `.md` file is skipped as documentation. That is a judgement about - // bytes the walk could have read; a link's NAME is no evidence about what is - // on the other side, so the exemption must not carry over to one. - const root = makeRepo(); - writeFileSync(join(root, TARGET_NAME), SYNTHETIC_PHI); - symlinkSync(join("..", TARGET_NAME), join(root, "src", "notes.md")); - - const r = runIn(root, []); + it("a link at a .md path is refused, and a REGULAR .md is now READ rather than skipped", () => { + // The walk used to skip a regular `*.md` before reading a byte of it, on the + // argument that documentation may legitimately describe violator values. + // That exemption is GONE, and this case pins both halves of its removal: + // a link ending in `.md` is still refused (its NAME is no evidence about + // what is on the other side), and a REGULAR `.md` full of the payload is now + // a hit rather than a silent pass. Red before the change on the second half: + // the same file exited 0. + const linked = makeRepo(); + writeFileSync(join(linked, TARGET_NAME), SYNTHETIC_PHI); + symlinkSync(join("..", TARGET_NAME), join(linked, "src", "notes.md")); + + const r = runIn(linked, []); expect(r.code, `stderr: ${r.stderr}`).toBe(2); expect(r.stderr).toContain("src/notes.md"); expectNoPhi(r.stderr); + + const regular = makeRepo(); + writeFileSync(join(regular, "src", "notes.md"), SYNTHETIC_PHI); + const rr = runIn(regular, []); + expect(rr.code, `stderr: ${rr.stderr}`).toBe(1); + expect(rr.stderr).toContain("src/notes.md"); + expect(rr.stderr).toContain(SSN); }); it("has no extension scope of its own: a link at a non-.ts path is refused too", () => { - // `src/**.ts` is the `--staged` route's boundary, NOT the walk's. The walk - // skips regular `*.md` as documentation and takes everything else. + // `src/**.ts` was the `--staged` route's boundary, never the walk's. The + // walk takes every regular file under a root, whatever it is named. const root = makeRepo(); writeFileSync(join(root, TARGET_NAME), SYNTHETIC_PHI); symlinkSync(join("..", TARGET_NAME), join(root, "src", "leak.json")); @@ -302,7 +353,11 @@ describe("phi-scan: the all-mode walk refuses a non-regular entry", () => { const root = makeRepo(); writeFileSync(join(root, TARGET_NAME), SYNTHETIC_PHI); symlinkSync(join("..", TARGET_NAME), join(root, "src", "leak.ts")); - writeFileSync(join(root, ".gitignore"), "src/leak.ts\n"); + // The payload itself is ignored too, and that line is not decoration: the + // walk now enumerates repo-ROOT regular files, so the payload sitting beside + // the link is in scope on its own merits and would report a hit of its own. + // Ignoring both is what leaves this case testing the link and nothing else. + writeFileSync(join(root, ".gitignore"), `src/leak.ts\n/${TARGET_NAME}\n`); const r = runIn(root, []); expect(r.code, `stderr: ${r.stderr}`).toBe(0); @@ -373,7 +428,7 @@ describe("phi-scan: the named-path route refuses a non-regular path it is handed writeFileSync(join(root, "src", "violator.ts"), SYNTHETIC_PHI); const hit = runIn(root, ["src/violator.ts"]); expect(hit.code, `stderr: ${hit.stderr}`).toBe(1); - expect(hit.stderr).toContain("123-45-6789"); + expect(hit.stderr).toContain(SSN); const missing = runIn(root, ["src/nope.ts"]); expect(missing.code).toBe(2); @@ -393,7 +448,7 @@ describe("phi-scan: the --staged route refuses a staged non-regular entry", () = expect(gitOut(root, ["ls-files", "--stage", "src/leak.ts"])).toMatch(/^120000 /); const shown = gitOut(root, ["show", ":src/leak.ts"]); expect(shown.trim()).toBe(`../${TARGET_NAME}`); - expect(shown).not.toContain("123-45-6789"); + expect(shown).not.toContain(SSN); }); it("refuses a staged symlink (exit 2), and reports no PHI", () => { @@ -446,7 +501,7 @@ describe("phi-scan: the --staged route refuses a staged non-regular entry", () = const r = runIn(root, ["--staged"]); expect(r.code, `stderr: ${r.stderr}`).toBe(1); - expect(r.stderr).toContain("123-45-6789"); + expect(r.stderr).toContain(SSN); }); it("refuses a staged gitlink under a scanned prefix (exit 2)", () => { @@ -476,7 +531,7 @@ describe("phi-scan: the --staged route refuses a staged non-regular entry", () = const r = runIn(root, ["--staged"]); expect(r.code, `stderr: ${r.stderr}`).toBe(1); expect(r.stderr).toContain("src/violator.ts"); - expect(r.stderr).toContain("123-45-6789"); + expect(r.stderr).toContain(SSN); }); it("reads every record of a multi-file stage, not just the first", () => { @@ -549,20 +604,218 @@ describe("phi-scan: the --staged route refuses a staged non-regular entry", () = expectNoPhi(r.stderr); }); - it("a staged link OUTSIDE the route's scope is left alone (the scope is unchanged)", () => { - // `--staged` only ever covered `test/fixtures/**` and `src/**.ts`. The mode - // check narrows what that scope admits; it does not widen the scope, and - // saying otherwise would overstate what this closes. + it("a staged link OUTSIDE the route's scope is still left alone (the scope is bounded)", () => { + // The scope widened, so it needs a path that is genuinely outside it. `lib/` + // is not a declared root, and a case that could not distinguish "in scope" + // from "out of scope" would be evidence for neither. const root = makeRepo(); + mkdirSync(join(root, "lib")); writeFileSync(join(root, TARGET_NAME), SYNTHETIC_PHI); - symlinkSync(TARGET_NAME, join(root, "docs-link.txt")); - git(root, ["add", "docs-link.txt"]); + symlinkSync(join("..", TARGET_NAME), join(root, "lib", "docs-link.txt")); + git(root, ["add", "lib/docs-link.txt"]); const r = runIn(root, ["--staged"]); expect(r.code, `stderr: ${r.stderr}`).toBe(0); }); }); +// --------------------------------------------------------------------------- +// The widened scope: the tracked corpus, reconciled against `git ls-files` +// --------------------------------------------------------------------------- +// +// Measured on this repository at `daf75c3`, both enumerating routes covered +// `test/fixtures/` + `src/**.ts`, which was 31 of 102 tracked files: 71 read by +// NEITHER route, 27 of them under `test/`, 8 of those carrying inline `PID|` +// literals. And `test/fixtures/` HAS NEVER EXISTED on any commit here, so the +// walk's `existsSync` guard returned on its first line for that root on every +// run this scanner has ever made, while the run reported clean. + +describe("phi-scan: the walk covers the tracked corpus, in addition to what it covered", () => { + it("SUPERSET CONTROL: everything the old scope opened is still opened", () => { + // The widening must only ever ADD. `test` contains `test/fixtures`, and + // `src` dropped its `.ts` restriction, so both previous scopes are strictly + // inside the new one. A violator at each old location still reports. + const root = makeRepo(); + writeFileSync(join(root, "src", "violator.ts"), SYNTHETIC_PHI); + writeFileSync(join(root, "test", "fixtures", "violator.hl7"), SYNTHETIC_PHI); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("src/violator.ts"); + expect(r.stderr).toContain("test/fixtures/violator.hl7"); + }); + + it("reads a tracked file under test/ that is NOT under test/fixtures/ (the class)", () => { + // The population this whole change exists for: 27 tracked files here sat in + // exactly this position and were read by neither route. Red before: this + // file exited 0. + const root = makeRepo(); + mkdirSync(join(root, "test", "messages"), { recursive: true }); + writeFileSync(join(root, "test", "messages", "case.test.ts"), SYNTHETIC_PHI); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("test/messages/case.test.ts"); + }); + + it("reads a repo-ROOT regular file, which has no directory to declare as a root", () => { + const root = makeRepo(); + writeFileSync(join(root, "NOTES.txt"), SYNTHETIC_PHI); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("NOTES.txt"); + }); + + it("REFUSES when a tracked file was never opened by the walk (exit 2, named)", () => { + // Existence is not observation, and a count cannot substitute: a count + // counts the roots that DID exist. This reconciles the OPENED set against + // `git ls-files`, so a tracked path outside every root is named rather than + // quietly absent from a clean report. + const root = makeRepo(); + mkdirSync(join(root, "lib")); + writeFileSync(join(root, "lib", "stray.ts"), "export const a = 1;\n"); + git(root, ["add", "lib/stray.ts"]); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("lib/stray.ts"); + expect(r.stderr).toContain("never opened"); + expect(r.stdout).not.toMatch(/OK/); + }); + + it("REFUSES an EMPTIED root, which existence checks and counts both miss", () => { + // The half a missing-root check does not cover. The directory is still + // there and still walkable; its tracked contents are simply gone from disk, + // so the walk opens nothing and every count looks healthy. + const root = makeRepo(); + writeFileSync(join(root, "src", "kept.ts"), "export const a = 1;\n"); + git(root, ["add", "src/kept.ts"]); + rmSync(join(root, "src", "kept.ts")); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("src/kept.ts"); + }); + + it("the reconciliation is VACUOUS on an empty index, and says nothing either way", () => { + // Stated rather than implied: with nothing tracked there is nothing to + // reconcile against, so a clean run here rests entirely on the walk's own + // refusals. Every throwaway repo above is in this state. + const root = makeRepo(); + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + expect(r.stdout).toMatch(/OK: no hits/); + }); + + it("REFUSES a DANGLING walk root instead of reporting clean over it (exit 2)", () => { + // `existsSync` FOLLOWS, so it answered false here and `walk` returned on its + // first line with the corpus off the disk. Measured before this change: + // "OK: no hits", exit 0. + const root = makeRepo(); + rmSync(join(root, "test"), { recursive: true, force: true }); + symlinkSync(join(root, "nowhere-at-all"), join(root, "test")); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("test"); + expect(r.stderr).toContain("a symbolic link"); + }); + + it("REFUSES a walk root that is a symlink to a real directory (it used to be FOLLOWED)", () => { + const root = makeRepo(); + mkdirSync(join(root, "elsewhere")); + writeFileSync(join(root, "elsewhere", "payload.txt"), SYNTHETIC_PHI); + rmSync(join(root, "test"), { recursive: true, force: true }); + symlinkSync(join(root, "elsewhere"), join(root, "test")); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("a symbolic link"); + expectNoPhi(r.stderr); + }); + + it("REFUSES a walk root that is a regular file, and that is exit 2 here", () => { + // The per-repo exit code, derived from this script's own contract rather + // than ported: `existsSync` answers true, `readdirSync` throws `ENOTDIR` + // into `walk`'s catch, and an InvocationError returns 2. The `lstat` + // preflight now answers first and returns the same 2. + const root = makeRepo(); + rmSync(join(root, "test"), { recursive: true, force: true }); + writeFileSync(join(root, "test"), "not a directory\n"); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("test"); + }); + + it("an ABSENT root is not an error on its own (a tree may legitimately lack one)", () => { + const root = makeRepo(); + rmSync(join(root, "test"), { recursive: true, force: true }); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + }); +}); + +describe("phi-scan: --staged widened by union, and it exempts nothing", () => { + it("blocks a staged violator under test/ outside test/fixtures/ (a new 0 -> 1)", () => { + const root = makeRepo(); + mkdirSync(join(root, "test", "messages"), { recursive: true }); + writeFileSync(join(root, "test", "messages", "case.test.ts"), SYNTHETIC_PHI); + git(root, ["add", "test/messages/case.test.ts"]); + + const r = runIn(root, ["--staged"]); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("test/messages/case.test.ts"); + }); + + it("blocks a staged violator at a non-.ts path under src/ (the suffix bound is gone)", () => { + const root = makeRepo(); + writeFileSync(join(root, "src", "leak.json"), SYNTHETIC_PHI); + git(root, ["add", "src/leak.json"]); + + const r = runIn(root, ["--staged"]); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("src/leak.json"); + }); + + it("blocks a staged violator at the repository root", () => { + const root = makeRepo(); + writeFileSync(join(root, "NOTES.txt"), SYNTHETIC_PHI); + git(root, ["add", "NOTES.txt"]); + + const r = runIn(root, ["--staged"]); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("NOTES.txt"); + }); + + it("the all-route exemption list does NOT reach --staged, nor the named-path route", () => { + // The rule a sibling paid an INTRODUCED major for: an exemption that + // reaches the commit-blocking route SUBTRACTS a detection the base had. + // `vendor/` is excused by the reconciliation only. Staging one of those + // literal paths still blocks, and naming it still scans it. + const root = makeRepo(); + mkdirSync(join(root, "vendor")); + const tarball = join(root, "vendor", "cosyte-fhir-0.0.0.tgz"); + writeFileSync(tarball, SYNTHETIC_PHI); + + // `vendor/` is outside the staged route's scope exactly as it was at base, + // so this is unchanged rather than newly exempt. + git(root, ["add", "vendor/cosyte-fhir-0.0.0.tgz"]); + expect(runIn(root, ["--staged"]).code).toBe(0); + + // But the named-path route reads it, and reports what it finds. + const named = runIn(root, ["vendor/cosyte-fhir-0.0.0.tgz"]); + expect(named.code, `stderr: ${named.stderr}`).toBe(1); + expect(named.stderr).toContain(SSN); + + // And the reconciliation excuses it rather than refusing over it. + const all = runIn(root, []); + expect(all.code, `stderr: ${all.stderr}`).toBe(0); + }); +}); + // --------------------------------------------------------------------------- // A staged RENAME into a scan root // --------------------------------------------------------------------------- @@ -635,7 +888,7 @@ describe("phi-scan: the --staged route enumerates a staged rename", () => { const r = runIn(root, ["--staged"]); expect(r.code, `stderr: ${r.stderr}`).toBe(1); expect(r.stderr).toContain("src/payload.ts"); - expect(r.stderr).toContain("123-45-6789"); + expect(r.stderr).toContain(SSN); }); it("holds whatever the caller's rename/copy detection is configured to", () => { @@ -690,7 +943,7 @@ describe("phi-scan: the --staged route enumerates a staged rename", () => { const r = runIn(root, ["--staged"]); expect(r.code, `stderr: ${r.stderr}`).toBe(1); expect(r.stderr).toContain("src/violator.ts"); - expect(r.stderr).toContain("123-45-6789"); + expect(r.stderr).toContain(SSN); }); }); @@ -732,3 +985,219 @@ describe("phi-scan: a scan that cannot run exits 2, not 1", () => { } }); }); + +// --------------------------------------------------------------------------- +// The HL7 v2 structured pass +// --------------------------------------------------------------------------- +// +// ▶ ENUMERATING MORE FILES BUYS THE SSN/EMAIL FLOOR AND NOTHING ELSE, AND IN +// THIS REPOSITORY THE FLOOR FINDS NOTHING IN THE FIXTURES AT ALL. Measured +// over the 8 tracked files carrying `PID|`: zero dashed SSNs, zero emails. +// What they carry is names, DOBs, MRNs, one undashed SSN in an `SS`-typed +// identifier, one street address and two phone numbers. Widening the walk +// without this pass would have opened all 8 and reported every one clean. +// +// ▶ AND THE SHAPE THAT MAKES THIS PACKAGE DIFFERENT FROM ITS SIBLINGS: there +// is no standalone `.hl7` fixture in this repository. Every message is a +// `.ts` STRING LITERAL, so a recogniser that assumed the file IS the message +// would find nothing. These cases pin that the pass finds segments inline. + +describe("phi-scan: the HL7 v2 structured pass finds field-level PHI", () => { + const pid = (fields: string): string => `PID|${fields}`; + + /** Place values at their 1-indexed v2 field positions, so a case cannot be off by one. */ + const seg = (name: string, fields: Readonly>): string => { + const max = Math.max(0, ...Object.keys(fields).map(Number)); + const parts = [name]; + for (let i = 1; i <= max; i += 1) parts.push(fields[i] ?? ""); + return parts.join("|"); + }; + + it("catches a person NAME in PID-5 that is not declared synthetic (exit 1)", () => { + const r = scan( + "name.ts", + `const P = "${pid(`1||X^^^H^MR||${PAYLOAD_FAMILY}^${PAYLOAD_GIVEN}`)}";\n`, + ); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("PID-5"); + expect(r.stderr).toContain(PAYLOAD_FAMILY); + }); + + it("catches a DATE OF BIRTH in PID-7, and normalizes a zoned TS to its 8 digits", () => { + const r = scan("dob.ts", `const P = "${pid(`1||||A^B||${PAYLOAD_DOB_V2}143000-0500|F`)}";\n`); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("PID-7"); + expect(r.stderr).toContain(PAYLOAD_DOB_V2); + }); + + it("catches an MRN in PID-3, reading CX-1 and not the assigning authority or type", () => { + const r = scan("mrn.ts", `const P = "${pid(`1||${PAYLOAD_MRN}^^^HOSP^MR`)}";\n`); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("PID-3"); + expect(r.stderr).toContain(PAYLOAD_MRN); + // HOSP (CX-4) and MR (CX-5) are not identifiers and must not be reported. + expect(r.stderr).not.toContain('value="HOSP"'); + expect(r.stderr).not.toContain('value="MR"'); + }); + + it("names an SS-typed identifier as an SSN, which the dashed-SSN floor cannot see", () => { + // The exact shape this repository's own ADT fixture carries. It has no + // dashes, so the floor is structurally blind to it. + const undashed = ["555", "44", "3210"].join(""); + const r = scan("ssn-cx.ts", `const P = "${pid(`1||${undashed}^^^SSA^SS`)}";\n`); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("social security number"); + // Non-vacuity: the floor really does miss it on its own. + const floorOnly = scan("ssn-plain.txt", `${undashed}\n`); + expect(floorOnly.code, `stderr: ${floorOnly.stderr}`).toBe(0); + }); + + it("catches an ADDRESS in PID-11 and a PHONE in PID-13", () => { + const street = ["9", "Nowhere", "Terrace"].join(" "); + const phone = ["617", "0000"].join("-"); + const segment = seg("PID", { + 1: "1", + 11: `${street}^^Springfield^ZZ^99999`, + 13: phone, + }); + const r = scan("addr.ts", `const P = "${segment}";\n`); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("PID-11"); + expect(r.stderr).toContain("PID-13"); + expect(r.stderr).toContain(street); + }); + + it("reads NK1, GT1 and IN1 too, not PID alone", () => { + const name = `${PAYLOAD_FAMILY}^${PAYLOAD_GIVEN}`; + + const nk1 = scan("nk1.ts", `const P = "${seg("NK1", { 1: "1", 2: name, 3: "SPO" })}";\n`); + expect(nk1.code, `stderr: ${nk1.stderr}`).toBe(1); + expect(nk1.stderr).toContain("NK1-2"); + + const gt1 = scan("gt1.ts", `const P = "${seg("GT1", { 1: "1", 3: name })}";\n`); + expect(gt1.code, `stderr: ${gt1.stderr}`).toBe(1); + expect(gt1.stderr).toContain("GT1-3"); + + const in1 = scan("in1.ts", `const P = "${seg("IN1", { 1: "1", 16: name })}";\n`); + expect(in1.code, `stderr: ${in1.stderr}`).toBe(1); + expect(in1.stderr).toContain("IN1-16"); + }); + + it("finds a segment INSIDE a TypeScript literal and stops at the closing quote", () => { + // The shape this whole repository's corpus has. The trailing code after the + // closing quote must not be read as further fields. + const content = `const lines = ["MSH|^~\\\\&|A|B|C|D|20260101||ADT^A01|M1|P|2.5.1", "${pid( + `1||||${PAYLOAD_FAMILY}^${PAYLOAD_GIVEN}`, + )}"]; // ${PAYLOAD_MRN} in a comment\n`; + const r = scan("inline.ts", content); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("PID-5"); + // The comment after the literal is outside the segment: it is not a field. + expect(r.stderr).not.toContain(PAYLOAD_MRN); + }); + + it("NEGATIVE CONTROL: an allow-listed fixture line is clean, so the pass is not a blind regex", () => { + // Everything here is declared in scripts/phi-allow-list.txt. A pass that + // reported this would be unusable, and a pass that reported nothing at all + // would look identical to a broken one, which is why every case above sits + // beside this one. + const r = scan( + "declared.ts", + `const P = "PID|1||MRN1^^^HOSP^MR||Doe^Jane||19900101|F|||123 Main St^Apt 4^Boston^MA^02101|||555-1234";\n`, + ); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + }); + + it("NEGATIVE CONTROL: a coded value in a scanned field is not read as a name", () => { + // `CBC^Complete Blood Count` and `Boston^MA` are the false-confidence shapes + // the docblock warns about. Component positions and a name charset keep + // name-type codes (`L`, `ZZ`) and prefixes (`Mrs.`) out. + const r = scan("coded.ts", `const P = "PID|1||||A^B^^^Mrs.^^L||19900101|F";\n`); + expect(r.stderr).not.toContain('value="L"'); + expect(r.stderr).not.toContain('value="Mrs."'); + }); + + it("NEGATIVE CONTROL against the WRONG standard: an X12 NM1 segment is not an HL7 one", () => { + // This package transforms HL7 v2. A pass that fired on a sibling standard's + // wire format would be matching text rather than parsing a message, and the + // measurement would not be about this repository at all. + const r = scan( + "x12.txt", + `NM1*IL*1*${PAYLOAD_FAMILY}*${PAYLOAD_GIVEN}****MI*${PAYLOAD_MRN}~\n`, + ); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + }); + + it("declines to judge a value injected by TEMPLATE INTERPOLATION, rather than guessing", () => { + // A static scan cannot see what a placeholder resolves to. Reporting the + // placeholder text would be a fabricated hit; reporting nothing about it is + // a stated blind spot rather than a silent one. + const r = scan("interp.ts", "const P = `PID|1||${mrn}^^^HOSP^MR||${family}^${given}`;\n"); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + }); + + it("runs IN ADDITION TO the floor on the same target, never instead of it", () => { + const r = scan( + "both.ts", + `// contact ${REAL_EMAIL}\nconst P = "PID|1||||${PAYLOAD_FAMILY}^${PAYLOAD_GIVEN}";\n`, + ); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("(email)"); + expect(r.stderr).toContain("PID-5"); + }); +}); + +describe("phi-scan: the EMAIL allow-list entry, and exactly how far it reaches", () => { + const MAILBOX = ["hello", "cosyte.com"].join("@"); + + it("clears the declared mailbox IN THE DECLARED FILE, and that is the measured cost", () => { + // ▶ THE ONE CELL THIS CHANGE SUBTRACTS, PINNED SO IT IS VISIBLE RATHER THAN + // DISCOVERED. Before this change `pnpm phi-scan package.json` exited 1 on + // the npm publisher contact in its `author` field. It exits 0 now. That is + // the price of the walk covering package.json at all instead of exempting + // the whole file, and it is the only 1 -> 0 in the change. + const root = makeRepo(); + writeFileSync(join(root, "package.json"), `{ "author": "Cosyte <${MAILBOX}>" }\n`); + expect(runIn(root, ["package.json"]).code).toBe(0); + }); + + it("does NOT clear the SAME mailbox in a DIFFERENT file (the path half is real)", () => { + // Two literals, so this entry is as narrow as the mechanism goes. If this + // ever passes, someone has dropped the path and made it global. + const root = makeRepo(); + writeFileSync(join(root, "src", "contact.ts"), `// ${MAILBOX}\nexport const a = 1;\n`); + const r = runIn(root, ["src/contact.ts"]); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain(MAILBOX); + }); + + it("does NOT clear a DIFFERENT mailbox at the same domain (it is not a domain entry)", () => { + // If this ever passes, someone has replaced it with `EMAILDOMAIN cosyte.com`. + const other = ["not-the-publisher", "cosyte.com"].join("@"); + const root = makeRepo(); + writeFileSync(join(root, "package.json"), `{ "author": "${other}" }\n`); + const r = runIn(root, ["package.json"]); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain(other); + }); + + it("IS ROUTE-BLIND within that file, and that reach is pinned rather than assumed", () => { + // An allow-list entry clears its literal on every route, the commit-blocking + // one included. This case exists so that fact is measured and visible. + const root = makeRepo(); + writeFileSync(join(root, "package.json"), `{ "author": "Cosyte <${MAILBOX}>" }\n`); + git(root, ["add", "package.json"]); + expect(runIn(root, ["--staged"]).code).toBe(0); + expect(runIn(root, []).code).toBe(0); + }); + + it("REFUSES an EMAIL entry with no path rather than reading it as a global clearance", () => { + const root = makeRepo(); + const listPath = join(root, "scripts", "phi-allow-list.txt"); + writeFileSync(listPath, `EMAIL ${MAILBOX}\n`); + + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("EMAIL entry needs a path"); + }); +}); From 689931e0cb57f89ac6dae0603bdf2a1cf29ff414 Mon Sep 17 00:00:00 2001 From: Noah Schatz Date: Sat, 8 Aug 2026 12:47:13 +0000 Subject: [PATCH 2/4] fix(phi-scan): close two silent misses the refuter measured, correct IN1-17 and the published figures (PHI-SCAN-WALK-ROOT-SCOPE) Pass 1 REFUTED with three INTRODUCED findings. Two of them were reachable silent misses reported CLEAN, so they are FIXED rather than disclosed; the third was a wrong spec clause. Every figure is re-derived after the changeset existed, which is what made the first set off by one. 1. A NAME COMPONENT OUTSIDE ASCII reported clean. `looksLikeNameToken` was `[A-Za-z]`, so an unaccented name hit while the same name with its accent exited 0, as did every name in a non-Latin script: a gate blind to exactly the names least likely to be synthetic. It is a Unicode letter class now, which still excludes digits so a coded value stays out. Purely additive. 2. A WHOLE MESSAGE IN ONE LITERAL WITH ESCAPED SEPARATORS was never located, because the character before `PID|` is the letter `r` of the escape and the boundary class rejected it. Measured before: an ADT carrying a name, a DOB, an MRN, an address and two phones scanned clean at exit 0, while the same message one segment per array element produced 8 hits. That is the other way a `.ts` file carries a message here, and the shape `parseHL7(raw)` consumes. The escaped separator is now a boundary AND a terminator, and the terminator half is load-bearing: without it the message is read as ONE segment and the next-of-kin's relationship code is reported as the patient's PID-11 address. 3. IN1-17 WAS MAPPED AS A TELEPHONE FIELD. HL7 v2.5.1 defines it as Insured's Relationship To Patient, so a SNOMED relationship code was reported as a phone number and the remedy it steered a developer toward was a global `PHONE` clearance of that digit string. IN1 carries no insured telephone at all (IN1-7 is the payer's), so IN1 is now absent from `PHONE_FIELDS` deliberately. Every field number now cites its v2.5.1 chapter and clause, because an uncited table is what produced this one. 4. THE BANNER'S "STILL NOT DETECTED" LIST WAS ASSERTED EXHAUSTIVE AND WAS NOT, and two other files defer to it as the authoritative statement of limits. It now also names the non-default component separator, the one-character name component, the unmapped PID fields, and binary or compressed targets. 5. THE PUBLISHED FIGURES WERE OFF BY ONE, self-referentially: writing the changeset added a tracked file the changeset then counted. Both denominators are now stated. Base `daf75c3`: 102 tracked / 31 opened / 71 in neither / 27 of those under `test/`. Head: 103 tracked / 101 opened / 2 in neither / 0 under `test/`. 70 newly opened, of which 69 existed at base and were hand- read. The removed `*.md` walk skip dropped 14 files at base, not 16. Also: `refuseNonDirectoryRoots` swallowed every `lstat` error as "absent" and now excuses only `ENOENT`; the reconciliation's remedy text steered toward widening the exemption list for a deleted-but-tracked file and now names that cause first; and the untracked-outside-every-root residual the refuter named is disclosed AND pinned by a test that asserts the gap, so closing it later reds that case rather than letting the disclosure outlive the defect. THE GRID WAS RE-RUN AGAINST THE CORRECTED HEAD: 37 base `1` cells still `1`, 74 cells `0 -> 1`, and the same single disclosed `1 -> 0` on the publisher mailbox. The two recogniser widenings added detections and subtracted none. The new escaped-separator case is the one test whose payload is itself a live segment literal, so every value in it is assembled at runtime. The gate caught it as a literal first, which is the evidence the widening works. --- .changeset/wild-pugs-remain.md | 8 +-- documentation/agent-notes.md | 52 ++++++++++++++---- scripts/phi-scan.ts | 96 ++++++++++++++++++++++++++++----- test/scripts/phi-scan.test.ts | 99 ++++++++++++++++++++++++++++++++++ 4 files changed, 228 insertions(+), 27 deletions(-) diff --git a/.changeset/wild-pugs-remain.md b/.changeset/wild-pugs-remain.md index 98fe875..d619616 100644 --- a/.changeset/wild-pugs-remain.md +++ b/.changeset/wild-pugs-remain.md @@ -2,7 +2,7 @@ "@cosyte/transform": patch --- -No runtime impact: the repository's own PHI commit-gate read 31 of its 102 tracked files, and it now reads 100 of them and refuses when it cannot account for the rest. +No runtime impact: the repository's own PHI commit-gate read 31 of its 102 tracked files, and it now reads 101 of its 103 and refuses when it cannot account for the rest. Both of the gate's enumerating routes covered `test/fixtures/` and `src/` only. Seventy-one tracked files were read by neither of them, twenty-seven of those under `test/`, and eight of those carried inline HL7 v2 patient-identification segments with names, dates of birth and medical record numbers in them. The sharper half is that `test/fixtures/` has never existed in this repository, on any commit: the walk's existence check returned on its first line for that root on every run the gate has ever made, and every one of those runs printed a clean result and exited zero. An unopened root and a clean one are indistinguishable from the outside. @@ -10,8 +10,10 @@ Neither a file count nor an existence check detects that, and both were consider Enumerating more files buys the cross-cutting social security number and email checks and nothing else, and measured on this repository those two find nothing at all in the eight fixture files: they carry no dashed social security number and no email address. What they carry is names, dates of birth, record numbers, one undashed social security number, a street address and two telephone numbers. So a structured pass ships alongside the existing one, never in place of it, reading HL7 v2 patient, next-of-kin, guarantor and insurance segments field by field and component by component. It finds segments inline rather than assuming a file is a message, because this package ships no standalone message file at all: every fixture is a string literal inside TypeScript. -Every value the structured pass reports is checked against the reviewed synthetic-fixture declaration list, which gains entries for this repository's placeholders. Each was read by hand first and each is named in that file rather than removed, because removing them would destroy the evidence the audit happened. Nothing patient-identifying was found in any of the sixty-nine files the widening newly opened. +Every value the structured pass reports is checked against the reviewed synthetic-fixture declaration list, which gains entries for this repository's placeholders. Each was read by hand first and each is named in that file rather than removed, because removing them would destroy the evidence the audit happened. Seventy files are newly read, sixty-nine of which already existed and were read by hand; nothing patient-identifying was found in any of them. The two that stay unread are the vendored compressed archives, whose stored bytes are not the text they carry, and each is declared by its exact path. One detection is subtracted and it is the only one. The package manifest carries the publisher's own contact address, which the email check cannot tell from a patient's, so scanning the manifest at all required declaring that one address. It is declared with a path as well as an address, so the same address in any other file still reports and any other address in the manifest still reports, and the cost is that naming the manifest directly no longer reports it. Every other outcome is unchanged or newly caught: a hundred and sixty-nine before-and-after cases across every path shape, every payload shape and all three ways the gate can be invoked, with thirty-seven that reported before still reporting and seventy-four that are newly caught. -Four limits stay disclosed rather than quietly closed, including that the reconciliation compares path names and not the bytes stored at them. +Two further blind spots were found while grading this change and were closed rather than written down, because both reported a clean result over content a reader would expect to be caught: a person's name spelled with any character outside the plain English alphabet, and a whole message pasted into a single string with its separators written as escapes. A wrong field position was corrected too, where an insurance segment's relationship code was reported as a telephone number. + +Several limits stay disclosed rather than quietly closed, including that the reconciliation compares path names and not the bytes stored at them, and the complete list of what the gate still cannot see is written at the top of the scanner itself. diff --git a/documentation/agent-notes.md b/documentation/agent-notes.md index 8352206..a79f97b 100644 --- a/documentation/agent-notes.md +++ b/documentation/agent-notes.md @@ -346,8 +346,14 @@ was the **`--staged`** route's boundary, not the walk's, and that suffix bound i **▶ THE HEADLINE, MEASURED ON THIS REPOSITORY AT `daf75c3` RATHER THAN PORTED FROM A SIBLING.** Both enumerating routes covered `test/fixtures/` plus `src/`, and that was **31 of 102 tracked files: 71 read by NEITHER route, 27 of them under `test/`, 8 of those carrying inline HL7 `PID|` literals** -with names, DOBs and MRNs in them. Head state: **102 tracked / 100 opened / 2 in neither**, both of -those declared literal exemptions, and **0 under `test/`**. +with names, DOBs and MRNs in them. + +**▶ STATE BOTH DENOMINATORS, AND RE-DERIVE THEM AFTER THE CHANGESET IS WRITTEN.** They are not the +same number, because this change adds a tracked file of its own and an earlier draft counted itself +out of date: base `daf75c3` is **102 tracked / 31 opened / 71 in neither / 27 of those under +`test/`**; head is **103 tracked / 101 opened / 2 in neither / 0 under `test/`**, and both of the two +are declared literal exemptions. **70 files were newly opened, of which 69 existed at base and were +hand-read**; the seventieth is this change's own changeset. **▶ AND THE SHARPEST HALF, WHICH IS THIS REPOSITORY'S OWN AND NOT A SIBLING'S: `test/fixtures/` HAS NEVER EXISTED HERE, ON ANY COMMIT.** `git log --all -- 'test/fixtures*'` is empty. The walk's @@ -435,14 +441,40 @@ stopping there would scan less. It parses **PID / NK1 / GT1 / IN1** by field and component and checks names (XPN 1/2/3), DOB (the leading 8 digits of a TS), ids (CX-1 across `~` repetitions, with an `SS` type code or a bare 9-digit value named as an SSN), addresses (XAD 1/2/3/5) and phones (XTN components with 4+ digits). -**Four things it does NOT do, stated rather than implied:** a value injected by template -interpolation is skipped rather than guessed at; PROVIDER names in PV1/ORC/OBR XCN fields are out of -scope (a clinician is not the patient, and XCN is a different layout); a single-character middle -initial is below the name-token floor; and a segment written with a non-default field separator is -not recognised. - -**Nothing patient-identifying was found in the 69 files the widening newly opened**, every one of -them hand-read. The fixture values are placeholders and are **named in the allow-list rather than + +**▶ EVERY FIELD NUMBER IS CITED TO HL7 v2.5.1 BY CHAPTER AND CLAUSE IN THE SOURCE, AND THE REASON IS +A MEASURED DEFECT: an uncited table produced `IN1-17` as a telephone field.** IN1-17 is *Insured's +Relationship To Patient*, so a SNOMED relationship code was reported as a phone number, and the +remedy that diagnostic steered a developer toward was a global `PHONE` clearance of that digit +string. **IN1 carries no insured telephone at all**; IN1-7 is the payer's. Found by the refuter. + +**▶ TWO SILENT MISSES THE REFUTER FOUND WERE FIXED RATHER THAN DISCLOSED, BECAUSE BOTH REPORTED +CLEAN OVER CONTENT THE GATE CLAIMS TO CATCH.** (1) The name-token class was `[A-Za-z]`, so `Garcia` +hit and the same name written with its accent exited 0, as did every name in a non-Latin script: a +gate blind to exactly the names least likely to be synthetic. It is a Unicode letter class now, +which still excludes digits so a coded value stays out. (2) A whole message pasted into ONE literal +with **escaped** `\r` separators was never located, because the character before `PID|` is the +letter `r` of the escape: measured, an ADT carrying a name, a DOB, an MRN, an address and two phones +scanned clean at exit 0 while the same message one segment per array element produced 8 hits. The +escaped separator is now both a boundary AND a terminator, and the terminator half is load-bearing: +without it the whole message is read as one segment and the next-of-kin's relationship code is +reported as the patient's PID-11 address. + +**The list of what it still does NOT do lives in the banner at the top of `scripts/phi-scan.ts`, +and `.github/workflows/ci.yml` and `phi-scan-overrides.md` both defer to it**, so an omission there +is the gate claiming to be wider than it is. It covers template interpolation, PV1/ORC/OBR XCN +provider names, non-default separators, one-character name components, the PID fields outside the +mapped set, and binary or compressed targets. **A refuter measured an earlier version of that list +incomplete in the false-confidence direction; add to it before you add to the code.** + +**▶ AND ONE PRE-EXISTING RESIDUAL THE REFUTER NAMED: UNTRACKED content under an undeclared top-level +directory is invisible to BOTH enumerating routes.** The reconciliation covers the tracked half +only, by construction. Head is strictly better than base here, and the gap is pinned by a test that +asserts it, so a future edit closing it reds that case rather than letting the disclosure outlive +the defect. + +**Nothing patient-identifying was found in the 69 pre-existing files the widening newly opened**, +every one of them hand-read. The fixture values are placeholders and are **named in the allow-list rather than scrubbed**: `Jane Q. Public` and `Jane Doe`, the mnemonics keyed to their suites (`Appt^Amy`, `Doc^Dana`, `Imm^Ian`, `Kin^Next`), `MRN1`/`MRN2`/`MRN12345`, an SSN-shaped `999887777` in area number 999 which the Social Security Administration has never issued, placeholder street lines, and diff --git a/scripts/phi-scan.ts b/scripts/phi-scan.ts index 654b92d..48c8880 100644 --- a/scripts/phi-scan.ts +++ b/scripts/phi-scan.ts @@ -38,15 +38,34 @@ * names, DOBs and MRNs and reported every one of them clean. That is the * false confidence this banner exists to refuse. * - * ⚠ STILL NOT DETECTED, stated rather than implied: + * ⚠ STILL NOT DETECTED. **THIS LIST IS THE AUTHORITATIVE STATEMENT OF THIS + * GATE'S LIMITS AND TWO OTHER FILES DEFER TO IT**, so an omission here is + * not a documentation slip, it is the gate claiming to be wider than it is. + * A refuter measured an earlier version of this list INCOMPLETE in the + * false-confidence direction. Add to it before you add to the code: * - a value injected by TEMPLATE INTERPOLATION (`${…}`) into a segment * literal. A static text scan cannot see what a placeholder resolves * to; such a component is skipped rather than guessed at. * - PROVIDER names in PV1 / ORC / OBR XCN fields. Those identify a * clinician, not a patient, and the XCN layout differs from XPN; * declared out of scope rather than half-implemented. - * - a segment written with a NON-DEFAULT field separator. The pass keys - * on `SEG|`, the encoding this corpus and the v2 default both use. + * - a segment written with a NON-DEFAULT field separator, or with a + * non-default component separator. The pass keys on `SEG|` and splits + * on `^` / `~` / `&`, which is the v2 default and what this corpus uses; + * MSH-1/MSH-2 are not consulted. + * - a NAME COMPONENT THAT IS ONE CHARACTER, so a middle initial is below + * the token floor. Raising it competes with the one- and two-letter + * CODE values that share those component positions. + * - a person name carried somewhere OTHER than a v2 segment literal, and + * the whole of PID-2 (external id), PID-4 (alternate id), PID-18 + * (account number) and the patient-visit segments. + * - anything inside a BINARY or compressed target: the passes decode as + * UTF-8 text and a name inside a gzip stream survives that unreadable. + * + * ▶ TWO ENTRIES THAT USED TO BE ON THIS LIST WERE FIXED RATHER THAN + * DISCLOSED, because both were reachable and both reported CLEAN: a name + * component outside ASCII (`García`, `Nguyễn`), and a whole message pasted + * into ONE literal with ESCAPED `\r` separators. Do not re-narrow either. * * Worked examples of structured, format-aware detection live in the sibling * parsers: @@ -590,7 +609,7 @@ function walk(dir: string, out: string[], unscannable: Unscannable[]): void { walk(full, out, unscannable); } else if (e.isFile()) { // ▶ THE `*.md` SKIP THAT USED TO SIT HERE IS GONE, AND ITS REMOVAL IS - // PURELY ADDITIVE. It dropped 16 tracked markdown files before a byte of + // PURELY ADDITIVE. It dropped 14 tracked markdown files before a byte of // any of them was read, on the argument that documentation may // legitimately describe violator values. Two things were wrong with that. // First, it was an ENUMERATION-time judgement standing in for a @@ -599,7 +618,7 @@ function walk(dir: string, out: string[], unscannable: Unscannable[]): void { // it was never true of the other routes: `pnpm phi-scan notes.md` ran the // same content passes at base and reported what it found, so the skip made // the two routes disagree about the same bytes. Measured over this repo's - // tracked corpus, opening all 16 produced ZERO new hits. + // tracked corpus at `daf75c3`, opening all 14 produced ZERO new hits. out.push(full); } else { unscannable.push({ path: normalizePath(full), kind: entryKind(e) }); @@ -667,8 +686,15 @@ function refuseNonDirectoryRoots(roots: string[]): void { let st; try { st = lstatSync(root); - } catch { - continue; // absent, not an error: the reconciliation owns this case + } catch (err) { + // ONLY a genuine absence is excused. Any other `lstat` failure (`EACCES` + // on the parent, `ELOOP`, `ENAMETOOLONG`) is a root the walk cannot + // account for, and swallowing all of them as "absent" would be the same + // shape as the missing-root false clean this preflight exists to close. + const code = err instanceof Error && "code" in err ? String(err.code) : "unknown"; + if (code === "ENOENT") continue; + bad.push({ path: normalizePath(root), kind: `unreadable (${code})` }); + continue; } if (st.isDirectory()) continue; bad.push({ @@ -731,9 +757,13 @@ function reconcileWithGit(opened: Set): void { throw new InvocationError( `refusing the scan: ${String(missing.length)} ${noun} never opened by the walk:\n${lines}\n` + `A declared root that is missing, emptied or replaced looks exactly like a clean one, so ` + - `the walk is reconciled against git's index rather than trusted. ` + - `Add the directory to WALK_ROOT_NAMES in this script, or (if the path genuinely cannot be ` + - `scanned) add that literal path to RECONCILE_EXEMPT with the reason written down.`, + `the walk is reconciled against git's index rather than trusted.\n` + + `Read the paths before reaching for a remedy, because the commonest cause is not a scope ` + + `problem at all: a tracked file DELETED from the working tree but not from the index reports ` + + `here, and the fix is to restore it or to stage the deletion. Only if the path is genuinely ` + + `outside every scan root should you add its directory to WALK_ROOT_NAMES, and only if it ` + + `genuinely cannot be scanned should you add that literal path to RECONCILE_EXEMPT with the ` + + `reason written down. Widening RECONCILE_EXEMPT is the last resort, never the first.`, ); } @@ -1058,6 +1088,17 @@ function scanCommonShapes(path: string, content: string, allow: AllowList, hits: // carry is names, DOBs, MRNs, one undashed SSN in an `SS`-typed identifier, one // street address and two phone numbers, and the floor is blind to every one. +// ▶ EVERY FIELD NUMBER BELOW IS FROM HL7 v2.5.1, AND THE CLAUSE IS CITED BECAUSE +// AN UNCITED TABLE IS WHAT PRODUCES A WRONG ONE. PID is Chapter 3 §3.4.2, NK1 +// Chapter 3 §3.4.5, PV1 Chapter 3 §3.4.3, GT1 Chapter 6 §6.5.4 and IN1 Chapter 6 +// §6.5.6. Measured cost of not citing them: a first draft of this table mapped +// **IN1-17 as a telephone field**. IN1-17 is *Insured's Relationship To Patient* +// (CE, table 0063), so a SNOMED relationship code was reported as a phone +// number, and the remedy that diagnostic steered a developer toward was a global +// `PHONE` clearance of that digit string. **IN1 carries no insured telephone at +// all**: IN1-7 is the PAYER's number, an organisation's, so `IN1` is absent from +// `PHONE_FIELDS` deliberately rather than by omission. + /** PHI-bearing fields per segment, by v2 field number (`PID-5` is index 5). */ const NAME_FIELDS: Record = { PID: [5, 6, 9], @@ -1087,7 +1128,7 @@ const PHONE_FIELDS: Record = { PID: [13, 14], NK1: [5, 6], GT1: [6, 7], - IN1: [17], + // IN1 is absent on purpose: see the citation note above. }; const PHI_SEGMENTS = Object.keys(NAME_FIELDS); @@ -1096,8 +1137,22 @@ const PHI_SEGMENTS = Object.keys(NAME_FIELDS); * Locate a segment literal by its `SEG|` opening. The leading boundary keeps * `PID-3` in prose and `xPID|` in an identifier from matching; only a real * segment id immediately followed by the default field separator qualifies. + * + * ▶ THE `\\r` / `\\n` ALTERNATIVE IS NOT DECORATION, AND IT WAS A MEASURED + * SILENT MISS. A v2 message pasted into a TypeScript literal in one piece writes + * its segment terminator as the ESCAPE `\\r`, so the character immediately before + * `PID|` is the letter `r`, which `[^A-Za-z0-9]` rejects. Measured before this + * alternative was added: a whole ADT in one literal, carrying a name, a DOB, an + * MRN, an address and two phone numbers across PID and NK1, scanned + * `OK: no hits` at exit 0, while the identical message written one segment per + * array element produced 8 hits. That is the OTHER way a `.ts` file carries a + * message here, and it is exactly the shape `parseHL7(raw)` consumes. Purely + * additive: it only adds places a segment can start. */ -const SEGMENT_OPENING = new RegExp(`(?:^|[^A-Za-z0-9])(${PHI_SEGMENTS.join("|")})\\|`, "g"); +const SEGMENT_OPENING = new RegExp( + `(?:^|\\\\[rn]|[^A-Za-z0-9])(${PHI_SEGMENTS.join("|")})\\|`, + "g", +); /** Digits only, so `555-1234`, `(555) 1234` and `5551234` compare equal. */ function digitsOf(value: string): string { @@ -1126,7 +1181,14 @@ function firstSubcomponent(component: string): string { * already restrict which components are read at all. */ function looksLikeNameToken(component: string): boolean { - return /^[A-Za-z][A-Za-z'\-. ]+$/.test(component) && component.trim().length > 1; + // ▶ UNICODE LETTERS, NOT `[A-Za-z]`, AND THAT WAS A MEASURED SILENT MISS. An + // ASCII-only class reports a name it cannot spell as CLEAN rather than as + // unrecognised, so `Garcia` hit while the same name written with its accent + // exited 0, as did every name in a non-Latin script. A gate that is blind to + // exactly the names least likely to be synthetic is worse than no gate. + // `\p{L}` excludes digits, so a coded value stays out; combining marks are + // admitted so a decomposed accent does not split a token. + return /^\p{L}[\p{L}\p{M}'\-. ]+$/u.test(component) && component.trim().length > 1; } function fieldsOf(segment: string): string[] { @@ -1231,7 +1293,13 @@ function scanHl7Segments(path: string, content: string, allow: AllowList, hits: if (id === undefined) continue; const start = (opening.index ?? 0) + opening[0].length - id.length - 1; const rest = content.slice(start); - const end = rest.search(/[\r\n"`]/); + // The ESCAPED separators are terminators as well as boundaries. Without them + // a whole message in one literal is read as ONE segment, and every field + // after the first embedded `\r` lands at the wrong index: measured, a PID + // followed by an escaped separator and an NK1 reported the next-of-kin's + // relationship code as the patient's ADDRESS. Bounding on them makes each + // segment's field numbering its own again. + const end = rest.search(/\\[rn]|[\r\n"`]/); const segment = end < 0 ? rest : rest.slice(0, end); const fields = fieldsOf(segment); diff --git a/test/scripts/phi-scan.test.ts b/test/scripts/phi-scan.test.ts index e306460..0583809 100644 --- a/test/scripts/phi-scan.test.ts +++ b/test/scripts/phi-scan.test.ts @@ -1201,3 +1201,102 @@ describe("phi-scan: the EMAIL allow-list entry, and exactly how far it reaches", expect(r.stderr).toContain("EMAIL entry needs a path"); }); }); + +// --------------------------------------------------------------------------- +// The three silent misses a refuter measured, and the residual it named +// --------------------------------------------------------------------------- +// +// Each of the first three reported a CLEAN result over content this gate claims +// to catch, so each is pinned RED-before / GREEN-after rather than described. + +describe("phi-scan: the HL7 pass sees names it cannot spell, and messages in one literal", () => { + const family = ["Kowal", "ski"].join(""); + const given = ["Barb", "ara"].join(""); + + it("catches a NAME COMPONENT OUTSIDE ASCII, which an [A-Za-z] class reported clean", () => { + // A gate blind to exactly the names least likely to be synthetic is worse + // than no gate. Both a precomposed accent and a non-Latin script. + const accented = ["Garc", "ía"].join(""); + const vietnamese = ["Nguy", "ễn"].join(""); + const r = scan("nonascii.ts", `const m = "PID|1||||${accented}^${vietnamese}";\n`); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain(accented); + expect(r.stderr).toContain(vietnamese); + // Non-vacuity: a DIGIT string in the same component is still not a name. + const coded = scan("coded-name.ts", `const m = "PID|1||||125677006^^^^^^ZZ";\n`); + expect(coded.code, `stderr: ${coded.stderr}`).toBe(0); + }); + + it("catches a whole message in ONE literal with ESCAPED separators, and numbers its fields right", () => { + // The other way a `.ts` file carries a v2 message here, and the shape + // `parseHL7(raw)` consumes. Before the fix this exited 0: the character + // before `PID|` is the letter `r` of the escape, which the boundary class + // rejected. The escaped separator is a TERMINATOR too, or every field after + // the first one lands at the wrong index. + // Every value here is assembled, per the banner at the top of this file: + // this is the one case whose payload IS a live segment literal, so writing + // any of it out would red the repository's own gate on every run. + const street = ["9", "Elm", "Rd"].join(" "); + const city = ["Day", "ton"].join(""); + const zip = ["454", "02"].join(""); + const mrn = ["765", "4321"].join(""); + const dob = ["1963", "12", "07"].join(""); + const kin = ["Pet", "er"].join(""); + const phone = ["937", "5550187"].join(""); + const kinPhone = ["937", "5550188"].join(""); + const addr = `${street}^^${city}^OH^${zip}`; + const msh = "MSH|^~\\&|A|B|C|D|20260101||ADT^A01|M1|P|2.5.1"; + const pid = `PID|1||${mrn}^^^HOSP^MR||${family}^${given}||${dob}|F|||${addr}||${phone}`; + const nk1 = `NK1|1|${family}^${kin}|SPO|${addr}|${kinPhone}`; + const r = scan("escaped.ts", `const msg = "${msh}\\r${pid}\\r${nk1}";\n`); + + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("PID-5"); + expect(r.stderr).toContain("PID-7"); + expect(r.stderr).toContain("PID-11"); + expect(r.stderr).toContain("PID-13"); + expect(r.stderr).toContain("NK1-2"); + expect(r.stderr).toContain("NK1-4"); + // The field numbering is the half that breaks silently: without bounding on + // the escape, the next-of-kin's relationship code is reported as PID-11. + expect(r.stderr).not.toContain('segment=PID-11 value="SPO"'); + }); + + it("does NOT read IN1-17 as a telephone field: v2.5.1 defines it as a relationship code", () => { + // A wrong field number is a fabricated diagnostic, and the remedy it steers + // a developer toward is a global PHONE clearance of a SNOMED code. IN1 + // carries no insured telephone at all; IN1-7 is the payer's. + const r = scan( + "in1-17.ts", + 'const s = "IN1|1|PLAN|CO123|BlueCross|||||||||||||125677006^Relative^SCT";\n', + ); + expect(r.stderr).not.toContain("IN1-17"); + expect(r.stderr).not.toContain("telephone"); + }); + + it("REFUSES a walk root whose lstat fails for a reason other than absence", () => { + // Swallowing every lstat error as "absent" is the same shape as the + // missing-root false clean this preflight exists to close. `ENOTDIR` here: + // a root path whose own PARENT is a regular file. + const root = makeRepo(); + rmSync(join(root, "docs-content"), { recursive: true, force: true }); + writeFileSync(join(root, "docs-content"), "not a directory\n"); + const r = runIn(root, []); + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("docs-content"); + }); + + it("RESIDUAL, pinned as a residual: UNTRACKED content outside every root is invisible", () => { + // Disclosed rather than closed. The reconciliation covers the TRACKED half + // (a tracked stray refuses, pinned above); an untracked one under an + // undeclared top-level directory is seen by neither enumerating route. This + // case asserts the gap so a future edit that closes it reds here and the + // disclosure gets updated rather than silently outliving the defect. + const root = makeRepo(); + mkdirSync(join(root, "notes")); + writeFileSync(join(root, "notes", "leak.ts"), SYNTHETIC_PHI); + expect(runIn(root, []).code).toBe(0); + // And it is not invisible to the route that is handed it. + expect(runIn(root, ["notes/leak.ts"]).code).toBe(1); + }); +}); From e55adefd00f7ecf878201e2cab1e8c9e2c9a5e93 Mon Sep 17 00:00:00 2001 From: Noah Schatz Date: Sat, 8 Aug 2026 13:20:40 +0000 Subject: [PATCH 3/4] docs(phi-scan): state the gate's coverage POSITIVELY, and correct the last two figures (PHI-SCAN-WALK-ROOT-SCOPE) Pass 2 REFUTED. Every finding was about the accuracy of a claim rather than the behaviour of the gate, and the major one is the same shape twice. 1. THE EXHAUSTIVE NEGATIVE LIST WAS MEASURED INCOMPLETE A SECOND TIME, AFTER BEING EXTENDED IN ANSWER TO THE FIRST PASS. Seven PHI-bearing v2.5.1 fields reported clean while the list called itself the authoritative statement of this gate's limits: NK1-26, NK1-31, NK1-32, NK1-37, GT1-2, GT1-4 and IN1-49. Two of those matter beyond the count. NK1-37 is a social security number field, and an undashed one there is invisible to the floor as well, so both passes miss it at once. IN1-49 is the member id, while three files told a reader the pass covers "member id". A LIST OF THAT SHAPE CANNOT BE KEPT TRUE: every clause of every segment of the standard would have to appear on it, and two passes proved it. So the claim is inverted. The banner now enumerates EXACTLY the fields that are read and says anything not named is not checked. That is checkable against the code; the other shape never was. `phi-scan-overrides.md` and `ci.yml`, which defer to the banner, drop their unqualified "member id" wording and point at the named set instead. NEVER RESTORE THE NEGATIVE SHAPE. The table is NOT grown in answer to this. Growing it is what produced the IN1-17 defect: more uncited field numbers from the same memory. 2. PROVENANCE, WHOSE ABSENCE WAS THE ROOT CAUSE AND IS NOW SAID PLAINLY. The field numbers are asserted from HL7 v2.5.1 and were cross-corroborated IN-REPO ONLY, against `src/messages/related-person.ts` and the vendored `@cosyte/hl7` type surface. They were NOT checked against a published copy of the standard, and one of them was wrong on the way here. That is why the table is deliberately narrow and why widening it means citing a source. 3. A FOURTH RECOGNISER LIMIT, DISCLOSED RATHER THAN GUESSED AT: a literal backslash followed by `r` or `n` inside a field value ends the segment early, because the escaped separator is also the terminator. Measured, a Windows path in PID-11 truncates there and PID-13/14 go unread. It can only SHORTEN a segment, never renumber one, and the field it cuts in still reports. Not decidable from static text. 4. THE `*.md` SKIP FIGURE WAS WRONG IN BOTH DIRECTIONS AND IS NOW ZERO. Under the OLD roots the skip dropped no files at all: `src/` holds no markdown and `test/fixtures/` never existed, so it was dead code. Under the widened roots it would drop 15, which is the only reason removing it matters. 16 and 14 were both counts of something else. 5. THE GRID TOTALS NOW FACTOR AND THE PUBLISHED FIGURE DID NOT. It is 14 paths x 4 payloads x 3 routes = 168 cells: 37 base `1` still `1`, 74 `0 -> 1`, 1 `1 -> 0`, and 56 unchanged zeros, which sums to 168. "169 over 15 paths" factored to 180 and left 57 cells unaccounted for. It is recorded as a MEASUREMENT, not as a fixture: nothing pins the base half, because a head-only test structurally cannot, and saying otherwise was the overclaim. The new suite case asserts the coverage boundary from BOTH sides in one run: eight fields inside the named set must report, and eight outside it must be clean, so each zero is a declared gap rather than a detector failure. A field added to the table without being added to the banner reds there. The grid was re-run against this head: 37 / 74 / 1 / 56, unchanged, and the same single disclosed `1 -> 0` on the publisher mailbox. --- .changeset/wild-pugs-remain.md | 4 +- .github/workflows/ci.yml | 6 +- CLAUDE.md | 13 ++-- documentation/agent-notes.md | 37 +++++++++--- phi-scan-overrides.md | 18 +++--- scripts/phi-scan.ts | 106 +++++++++++++++++++++++---------- test/scripts/phi-scan.test.ts | 77 ++++++++++++++++++++++++ 7 files changed, 206 insertions(+), 55 deletions(-) diff --git a/.changeset/wild-pugs-remain.md b/.changeset/wild-pugs-remain.md index d619616..e0720e7 100644 --- a/.changeset/wild-pugs-remain.md +++ b/.changeset/wild-pugs-remain.md @@ -12,8 +12,10 @@ Enumerating more files buys the cross-cutting social security number and email c Every value the structured pass reports is checked against the reviewed synthetic-fixture declaration list, which gains entries for this repository's placeholders. Each was read by hand first and each is named in that file rather than removed, because removing them would destroy the evidence the audit happened. Seventy files are newly read, sixty-nine of which already existed and were read by hand; nothing patient-identifying was found in any of them. The two that stay unread are the vendored compressed archives, whose stored bytes are not the text they carry, and each is declared by its exact path. -One detection is subtracted and it is the only one. The package manifest carries the publisher's own contact address, which the email check cannot tell from a patient's, so scanning the manifest at all required declaring that one address. It is declared with a path as well as an address, so the same address in any other file still reports and any other address in the manifest still reports, and the cost is that naming the manifest directly no longer reports it. Every other outcome is unchanged or newly caught: a hundred and sixty-nine before-and-after cases across every path shape, every payload shape and all three ways the gate can be invoked, with thirty-seven that reported before still reporting and seventy-four that are newly caught. +One detection is subtracted and it is the only one. The package manifest carries the publisher's own contact address, which the email check cannot tell from a patient's, so scanning the manifest at all required declaring that one address. It is declared with a path as well as an address, so the same address in any other file still reports and any other address in the manifest still reports, and the cost is that naming the manifest directly no longer reports it. Every other outcome is unchanged or newly caught: a hundred and sixty-eight before-and-after cases, fourteen path shapes by four payload shapes by all three ways the gate can be invoked, with thirty-seven that reported before still reporting, seventy-four newly caught and fifty-six unchanged and quiet. Two further blind spots were found while grading this change and were closed rather than written down, because both reported a clean result over content a reader would expect to be caught: a person's name spelled with any character outside the plain English alphabet, and a whole message pasted into a single string with its separators written as escapes. A wrong field position was corrected too, where an insurance segment's relationship code was reported as a telephone number. +The way the gate's own limits are written changed with them, and that is the more useful half. Grading twice showed that a list of what a scanner does not catch cannot be kept true, because every clause of every segment of the standard would have to appear on it, and both versions of that list were measured incomplete in the direction that flatters the gate. The scanner now states the opposite way round: exactly which fields it reads, with anything not named there not checked. That claim can be checked against the code; the other one never could. + Several limits stay disclosed rather than quietly closed, including that the reconciliation compares path names and not the bytes stored at them, and the complete list of what the gate still cannot see is written at the top of the scanner itself. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index df338c6..ec1404d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,8 +42,10 @@ jobs: # The shared pipeline's optional PHI-scan step is switched on, and what it runs is the all-mode # walk over this repo's whole TRACKED corpus, reconciled against `git ls-files` so a root that is # missing, emptied or replaced refuses rather than reporting clean. It runs the cross-cutting - # SSN/email floor AND an HL7 v2 structured pass over PID/NK1/GT1/IN1. What it still does not see - # is written out in the banner at the top of scripts/phi-scan.ts. + # SSN/email floor AND an HL7 v2 structured pass over a NAMED SET of PID/NK1/GT1/IN1 fields. The + # banner at the top of scripts/phi-scan.ts enumerates exactly which fields those are and states + # that anything not named is not checked; read it as the whole of the coverage, because two + # refuter passes measured the opposite shape (a list of what is NOT covered) incomplete. ci: uses: cosyte/.github/.github/workflows/ci.yml@main with: diff --git a/CLAUDE.md b/CLAUDE.md index 034a428..0fdbf36 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -180,15 +180,14 @@ Measurements, the grid, the refuters, and the `--staged` ARGV traps (`--diff-fil DIRECTLY, that is not a regular file.** "Follows nothing" is looser and **two refuter passes measured it FALSE**; do not tighten it back, and never close it by following. **`lstat` answers for the FINAL COMPONENT ONLY**: touch `buildTargetsForPaths` and **re-measure, never re-assert**. -- **A refusal never echoes the link target** (path plus a closed-set kind token), **and that binds - the prose too**: write the target as a _shape_, never an example. + **A refusal never echoes the link target**, and that binds the prose: a _shape_, never an example. - **▶ THERE ARE THREE ROUTES, NOT TWO** (`all`, `--staged`, ``), all running the content passes. **Enumerate all three before calling anything additive.** - **▶ SCOPE IS THE TRACKED CORPUS, RECONCILED AGAINST `git ls-files` EVERY RUN**, because **the `fixtures` root HAD NEVER EXISTED ON ANY COMMIT** and went unopened on every run ever made while the run printed clean. **A count cannot detect that**, nor can an existence check: an EMPTIED root - opens nothing. **Roots stay DISJOINT** or nested files report twice. The `*.md` walk skip is gone - (purely additive); `src/**.ts` was the **`--staged`** bound, now widened. **Not one rule.** + opens nothing. **Roots stay DISJOINT** or nested files report twice. The `*.md` walk skip is + gone (additive); `src/**.ts` was the **`--staged`** bound, now widened. **Not one rule.** - **▶ WIDEN BY UNION AND PROVE THE GRID: every base `1` still `1`.** One cell is not: **`phi-scan package.json` on the npm publisher mailbox**, declared with `EMAIL` (a **path AND an address**). **Every allow-list entry is ROUTE-BLIND** and clears on `--staged`; every tag but @@ -196,9 +195,9 @@ Measurements, the grid, the refuters, and the `--staged` ARGV traps (`--diff-fil - **An exemption is a LITERAL PATH, never a predicate, and reaches the ALL route only**: the vendored gzip tarballs are the whole list, and `` still reads them. - **Exit `2` is every failure to complete; `1` is HITS FOUND. A regular-file root is `2` HERE, - derived from this contract; siblings differ, never port one.** A non-directory root refuses first: - **`existsSync` FOLLOWS**, so a dangling one printed clean over an off-disk corpus and a symlinked - one was followed. An **absent** root is fine. + derived from this contract; siblings differ, never port one.** A non-directory root refuses first, + because **`existsSync` FOLLOWS**: a dangling one printed clean over an off-disk corpus. An + **absent** root is fine. - **Enumerating buys the SSN/email floor and NOTHING else**, so the HL7 v2 pass ships **in addition to** it, never instead: the floor finds **zero** in this repo's `PID|` fixtures. **No standalone `.hl7` ships: every message is a `.ts` literal.** diff --git a/documentation/agent-notes.md b/documentation/agent-notes.md index a79f97b..e4c7557 100644 --- a/documentation/agent-notes.md +++ b/documentation/agent-notes.md @@ -393,9 +393,13 @@ porting one is the bug this item exists to stop.** sitting beside it: declaring both reports every nested file twice. The previous list is a strict SUBSET of the new one, and the `--staged` scope's previous predicate is a strict subset of its new one (the `.ts` suffix requirement is dropped, not kept), so nothing either route saw can stop being -seen. **Proved by grid rather than argued: 169 cells over 15 paths, 4 payloads and all 3 routes, +seen. **Proved by grid rather than argued: 168 cells, 14 paths x 4 payloads x 3 routes, each run against the base scanner and the head scanner. 37 base `1` cells still `1`, 74 cells -`0 -> 1`, and exactly ONE `1 -> 0`,** which is the next paragraph. +`0 -> 1`, and exactly ONE `1 -> 0`,** which is the next paragraph. **The totals factor and the +figure published before did not:** 14 paths x 4 payloads x 3 routes is 168 cells, and 37 + 74 + 1 + +56 unchanged zeros is 168. It is a recorded measurement rather than a fixture: nothing in the suite +pins the BASE half, because a head-only test structurally cannot. What the suite does pin is the +head side of every cell that carries the argument, each by a named case. **▶ THE ONE SUBTRACTION, NAMED RATHER THAN LEFT TO BE FOUND: `pnpm phi-scan package.json` exited 1 on the npm publisher contact in its `author` field and now exits 0.** That mailbox is public, @@ -460,12 +464,29 @@ escaped separator is now both a boundary AND a terminator, and the terminator ha without it the whole message is read as one segment and the next-of-kin's relationship code is reported as the patient's PID-11 address. -**The list of what it still does NOT do lives in the banner at the top of `scripts/phi-scan.ts`, -and `.github/workflows/ci.yml` and `phi-scan-overrides.md` both defer to it**, so an omission there -is the gate claiming to be wider than it is. It covers template interpolation, PV1/ORC/OBR XCN -provider names, non-default separators, one-character name components, the PID fields outside the -mapped set, and binary or compressed targets. **A refuter measured an earlier version of that list -incomplete in the false-confidence direction; add to it before you add to the code.** +**▶ THE COVERAGE STATEMENT IS POSITIVE, AND THAT SHAPE IS THE FINDING RATHER THAN A STYLE CHOICE.** +Two successive refuter passes measured an EXHAUSTIVE NEGATIVE LIST of "what this does not catch" +incomplete in the false-confidence direction, and the second measured it incomplete AGAIN after it +had been extended in answer to the first. Seven PHI-bearing v2.5.1 fields reported clean while the +list called itself authoritative: NK1-26, NK1-31, NK1-32, NK1-37 (a contact SSN, invisible to the +floor too because it is undashed), GT1-2, GT1-4 and IN1-49 (the insured's id, while three files told +a reader the pass covers "member id"). **A negative list of that shape cannot be kept true**, because +every clause of every segment would have to appear on it. The banner in `scripts/phi-scan.ts` now +enumerates EXACTLY the fields that are read, and says that anything not named is not checked. That +claim is checkable; the other one was not. **Correct it by narrowing the claim, never by silently +adding a field number.** + +**▶ PROVENANCE, RECORDED BECAUSE ITS ABSENCE WAS THE ROOT CAUSE.** The field numbers are asserted +from HL7 v2.5.1 and were cross-corroborated **in-repo only**, against `src/messages/related-person.ts` +and the vendored `@cosyte/hl7` type surface. They were **not** checked against a published copy of +the standard, and one of them was wrong on the way here (IN1-17). That is why the table is +deliberately narrow and why widening it means citing a source, not adding a number. + +**And a fourth recogniser limit the second pass found**: a literal backslash followed by `r` or `n` +inside a field value ends the segment early, because the escaped separator is also the terminator. +Measured, a Windows path in PID-11 truncates there and PID-13/14 go unread. It can only SHORTEN a +segment, never renumber one (the fields before the cut keep their positions) and the field it cuts +in still reports, so it is disclosed rather than guessed at: it is not decidable from static text. **▶ AND ONE PRE-EXISTING RESIDUAL THE REFUTER NAMED: UNTRACKED content under an undeclared top-level directory is invisible to BOTH enumerating routes.** The reconciliation covers the tracked half diff --git a/phi-scan-overrides.md b/phi-scan-overrides.md index 7fe64a8..486253e 100644 --- a/phi-scan-overrides.md +++ b/phi-scan-overrides.md @@ -10,13 +10,17 @@ _every_ check for that file. > **What the scanner detects, so a bypass is judged against the real gate rather > than a template.** Two passes run on every target, on all three routes: the -> cross-cutting SSN/email floor, and an HL7 v2 structured pass over PID / NK1 / -> GT1 / IN1 checking names, date of birth, MRN / member id / SSN, address and -> phone against `scripts/phi-allow-list.txt`. Segment literals are found inline, -> because this package ships no standalone `.hl7` file: every message in its -> corpus is a `.ts` string literal. What it still does NOT see is written out in -> the banner at the top of `scripts/phi-scan.ts`, and a bypass here silences all -> of it for that path. +> cross-cutting SSN/email floor, and an HL7 v2 structured pass over a NAMED SET +> of PID / NK1 / GT1 / IN1 fields, checked against `scripts/phi-allow-list.txt`. +> Segment literals are found inline, because this package ships no standalone +> `.hl7` file: every message in its corpus is a `.ts` string literal. +> +> **Read the set before you judge a bypass against it, and read it as the whole +> of the coverage.** The banner at the top of `scripts/phi-scan.ts` enumerates +> exactly which fields are read and states that anything not named there is not +> checked: two refuter passes measured the opposite shape, a list of what is NOT +> covered, incomplete in the false-confidence direction. A bypass here silences +> the named set AND the floor for that path. ## Format diff --git a/scripts/phi-scan.ts b/scripts/phi-scan.ts index 48c8880..3c4f5e1 100644 --- a/scripts/phi-scan.ts +++ b/scripts/phi-scan.ts @@ -23,14 +23,7 @@ * actually carries. `@cosyte/transform` ships NO standalone `.hl7` * fixture files at all: every message in the corpus is an inline `.ts` * STRING LITERAL, so this pass finds segment literals ANYWHERE in a - * target's text rather than assuming the file IS the message. It parses - * PID / NK1 / GT1 / IN1 by field and component and checks each - * PHI-bearing field against the allow-list: - * - person NAMES (XPN family / given / middle) - * - DATE OF BIRTH (TS, leading 8 digits) - * - MRN / member id / SSN (CX repetitions and bare id fields) - * - ADDRESS (XAD street / other designation / city / postal code) - * - PHONE (XTN components carrying 4 or more digits) + * target's text rather than assuming the file IS the message. * * ⚠ ENUMERATING MORE FILES BUYS THE FLOOR AND NOTHING ELSE. The floor finds * ZERO in this repository's HL7 fixtures: they carry no dashed SSN and no @@ -38,31 +31,79 @@ * names, DOBs and MRNs and reported every one of them clean. That is the * false confidence this banner exists to refuse. * - * ⚠ STILL NOT DETECTED. **THIS LIST IS THE AUTHORITATIVE STATEMENT OF THIS - * GATE'S LIMITS AND TWO OTHER FILES DEFER TO IT**, so an omission here is - * not a documentation slip, it is the gate claiming to be wider than it is. - * A refuter measured an earlier version of this list INCOMPLETE in the - * false-confidence direction. Add to it before you add to the code: + * ══════════════════════════════════════════════════════════════════════════ + * ▶ THE COVERAGE STATEMENT IS POSITIVE, AND THAT SHAPE IS THE POINT. + * + * Two successive refuter passes measured an EXHAUSTIVE NEGATIVE LIST of "what + * this does not catch" incomplete, in the false-confidence direction, and the + * second measured it incomplete AGAIN after it had been extended in answer to + * the first. Seven PHI-bearing v2.5.1 fields reported clean while the list + * claimed to be authoritative: NK1-26 (mother's maiden name), NK1-31 (contact + * telephone), NK1-32 (contact address), NK1-37 (contact SSN), GT1-2 + * (guarantor number), GT1-4 (guarantor spouse name) and IN1-49 (insured's id). + * + * A negative list of that shape CANNOT be kept true: every clause of every + * segment of the standard would have to appear on it. So the claim is stated + * the only way that is checkable, as EXACTLY WHAT IS READ. Anything not named + * below IS NOT CHECKED, including but not limited to the seven fields above. + * + * PID-3, PID-19, PID-20 ....... id / SSN / driver's licence + * PID-5, PID-6, PID-9 ......... name / mother's maiden name / alias + * PID-7 ....................... date of birth + * PID-11 ...................... address + * PID-13, PID-14 .............. home / business telephone + * NK1-2, NK1-30 ............... name / contact person's name + * NK1-4 ....................... address + * NK1-5, NK1-6 ................ telephone + * NK1-16 ...................... date of birth + * NK1-33 ...................... next-of-kin identifiers + * GT1-3 ....................... guarantor name + * GT1-5 ....................... guarantor address + * GT1-6, GT1-7 ................ guarantor telephone + * GT1-8 ....................... guarantor date of birth + * GT1-12, GT1-19 .............. guarantor SSN / employee id + * IN1-16, IN1-18, IN1-19 ...... insured name / DOB / address + * IN1-36 ...................... insured's policy identifier + * + * NO OTHER SEGMENT IS READ AT ALL: not PV1, ORC, OBR, OBX, RXA, SCH, TXA, and + * not MSH. NO OTHER FIELD of the four segments above is read. Within a field, + * only the components named in each `check…Field` are read. + * + * ▶ PROVENANCE, SAID PLAINLY BECAUSE ITS ABSENCE WAS THE ROOT CAUSE OF A + * MEASURED DEFECT. The field numbers are asserted from HL7 v2.5.1 (PID and + * NK1 in Chapter 3, GT1 and IN1 in Chapter 6) and were cross-corroborated + * in-repo only, against `src/messages/related-person.ts` and the vendored + * `@cosyte/hl7` type surface. **They were NOT checked against a published copy + * of the standard.** One of them was wrong on the way here: IN1-17 shipped as + * a telephone field and is in fact Insured's Relationship To Patient, so a + * SNOMED code was reported as a phone number. That is why the table is + * deliberately narrow, why it is stated positively, and why widening it means + * citing a source rather than adding a number. + * ══════════════════════════════════════════════════════════════════════════ + * + * ⚠ FOUR THINGS THE PASS CANNOT SEE EVEN INSIDE THE FIELDS IT READS. These + * are properties of the recogniser rather than of the table above: * - a value injected by TEMPLATE INTERPOLATION (`${…}`) into a segment * literal. A static text scan cannot see what a placeholder resolves * to; such a component is skipped rather than guessed at. - * - PROVIDER names in PV1 / ORC / OBR XCN fields. Those identify a - * clinician, not a patient, and the XCN layout differs from XPN; - * declared out of scope rather than half-implemented. - * - a segment written with a NON-DEFAULT field separator, or with a - * non-default component separator. The pass keys on `SEG|` and splits - * on `^` / `~` / `&`, which is the v2 default and what this corpus uses; - * MSH-1/MSH-2 are not consulted. + * - a segment written with a NON-DEFAULT field or component separator. + * The pass keys on `SEG|` and splits on `^` / `~` / `&`, the v2 default + * and what this corpus uses; MSH-1 and MSH-2 are not consulted. * - a NAME COMPONENT THAT IS ONE CHARACTER, so a middle initial is below * the token floor. Raising it competes with the one- and two-letter * CODE values that share those component positions. - * - a person name carried somewhere OTHER than a v2 segment literal, and - * the whole of PID-2 (external id), PID-4 (alternate id), PID-18 - * (account number) and the patient-visit segments. - * - anything inside a BINARY or compressed target: the passes decode as - * UTF-8 text and a name inside a gzip stream survives that unreadable. + * - A LITERAL BACKSLASH FOLLOWED BY `r` OR `n` INSIDE A FIELD VALUE ENDS + * THE SEGMENT EARLY, because the escaped separator is also the + * terminator. Measured: a Windows path in PID-11 (`C:\records\…`) + * truncates there, so PID-13 and PID-14 go unread. It can only SHORTEN + * a segment, never renumber one, since the fields before the cut keep + * their positions; and the field it cuts in still reports. Not + * decidable from static text, so it is disclosed rather than guessed. + * + * Anything inside a BINARY or compressed target is unreadable to both + * passes: they decode as UTF-8, and a name inside a gzip stream survives. * - * ▶ TWO ENTRIES THAT USED TO BE ON THIS LIST WERE FIXED RATHER THAN + * ▶ TWO ENTRIES THAT USED TO BE ON A NEGATIVE LIST WERE FIXED RATHER THAN * DISCLOSED, because both were reachable and both reported CLEAN: a name * component outside ASCII (`García`, `Nguyễn`), and a whole message pasted * into ONE literal with ESCAPED `\r` separators. Do not re-narrow either. @@ -609,16 +650,21 @@ function walk(dir: string, out: string[], unscannable: Unscannable[]): void { walk(full, out, unscannable); } else if (e.isFile()) { // ▶ THE `*.md` SKIP THAT USED TO SIT HERE IS GONE, AND ITS REMOVAL IS - // PURELY ADDITIVE. It dropped 14 tracked markdown files before a byte of - // any of them was read, on the argument that documentation may - // legitimately describe violator values. Two things were wrong with that. + // PURELY ADDITIVE, AND THE COUNT IT USED TO CARRY WAS WRONG TWICE. Under the + // OLD roots the skip dropped **ZERO** files: `src/` holds no markdown and + // `test/fixtures/` never existed, so it was dead code. Under the widened + // roots it would drop **15**, which is the only reason removing it matters + // at all, and it is why the two figures published before (16, then 14) + // were both counts of something else. The skip existed on the argument + // that documentation may legitimately describe violator values. Two + // things were wrong with that. // First, it was an ENUMERATION-time judgement standing in for a // CONTENT-time one: the allow-list already exists to say "this literal is // synthetic", by value and under review, which a filename cannot. Second, // it was never true of the other routes: `pnpm phi-scan notes.md` ran the // same content passes at base and reported what it found, so the skip made // the two routes disagree about the same bytes. Measured over this repo's - // tracked corpus at `daf75c3`, opening all 14 produced ZERO new hits. + // tracked corpus, opening all 15 produced ZERO new hits. out.push(full); } else { unscannable.push({ path: normalizePath(full), kind: entryKind(e) }); diff --git a/test/scripts/phi-scan.test.ts b/test/scripts/phi-scan.test.ts index 0583809..295a496 100644 --- a/test/scripts/phi-scan.test.ts +++ b/test/scripts/phi-scan.test.ts @@ -1300,3 +1300,80 @@ describe("phi-scan: the HL7 pass sees names it cannot spell, and messages in one expect(runIn(root, ["notes/leak.ts"]).code).toBe(1); }); }); + +describe("phi-scan: the coverage claim is POSITIVE, and this is what makes it checkable", () => { + // Two refuter passes measured an exhaustive NEGATIVE list ("what this does not + // catch") incomplete in the false-confidence direction, the second time after + // it had been extended in answer to the first. A negative list of that shape + // cannot be kept true. The banner now enumerates exactly which fields are + // read; these cases assert the boundary of that enumeration from both sides, + // so a field quietly added to the table without being added to the banner reds + // here rather than shipping as a silently wider claim. + const family = ["Kowal", "ski"].join(""); + const given = ["Barb", "ara"].join(""); + + const seg = (name: string, fields: Readonly>): string => { + const max = Math.max(0, ...Object.keys(fields).map(Number)); + const parts = [name]; + for (let i = 1; i <= max; i += 1) parts.push(fields[i] ?? ""); + return parts.join("|"); + }; + + it("reads every field the banner names, and NONE that it does not", () => { + const name = `${family}^${given}`; + // In the named set: each must report. + const covered: [string, string, Record][] = [ + ["PID", "PID-5", { 5: name }], + ["PID", "PID-7", { 7: "19631207" }], + ["NK1", "NK1-2", { 2: name }], + ["NK1", "NK1-33", { 33: "A77321" }], + ["GT1", "GT1-3", { 3: name }], + ["GT1", "GT1-12", { 12: "555443210" }], + ["IN1", "IN1-16", { 16: name }], + ["IN1", "IN1-36", { 36: "POL77321" }], + ]; + for (const [segment, label, fields] of covered) { + const r = scan(`cov-${label}.ts`, `const m = "${seg(segment, fields)}";\n`); + expect(r.code, `${label} should report. stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain(label); + } + + // OUTSIDE the named set: each must be clean, and each zero is a GAP the + // banner declares, not a clearance. They run in the same suite as the + // controls above so a wholesale detector failure cannot produce them. + const uncovered: [string, string, Record][] = [ + ["NK1", "NK1-26", { 26: name }], + ["NK1", "NK1-31", { 31: "9375550188" }], + ["NK1", "NK1-32", { 32: "9 Elm Rd^^Dayton^OH^45402" }], + ["NK1", "NK1-37", { 37: "555443210" }], + ["GT1", "GT1-2", { 2: "G77321" }], + ["GT1", "GT1-4", { 4: name }], + ["IN1", "IN1-49", { 49: "MEM77321" }], + ["PV1", "PV1-7", { 7: `1234^${family}^${given}` }], + ]; + for (const [segment, label, fields] of uncovered) { + const r = scan(`unc-${label}.ts`, `const m = "${seg(segment, fields)}";\n`); + expect(r.code, `${label} is declared OUT of scope. stderr: ${r.stderr}`).toBe(0); + } + }); + + it("a literal backslash before r or n truncates the segment, and only shortens it", () => { + // The fourth recogniser limit, disclosed rather than guessed at: the escaped + // separator is also the terminator, so a Windows path in an address field + // ends the segment there. Pinned so the disclosure cannot outlive the + // behaviour. The fields BEFORE the cut keep their positions, which is what + // makes this a truncation rather than a renumbering. + const winPath = ["C:", "records", "scan.tif"].join("\\\\"); + const r = scan( + "backslash.ts", + `const p = "${seg("PID", { 5: `${family}^${given}`, 7: "19631207", 11: `${winPath}^^Springfield^ZZ^99999`, 13: "5551230000" })}";\n`, + ); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + // Everything before the cut is still read, at its right field number. + expect(r.stderr).toContain("PID-5"); + expect(r.stderr).toContain("PID-7"); + expect(r.stderr).toContain("PID-11"); + // And the truncation itself: PID-13 is past the cut. + expect(r.stderr).not.toContain("PID-13"); + }); +}); From 3f6e2c5e8c713393d11f348a44cd57a9b156ff66 Mon Sep 17 00:00:00 2001 From: Noah Schatz Date: Sat, 8 Aug 2026 13:44:45 +0000 Subject: [PATCH 4/4] docs(phi-scan): land the four claim corrections pass 3 measured, and pin all 28 named fields (PHI-SCAN-WALK-ROOT-SCOPE) Pass 3 NOT REFUTED, with four minor claim-accuracy corrections to land without another round (the ADR 0016 cap is spent) and one stale citation. All four were in the flattering direction, inside the commit whose purpose was to fix claim-accuracy defects, and one of them was the same figure wrong a third time. 1. THE `*.md` SKIP FIGURE IS 11, NOT 15, AND THAT IS THE FOURTH VALUE PUBLISHED FOR IT. Measured by restoring the skip byte for byte and running the scanner: 11 tracked files are opened by removing it. The other four tracked markdown files sit at the repository ROOT and are enumerated by `walkTopLevel`, a function that never carried the skip, so they were never the skip's to drop. 16, 14 and 15 each counted something else (tracked markdown at head, at base, at head again). Count what the SKIP drops, not what matches `*.md`. 2. THE PROVENANCE SENTENCE OVERCLAIMED ITS OWN CORROBORATION, so it is now field by field. 13 of the 28 numbers are corroborated in-repo (PID-3/5/7/11/13/14, NK1-2/4/5/6/16, IN1-16/36). 15 are corroborated by nothing here at all, and that is where the residual risk sits: the WHOLE GT1 row, which no in-repo source mentions even once, plus PID-6/9/19/20, NK1-30/33 and IN1-18/19. Saying "cross-corroborated in-repo" without naming which concealed exactly the numbers most likely to be the next IN1-17. 3. THE TRUNCATION DISCLOSURE CARRIED A FALSE SUB-CLAUSE. "The field it cuts in still reports" holds only when the surviving prefix clears a recogniser floor. Measured: a cut inside a family name silences that field AND everything after it, leaving only the fields before the cut. The "can only shorten, never renumber" half is true and stays. A second probe pins the corrected half. 4. THE NEW CASE'S DURABILITY CLAIM WAS FALSE, so the claim is made TRUE rather than narrowed. It named eight fields and said a field added to the table without being added to the banner would red there; adding two left the suite green. The NARROWING direction was worse: 15 of the 28 named fields fired in no test at all, so the code dropping a field while the banner kept promising it also shipped green. The case now enumerates ALL 28 positives beside the ten declared gaps, in one run, so both directions red. 5. A stale citation: the field-table header cited PV1 Chapter 3 beside segments PV1 does not appear in, while the banner says PV1 is not read at all. No behaviour changed in this commit. The gate reads exactly what the banner says it reads, in both directions, verified mechanically: named-not-read is empty and read-not-named is empty, 28 each. --- .changeset/wild-pugs-remain.md | 2 +- documentation/agent-notes.md | 27 ++++++++++---- scripts/phi-scan.ts | 13 ++++--- test/scripts/phi-scan.test.ts | 66 ++++++++++++++++++++++++++++++---- 4 files changed, 88 insertions(+), 20 deletions(-) diff --git a/.changeset/wild-pugs-remain.md b/.changeset/wild-pugs-remain.md index e0720e7..7e3ff52 100644 --- a/.changeset/wild-pugs-remain.md +++ b/.changeset/wild-pugs-remain.md @@ -16,6 +16,6 @@ One detection is subtracted and it is the only one. The package manifest carries Two further blind spots were found while grading this change and were closed rather than written down, because both reported a clean result over content a reader would expect to be caught: a person's name spelled with any character outside the plain English alphabet, and a whole message pasted into a single string with its separators written as escapes. A wrong field position was corrected too, where an insurance segment's relationship code was reported as a telephone number. -The way the gate's own limits are written changed with them, and that is the more useful half. Grading twice showed that a list of what a scanner does not catch cannot be kept true, because every clause of every segment of the standard would have to appear on it, and both versions of that list were measured incomplete in the direction that flatters the gate. The scanner now states the opposite way round: exactly which fields it reads, with anything not named there not checked. That claim can be checked against the code; the other one never could. +The way the gate's own limits are written changed with them, and that is the more useful half. Grading three times showed that a list of what a scanner does not catch cannot be kept true, because every clause of every segment of the standard would have to appear on it, and both versions of that list were measured incomplete in the direction that flatters the gate. The scanner now states the opposite way round: exactly which fields it reads, with anything not named there not checked. That claim can be checked against the code, and the suite now checks it from both sides in a single run, so a field added to the reader without being added to the statement, or dropped from the reader while the statement keeps promising it, both turn the build red. It also records which of those field numbers are corroborated by something in this repository and which are not, because that is where the remaining risk sits. Several limits stay disclosed rather than quietly closed, including that the reconciliation compares path names and not the bytes stored at them, and the complete list of what the gate still cannot see is written at the top of the scanner itself. diff --git a/documentation/agent-notes.md b/documentation/agent-notes.md index e4c7557..a633715 100644 --- a/documentation/agent-notes.md +++ b/documentation/agent-notes.md @@ -401,6 +401,13 @@ figure published before did not:** 14 paths x 4 payloads x 3 routes is 168 cells pins the BASE half, because a head-only test structurally cannot. What the suite does pin is the head side of every cell that carries the argument, each by a named case. +**▶ AND THE COVERAGE CLAIM IS PINNED IN BOTH DIRECTIONS, WHICH TOOK TWO GOES.** A first version of +that case named eight fields and claimed a field added to the table without being added to the +banner would red there. Measured: adding two did nothing, the suite stayed green. Worse was the +NARROWING direction, where the code drops a field and the banner keeps claiming it: **15 of the 28 +named fields fired in no test at all**, so that shipped green too. The case now enumerates **all +28** positives beside the declared gaps, in one run, so both directions red. + **▶ THE ONE SUBTRACTION, NAMED RATHER THAN LEFT TO BE FOUND: `pnpm phi-scan package.json` exited 1 on the npm publisher contact in its `author` field and now exits 0.** That mailbox is public, organisational and not PHI; it is **named in `scripts/phi-allow-list.txt` rather than scrubbed**, @@ -476,17 +483,23 @@ enumerates EXACTLY the fields that are read, and says that anything not named is claim is checkable; the other one was not. **Correct it by narrowing the claim, never by silently adding a field number.** -**▶ PROVENANCE, RECORDED BECAUSE ITS ABSENCE WAS THE ROOT CAUSE.** The field numbers are asserted -from HL7 v2.5.1 and were cross-corroborated **in-repo only**, against `src/messages/related-person.ts` -and the vendored `@cosyte/hl7` type surface. They were **not** checked against a published copy of -the standard, and one of them was wrong on the way here (IN1-17). That is why the table is -deliberately narrow and why widening it means citing a source, not adding a number. +**▶ PROVENANCE, RECORDED FIELD BY FIELD, BECAUSE "cross-corroborated in repo" WAS ITSELF MEASURED AS +AN OVERCLAIM.** The numbers are asserted from HL7 v2.5.1 and were **not** checked against a published +copy of the standard. **13 of the 28 are corroborated in-repo** (PID-3/5/7/11/13/14, +NK1-2/4/5/6/16, IN1-16/36) by `src/messages/patient.ts`, `src/messages/related-person.ts` and the +vendored `@cosyte/hl7` type surface. **15 are corroborated by nothing here at all, and that is where +the residual risk sits: the WHOLE GT1 row** (3, 5, 6, 7, 8, 12, 19, which no in-repo source mentions +even once), plus PID-6, PID-9, PID-19, PID-20, NK1-30, NK1-33, IN1-18 and IN1-19. One number was +wrong on the way here (IN1-17). That is why the table is deliberately narrow and why widening it +means citing a source, not adding a number. **And a fourth recogniser limit the second pass found**: a literal backslash followed by `r` or `n` inside a field value ends the segment early, because the escaped separator is also the terminator. Measured, a Windows path in PID-11 truncates there and PID-13/14 go unread. It can only SHORTEN a -segment, never renumber one (the fields before the cut keep their positions) and the field it cuts -in still reports, so it is disclosed rather than guessed at: it is not decidable from static text. +segment, **never renumber one** (the fields before the cut keep their positions). **But the field it +cuts in can go SILENT too, and the first draft of this disclosure claimed otherwise**: the surviving +prefix reports only if it still clears a recogniser floor, so a cut inside a family name loses that +field and everything after it. Not decidable from static text, so disclosed rather than guessed. **▶ AND ONE PRE-EXISTING RESIDUAL THE REFUTER NAMED: UNTRACKED content under an undeclared top-level directory is invisible to BOTH enumerating routes.** The reconciliation covers the tracked half diff --git a/scripts/phi-scan.ts b/scripts/phi-scan.ts index 3c4f5e1..5c0b8b3 100644 --- a/scripts/phi-scan.ts +++ b/scripts/phi-scan.ts @@ -94,11 +94,14 @@ * CODE values that share those component positions. * - A LITERAL BACKSLASH FOLLOWED BY `r` OR `n` INSIDE A FIELD VALUE ENDS * THE SEGMENT EARLY, because the escaped separator is also the - * terminator. Measured: a Windows path in PID-11 (`C:\records\…`) - * truncates there, so PID-13 and PID-14 go unread. It can only SHORTEN - * a segment, never renumber one, since the fields before the cut keep - * their positions; and the field it cuts in still reports. Not - * decidable from static text, so it is disclosed rather than guessed. + * terminator. Measured: a Windows path in PID-11 truncates there, so + * PID-13 and PID-14 go unread. It can only SHORTEN a segment, NEVER + * RENUMBER one, since the fields before the cut keep their positions. + * **BUT THE FIELD IT CUTS IN CAN GO SILENT TOO**, and an earlier draft + * of this line claimed otherwise: the surviving prefix reports only if + * it still clears a recogniser floor, so a cut before the digits of a + * phone, or inside a family name, loses that field AND everything after + * it. Not decidable from static text, so disclosed rather than guessed. * * Anything inside a BINARY or compressed target is unreadable to both * passes: they decode as UTF-8, and a name inside a gzip stream survives. diff --git a/test/scripts/phi-scan.test.ts b/test/scripts/phi-scan.test.ts index 295a496..08f7b9b 100644 --- a/test/scripts/phi-scan.test.ts +++ b/test/scripts/phi-scan.test.ts @@ -1319,28 +1319,60 @@ describe("phi-scan: the coverage claim is POSITIVE, and this is what makes it ch return parts.join("|"); }; - it("reads every field the banner names, and NONE that it does not", () => { + it("reads EVERY ONE of the 28 fields the banner names, and NONE that it does not", () => { + // ▶ ALL 28, NOT A SAMPLE, AND THAT IS THE FINDING. An earlier draft named + // eight and its comment claimed a field added to the table without being + // added to the banner would red here. Measured: adding PID-4 and PID-18 to + // the table left the suite 75/75 GREEN. Worse is the NARROWING direction, + // where the code drops a field and the banner goes on claiming it: 15 of + // the 28 fired in no test at all, so that shipped green too. Enumerating + // every named field is what makes the claim durable in both directions. const name = `${family}^${given}`; - // In the named set: each must report. const covered: [string, string, Record][] = [ + ["PID", "PID-3", { 3: "A77321^^^HOSP^MR" }], ["PID", "PID-5", { 5: name }], + ["PID", "PID-6", { 6: name }], ["PID", "PID-7", { 7: "19631207" }], + ["PID", "PID-9", { 9: name }], + ["PID", "PID-11", { 11: "9 Elm Rd^^Dayton^OH^45402" }], + ["PID", "PID-13", { 13: "9375550187" }], + ["PID", "PID-14", { 14: "9375550186" }], + ["PID", "PID-19", { 19: "555443210" }], + ["PID", "PID-20", { 20: "DL77321" }], ["NK1", "NK1-2", { 2: name }], + ["NK1", "NK1-4", { 4: "9 Elm Rd^^Dayton^OH^45402" }], + ["NK1", "NK1-5", { 5: "9375550188" }], + ["NK1", "NK1-6", { 6: "9375550189" }], + ["NK1", "NK1-16", { 16: "19631207" }], + ["NK1", "NK1-30", { 30: name }], ["NK1", "NK1-33", { 33: "A77321" }], ["GT1", "GT1-3", { 3: name }], + ["GT1", "GT1-5", { 5: "9 Elm Rd^^Dayton^OH^45402" }], + ["GT1", "GT1-6", { 6: "9375550190" }], + ["GT1", "GT1-7", { 7: "9375550191" }], + ["GT1", "GT1-8", { 8: "19631207" }], ["GT1", "GT1-12", { 12: "555443210" }], + ["GT1", "GT1-19", { 19: "EMP77321" }], ["IN1", "IN1-16", { 16: name }], + ["IN1", "IN1-18", { 18: "19631207" }], + ["IN1", "IN1-19", { 19: "9 Elm Rd^^Dayton^OH^45402" }], ["IN1", "IN1-36", { 36: "POL77321" }], ]; + expect(covered).toHaveLength(28); for (const [segment, label, fields] of covered) { const r = scan(`cov-${label}.ts`, `const m = "${seg(segment, fields)}";\n`); - expect(r.code, `${label} should report. stderr: ${r.stderr}`).toBe(1); + expect(r.code, `${label} is NAMED in the banner and must report. stderr: ${r.stderr}`).toBe( + 1, + ); expect(r.stderr).toContain(label); } // OUTSIDE the named set: each must be clean, and each zero is a GAP the - // banner declares, not a clearance. They run in the same suite as the - // controls above so a wholesale detector failure cannot produce them. + // banner declares, not a clearance. They run in the SAME case as the 28 + // positives above, deliberately, so a wholesale detector failure cannot + // produce them. Seven of these were measured by a refuter against a banner + // that called itself the authoritative list of this gate's limits and did + // not mention one of them. const uncovered: [string, string, Record][] = [ ["NK1", "NK1-26", { 26: name }], ["NK1", "NK1-31", { 31: "9375550188" }], @@ -1349,20 +1381,28 @@ describe("phi-scan: the coverage claim is POSITIVE, and this is what makes it ch ["GT1", "GT1-2", { 2: "G77321" }], ["GT1", "GT1-4", { 4: name }], ["IN1", "IN1-49", { 49: "MEM77321" }], + ["PID", "PID-4", { 4: "ALT77321" }], + ["PID", "PID-18", { 18: "ACC77321" }], ["PV1", "PV1-7", { 7: `1234^${family}^${given}` }], ]; for (const [segment, label, fields] of uncovered) { const r = scan(`unc-${label}.ts`, `const m = "${seg(segment, fields)}";\n`); expect(r.code, `${label} is declared OUT of scope. stderr: ${r.stderr}`).toBe(0); } - }); + }, 60_000); - it("a literal backslash before r or n truncates the segment, and only shortens it", () => { + it("a literal backslash before r or n truncates the segment, and can silence the field it cuts", () => { // The fourth recogniser limit, disclosed rather than guessed at: the escaped // separator is also the terminator, so a Windows path in an address field // ends the segment there. Pinned so the disclosure cannot outlive the // behaviour. The fields BEFORE the cut keep their positions, which is what // makes this a truncation rather than a renumbering. + // + // ▶ AND THE SECOND HALF, WHICH AN EARLIER DRAFT OF THE DISCLOSURE GOT WRONG + // IN THE FLATTERING DIRECTION: it claimed the field the cut lands in still + // reports. It reports only if the surviving prefix still clears a + // recogniser floor. The second probe below cuts inside a family name, and + // that field goes silent along with everything after it. const winPath = ["C:", "records", "scan.tif"].join("\\\\"); const r = scan( "backslash.ts", @@ -1375,5 +1415,17 @@ describe("phi-scan: the coverage claim is POSITIVE, and this is what makes it ch expect(r.stderr).toContain("PID-11"); // And the truncation itself: PID-13 is past the cut. expect(r.stderr).not.toContain("PID-13"); + + // A cut INSIDE a name: the prefix no longer clears the name-token floor, so + // PID-5 itself goes silent and so does every field after it. Only PID-3, + // which precedes the cut, survives. + const cut = scan( + "backslash-name.ts", + `const p = "${seg("PID", { 3: "A77321^^^HOSP^MR", 5: "O\\rourke^Sean", 7: "19631207", 13: "5551230000" })}";\n`, + ); + expect(cut.code, `stderr: ${cut.stderr}`).toBe(1); + expect(cut.stderr).toContain("PID-3"); + expect(cut.stderr).not.toContain("PID-5"); + expect(cut.stderr).not.toContain("PID-7"); }); });