diff --git a/.changeset/phi-scan-widen-the-walk-roots.md b/.changeset/phi-scan-widen-the-walk-roots.md new file mode 100644 index 0000000..beb590d --- /dev/null +++ b/.changeset/phi-scan-widen-the-walk-roots.md @@ -0,0 +1,40 @@ +--- +"@cosyte/cli": patch +--- + +The PHI scanner's all-mode walk now covers this package's whole authored corpus. It was rooted at +`test/__fixtures__` and `src` only, so 89 of 123 tracked files were scanned by neither of its two +routes; it now roots at `src`, `test` and `scripts`, opening 72 tracked files instead of 34. + +Measured back to back on the base commit rather than inferred: a dashed SSN and an off-domain address +written into a file under `test/`, in this package's own inline-message shape (a whole HL7 message as +one TypeScript string literal with escape sequences between its segments), exited 0 with `OK, no +hits` in all mode while naming the same file explicitly reported both at exit 1 over the same bytes. +A file written under `scripts/` behaved identically. Both routes now report both. Every one of the 38 +newly opened files was read by hand: every message literal is a placeholder and the only SSN and +email shapes anywhere are the scanner's own declared synthetic payload, so the gap was one of +enumeration rather than a live exposure. + +`test` replaces `test/__fixtures__` rather than joining it, because the roots must stay disjoint: each +is walked independently and the results concatenated, so a nested root would enumerate every file +beneath it twice. The fixture directory is still watched, through the other condition of the +unobserved-root rule, wherever the repository tracks files under it. One cover was lost and is stated +rather than implied away: where nothing is tracked under it, an empty fixture directory no longer +refuses. `scripts/` is included because the scanner, its allow-list and its override log all live +there, so the one directory guaranteed to hold identifier-shaped text was the one nothing enumerated; +all nine files there were measured against the detector before the root was declared. + +Scanning `scripts/` has one consequence worth knowing before it surprises someone: the allow-list +documents an id entry as matching a social-security, medical-record or member-id shape, and the +dashed-id check consults no allow-list, so declaring one in the dashed shape now reports a hit on the +allow-list itself. Nothing shipped is affected, and the remedy is to write the synthetic id in a shape +the check does not match, never to weaken the check or to exempt the file. + +The scanner's own test file carries violator literals on purpose and is the single exempt path. That +exemption is applied after the file is read, so it still counts as observed and an unreadable one +still refuses; it is scoped to the sweep, so naming the file explicitly still reports every hit; and +it is per path rather than a pattern. + +What this does not change: the detector is still the cross-cutting SSN and email floor, now over 38 +more files, and structured field-level detection remains unimplemented. A test pins that limit. +`--staged` is unchanged, because widening it would change what a commit is blocked on. diff --git a/CHANGELOG.md b/CHANGELOG.md index eac0a9d..24b97aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,57 @@ still do. Each entry was assigned to the release whose tag first contains it, re ### Fixed +- **`pnpm phi-scan`'s all-mode walk was rooted at `test/__fixtures__` and `src` only, so 89 of this + repository's 123 tracked files were scanned by NEITHER of its two routes + (PHI-SCAN-WALK-ROOT-SCOPE).** The walk now roots at **`src`, `test` and `scripts`**, which opens 72 + tracked files instead of 34. + - **Measured back to back on the base commit, and re-derived for this repository rather than ported + from a sibling.** A dashed SSN and an off-domain address written into `test/planted.test.ts`, in + this repository's own inline-message shape (a whole HL7 message as one `.ts` string literal with + `\r` escapes between its segments), exited **0** with `OK, no hits` in all mode, while naming the + same file in paths mode reported both at **exit 1** over the same bytes. A file written under + `scripts/` behaved identically. Both routes now report both. + - **All 38 newly opened files were hand-read.** Every message literal is a placeholder and the only + SSN/email shapes anywhere are the scanner's own declared synthetic payload, so the gap was one of + **enumeration**, not a live exposure: the defect was that the gate could not see those files, so + nothing would have caught a real value if one appeared. + - **`test` REPLACES `test/__fixtures__` rather than joining it.** Roots must stay disjoint: each is + walked independently and the results concatenated, so a nested root would enumerate every file + beneath it twice and report each hit twice. The fixture directory is still watched, through the + observation rule's other condition, **wherever git tracks files under it** (here, seven); only the + root a refusal is filed under changed. + - **One cover was LOST, and it is stated rather than implied away:** where git tracks NOTHING under + `test/__fixtures__`, an empty one no longer refuses. As a declared root it refused by the + opened-nothing floor whatever git carried; as an ordinary directory it contributes no entry and the + reconciliation has no expected path to miss. A test pins it. + - **`scripts/` is a root because the scanner, its allow-list and its override log live there**, so + the one directory guaranteed to hold PHI-shaped text was the one nothing enumerated. All nine + files there were measured against the detector before the root was declared: no hits. + - **The cost of that root nobody would guess: the allow-list's own bytes.** The allow-list documents + `ID ` as a synthetic id "matching an SSN / MRN / member-id shape", and the dashed-SSN check + consults no allow-list, so an `ID` entry written in that dashed shape now reds the gate on the + allow-list itself. Latent today (the shipped file declares its only id in the `MRN-` form, and a + test pins that). **The remedy is to spell the id in a shape the check does not match**, never to + make that check consult the allow-list (which would delete a detection) and never to exempt the + file (which would leave the likeliest place for a real value unswept). + - **A narrowing that came with it:** `test/__fixtures__` is no longer a declared root, so a live or + dangling symbolic link at that path is now an enumerated entry and is refused outright, whatever + it points at. Only the three top-level roots can still be followed. + - **`test/scripts/phi-scan.test.ts` is exempt from the sweep, and is the only exempt path.** It + carries violator literals on purpose, as the positive half of the scanner's own tests. The + exemption is applied **after the read**, so the file still counts as observed and an unreadable + one still refuses; it is **scoped to the sweep**, so naming the file explicitly still reports + every hit; and it is **per path, never a pattern**. + - **What this does NOT buy, stated because a wider reading would be false:** the detector is still + the cross-cutting SSN + email floor, over 38 more files. Structured, field-level detection remains + unimplemented, and a test now pins that limit rather than leaving it as prose. + - **`--staged` is deliberately unchanged**, because widening it changes what a commit is blocked on. + The two routes therefore differ widely, and the scanner's own documentation says by how much. + - **Four claims about which directories are deliberately NOT scan roots are now checks rather than + prose**, after two of them were measured false: `docs-content/` and `.changeset/` each carry one + tracked non-markdown file, and the repository root is not clean under the detector (the `author` + field carries a real off-domain contact address). `documentation/` and `.github/` were correct. + - **`pnpm phi-scan` printed `OK, no hits` and exited 0 over a corpus it never opened (PHI-SCAN-OBSERVED-NOTHING-IS-GLOBAL).** A declared scan root that the walk never observed is now a refusal at **exit 2**, in the all-mode sweep CI runs. Each root's walk is reconciled against diff --git a/CLAUDE.md b/CLAUDE.md index 4754294..912571a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,10 +1,9 @@ # @cosyte/cli: Project Guide for Claude > **The narrative lives in [`documentation/agent-notes.md`](documentation/agent-notes.md). Read it -> before you touch anything a rule below tells you not to touch.** On 2026-08-04 this file was 51,276 -> bytes and it is always-read by every worker that enters this repo, so the per-incident write-ups, -> the shipped-phase histories and the long rationales were relocated there **verbatim**, under -> headings that say where they came from. Nothing was deleted. +> before you touch anything a rule below tells you not to touch.** This file is always-read by every +> worker that enters this repo, so the per-incident write-ups, the shipped-phase histories and the +> long rationales were relocated there **verbatim**; nothing was deleted. > > What stays here is the cursor, the rules, and **every** trap, each compressed to a one-line > imperative with a link to the section that proves it. **"I did not read the reason" is not a licence @@ -33,25 +32,24 @@ subpath still exports a small programmatic `core` API (`detectFormat`, `EXIT`, ` ## Status -**Feature-complete.** Phase 7 (release hardening) shipped and was the roadmap's final phase; no new -runtime command surface is planned. The CLI wraps **all eight** cosyte formats through one lazy +**Feature-complete.** The roadmap's final phase shipped; no new runtime command surface is planned. +The CLI wraps **all eight** cosyte formats through one lazy per-format adapter registry (`src/core/parsers.ts`), exposes the same `core` through a terminal bin (`cosyte`), an MCP server bin (`cosyte-mcp`) and the `.` / `./mcp` subpath exports, and states support **per (format, operation)** via `OP_SUPPORT`: an unsupported cell is a value-free `CLI_FORMAT_UNSUPPORTED`, never a fake. Exit-code contract: `0/1/2/65/66/69/70`. Per-phase histories: [agent-notes § Shipped phases](documentation/agent-notes.md#shipped-phases). -**Deferred, honestly and never faked:** `dicom` `parse`/`fmt` (binary model), `ccda` `parse` (XML is -the canonical `fmt` surface), `mllp` `fmt`/`validate`; `redact`/`deid` and `map-codes` MCP tools and -remote/HTTP MCP, deliberately not yet exposed; `redact`'s real de-identification (gated stub + seam -landed, waiting on `@cosyte/deid`, and **never a built-in partial scrub**, which would risk a -false-safety impression); `validate --profile` (reserved, `CLI_NOT_IMPLEMENTED`/`69`, no profiles -bundled). Detail: +**Deferred, honestly and never faked:** `dicom` `parse`/`fmt`, `ccda` `parse`, `mllp` +`fmt`/`validate`; `redact`/`deid` and `map-codes` MCP tools and remote/HTTP MCP; `redact`'s real +de-identification (gated stub + seam landed, waiting on `@cosyte/deid`, and **never a built-in +partial scrub**, which would risk a false-safety impression); `validate --profile` (reserved, +`CLI_NOT_IMPLEMENTED`/`69`). Detail: [agent-notes § Deferred](documentation/agent-notes.md#deferred). -**ADRs:** `documentation/decisions/0021` (a `bin` hard-deps first-party siblings), `0022` (one repo, -two bins over one core), `0023` (wire `transform` + `terminology`; the 2 → 4 dep-cap raise), `0024` -(the MCP SDK as an isolated runtime-optional dep), `0025` (breadth parsers optional, outside the cap). +**ADRs:** `documentation/decisions/0021` (a `bin` hard-deps first-party siblings), `0022` (two bins, +one core), `0023` (wire `transform` + `terminology`; 2 → 4 dep cap), `0024` (MCP SDK isolated and +runtime-optional), `0025` (breadth parsers optional, outside the cap). Summaries: [agent-notes § ADRs](documentation/agent-notes.md#adrs). ### The published package, and the FHIR hole (live, unresolved) @@ -89,13 +87,11 @@ Why: [agent-notes § The vendor to npm dependency swap](documentation/agent-note deliberately not added** (`npm install -g` would claim the name `cli` on the user's `PATH`): founder call, not an oversight. - **The public-flip stop is not yours to cross, and both original stops are already behind this - package.** It is - **public** (`gh repo view cosyte/cli --json visibility`) and it has published, so the original - "founder-gated tail (NOT crossed)" note is superseded, not still pending. **Flipping a repo's + package.** It is **public** (`gh repo view cosyte/cli --json visibility`) and it has published, so + the "founder-gated tail (NOT crossed)" note is superseded, not pending. **Flipping a repo's visibility is never waived**, so an agent still does not touch it; the `npm publish` half is - covered by a standing founder directive. The vendored `file:` sibling deps that gated a publish (a - published package cannot ship a `file:` dep) are already real npm ranges, except `@cosyte/fhir`, - which stays a `file:` **devDependency** and never reaches a consumer's install: see the swap note. + covered by a standing founder directive. The vendored `file:` deps that gated a publish are already + real npm ranges, except `@cosyte/fhir`, a `file:` **devDependency** no consumer install resolves. ### Hard runtime deps @@ -114,10 +110,10 @@ Why: [agent-notes § The vendor to npm dependency swap](documentation/agent-note Why: [agent-notes § The docs sidebar and the IA spine](documentation/agent-notes.md#the-docs-sidebar-and-the-ia-spine). - **An off-spine top-level label in `docs-content/sidebars.json` stops the WHOLE docs site - deploying**, and this package once held it down for four days. Canonical top-level order: - `Overview` (the `intro` **doc reference**, not a category), `Installation`, `Quickstart`, - `Core Concepts`, `Guides`, `API Reference`, `Troubleshooting`. Categories are **optional**; the rule - is that whatever you have is labelled and ordered canonically, so `{"docs":["intro"]}` is compliant. + deploying**, and this package once held it down for four days. Canonical top-level order (verbatim + in the linked section, starting `Overview` as the `intro` **doc reference**, not a category). + Categories are **optional**; the rule is that whatever you have is labelled and ordered + canonically, so `{"docs":["intro"]}` is compliant. - **🔴 NEVER AUTHOR AN `API Reference` CATEGORY.** The docs site injects it. A hand-authored one is a distinct, **harder** error than the off-spine label it would be replacing. - **Never claim where that injected category lands.** A refuter falsified "just before @@ -179,9 +175,21 @@ Why: [agent-notes § The pre-commit PHI gate and git mv](documentation/agent-not measured; an **ancestor** of a scan root is in neither route's scope; paths mode follows a named link). **Do not "fix" them inside an unrelated slice**: what is left needs a decision about how far ABOVE a root to look. -- **Other residuals, also not closed:** `D` and `U` are unenumerated (`U` costs nothing that can reach - a commit: `git commit` refuses an unmerged index, exit 128); under `src/` the staged route covers - `.ts` only while the all-mode walk covers every non-`.md` file, and the CI sweep is the cover. +- **Walk roots are `src`, `test`, `scripts`, re-derived; `test` REPLACED `test/__fixtures__` and roots + must stay DISJOINT** (a nested one double-reports). `scripts` is in, so **an example SSN in a + comment there reds the gate**. `test/scripts/phi-scan.test.ts` is the **ONE** exempt path: at the + **scan** (still read + reconciled), **all-mode only** (paths mode must still report it or a + detection is DELETED), **per path**. `EMAILDOMAIN` is global; never allow-list to green a file. +- **`scripts/` IS SWEPT: name a PHI shape, never SPELL a literal** (a draft banner red the gate on + itself). The SSN check reads **no** allow-list, so an `ID` in dashed shape reds + `phi-allow-list.txt`: respell `MRN-`. +- **🛑 THE WIDENING BOUGHT THE SSN/EMAIL FLOOR OVER 38 MORE FILES AND NOTHING ELSE** (all hand-read: + enumeration gap, not exposure). **The recogniser was NOT widened, on measurement** - this floor is + anchor-free, so it never had the "file IS the document" defect, and an escape-decoded view finds + nothing new. **A tripwire reds if that changes**; widen **in addition to** the raw pass. +- **Other residuals:** `D` and `U` are unenumerated (`U` costs nothing that can reach a commit: + `git commit` refuses an unmerged index, exit 128); the routes now differ widely: `--staged` is + `test/__fixtures__` + `src/*.ts` only, CI sweeps the rest. - **Give `test/scripts/phi-scan.test.ts` explicit timeouts.** Each case spawns `tsx` cold: 0.5s idle, **3.7s under contention**, against a shared 10s default. - **Assert the premise, not only the remedy.** Two vacuity traps already sprang in this suite: a @@ -198,8 +206,8 @@ Why: [agent-notes § The em-dash brand gate](documentation/agent-notes.md#the-em - **▶ READ THIS BEFORE PORTING THE GATE OR SWEEPING ANY REPO: AN EM DASH IS SOMETIMES A VALUE, NOT PUNCTUATION.** `docs-content/limitations.md` used a bare `U+2014` as a support-matrix **cell value meaning "not supported"**; the sweep rewrote it as punctuation and turned **"support absent" into - "support unstated"**, on the one page whose entire job is honest capability disclosure, reading as a - rendering artifact rather than a claim. **Grep for a cell or list-marker em dash first** + "support unstated"**, on the one page whose whole job is honest capability disclosure. + **Grep for a cell or list-marker em dash first** (`\|\s*\x{2014}\s*[\|\(]`) and convert each to a **WORD**, by hand, before any bulk transform. - **CUT, do not rewrite.** Softening a stated limit into an implied capability while tidying a sentence is a worse defect than the thing being removed. Revert a rewrite verbatim rather than @@ -278,11 +286,10 @@ Full ruleset, the required-context table and the per-check reasoning: merge**, which is the failure this repo's whole protection claim exists to prevent. `ci / prepublish` arrived that way on 2026-08-05 and was unrequired until measured and added. **Census `ci / *` against a real check run whenever `.github` moves**, then require it or write down why not, in `ci.yml`'s - banner. Detail: - [agent-notes § Branch protection](documentation/agent-notes.md#branch-protection-and-the-limits-of-this-claim). -- **`no-internal-refs` and `no-emdash` are bare JOB IDS**, not ` / `, because they are - ordinary jobs in this repo's own workflows. **Renaming the job silently detaches the required - check.** Rename the job and the ruleset together, or neither. + banner. +- **`no-internal-refs` and `no-emdash` are bare JOB IDS**, not ` / `: they are ordinary + jobs in this repo's own workflows. **Renaming the job silently detaches the required check.** Rename + the job and the ruleset together, or neither. - **A required job gates all of its steps.** Splitting a step out of `ci / verify` into its own job silently un-requires it. There is a banner on `ci.yml` where someone would trip it. - **Never add a `paths:` filter to `ci.yml`, `codeql.yml`, `no-internal-refs.yml` or @@ -337,16 +344,14 @@ return 0`, so no `--profile`, `--ignore-rules` or config setting reaches that ea - **Only a TOTAL loss of declarations is the false green; a PARTIAL one `attw` catches itself**, so the preflight must report both outcomes and **must not assert the exit 0**. Six packed-but-undeclared declarations decide which silence you get, and the obvious two-line version of this is false: a - first draft named one file, measured it on a throwaway fixture, wrote the fixture's result down as - this tree's, and a refuter falsified it in one run. **Re-measure before you shorten it.** + refuter falsified a first draft of it in one run. **Re-measure before you shorten it.** - **The post-check reads a string, so what would hide that string is refused**: `--quiet`, `-q`, `--format`, `-f`, `--config-path`, and a `.attw.json` setting `quiet` or `format`. **Say "exact argv token" of the ARGV refusal, never "wholesale"** (the stronger wording was live and was refuted); the `.attw.json` refusal **is** wholesale, and the two messages differ on purpose. - **Two holes are disclosed and deliberately left open**: clustered short forms `-fjson` / `-Pf json`, - and a declared path not starting with `.`. The bare invocation this replaced exited 0 with **no** - arguments at all, so the gate is strictly better either way and a short-option parser is a moving - part the guard does not need. + and a declared path not starting with `.`. Left open on purpose; the reasoning is in the linked + section. - `test/scripts/attw-gate.test.ts` pins both nets, the upstream exit 0 itself, a negative control, and that a real `attw` failure still fails with `attw`'s own status. @@ -386,16 +391,15 @@ Mirrors the three disciplines in the meta-repo's `documentation/conventions.md`. **shown**. Two sentences that read well and are **false**: "everything in `src/` ships", and "the bundles carry `//` comments verbatim". - **Never re-key the gate on the `WORD-N` shape.** This repo is where that trap is widest, because - the CLI reaches for all eight formats' vocabularies at once: `HL7-V2`, `FHIR-R4`, `DICOM-SR`, - `NCPDP-SCRIPT`, `X12-837P`, `CCDA-R2.1`, `MSH-2`, `NM1-03`, `ST-01`, `439-E4`, `ICD-10-CM` are - reference material a consumer came here for. The negative self-tests exist to make that attempt - red. + the CLI reaches for all eight formats' vocabularies at once, and every designation, segment, + element and code-system reference among them (listed in the linked section) is material a + consumer came here for. The negative self-tests exist to make that attempt red. - **Repair the head**: a sentence with an identifier stripped off the front reads worse than the text it replaced. - - **CUT, do not rewrite.** This package's whole posture is honesty about what it _cannot_ do: - gated stubs that exit `69`, value-free stderr, the `OP_SUPPORT` matrix. Softening a stated limit - into an implied capability while tidying a sentence is a worse defect than the bookkeeping being - removed. Delete the claim rather than replace it, and revert a rewrite verbatim. + - **CUT, do not rewrite.** This package's posture is honesty about what it _cannot_ do (gated + stubs at `69`, value-free stderr, `OP_SUPPORT`). Softening a stated limit into an implied + capability while tidying is worse than the bookkeeping removed. **Delete the claim rather than + replace it, and revert a rewrite verbatim.** - **The gate catches identifiers, not English sentences about our process**, and it reads `src/`, never `dist/`. A new programme prefix has to be added by hand. **The reviewer still owns half the rule.** diff --git a/documentation/agent-notes.md b/documentation/agent-notes.md index 68e67cd..afb4100 100644 --- a/documentation/agent-notes.md +++ b/documentation/agent-notes.md @@ -216,6 +216,162 @@ an unmerged `U` entry is already pinned as out of scope and unable to reach a co reader **was** open: a present-but-unreadable `phi-scan-overrides.md` threw a raw `EACCES` past every handler and node exited **1**, this contract's code for "hits found". Now exit 2. +### Widening the walk to this repository's whole authored corpus (2026-08-07) + +`PHI-SCAN-WALK-ROOT-SCOPE` in `cli`. The other half of the item above: the observation rule made the +gate refuse a root it never opened, and this one moved the roots so that they cover the files that +actually exist. + +**The four numbers, re-derived on `ba059a2` rather than ported.** 123 tracked files; **34** opened by +the all-mode walk (7 `test/__fixtures__` + 27 `src/`); **89** scanned by **neither** route; **7** of +those carrying an inline HL7 `PID|` literal. The sibling slice measured 122/34/88/6 at `cd221a0`; the +one-file and one-literal deltas are its own changeset file and its own narrative in +`documentation/agent-notes.md`, which is a small illustration of the point that this set grows on its +own. **All 38 files the widening newly opens were hand-read**: every message literal is a placeholder +(`DOE^JANE`, `X^^^H^MR`, `SENDER`, `ZZSENTINEL*`) and the only SSN/email shapes anywhere are this +scanner's own declared synthetic payload. **So the 89 were an ENUMERATION gap, not a live PHI +exposure** - the defect is that the gate could not SEE those files, so nothing would have caught a +real value if one appeared. + +**RED before, GREEN after, measured back to back on `ba059a2`.** A dashed SSN and an off-domain +address written into `test/planted.test.ts`, in this repo's own inline-message shape (a whole HL7 +message as one `.ts` string literal with `\r` escapes between segments), exited **0** `OK, no hits` +in all mode, while `phi-scan test/planted.test.ts` reported both at **exit 1** over the same bytes. A +file written to `scripts/` behaved identically. After the change both routes report both hits. +**Sixteen of the suite's 74 cases red against `ba059a2`'s scanner**, re-derived by swapping that +scanner under the head suite rather than quoted from a first draft (which read 14 of 68, before the +gate pass below added cases). + +**The roots are `src`, `test`, `scripts`, re-derived from this repository's own files.** `test` +REPLACES `test/__fixtures__` rather than joining it: `buildTargetsForAll` walks each root +independently and concatenates, so a nested root would enumerate every file beneath it twice and +report each hit twice. The fixture directory did not stop being watched - an emptied or missing one is +still refused, through the observation rule's OTHER condition (git tracks in-scope files under `test` +that the walk did not open), and the refusal still names each one. Only the root the message is filed +under changed, which is what the four updated assertions in the suite are. + +**But that cover is CONDITIONAL, and the first draft of the sentence above stated it flat.** It holds +where git tracks files under the fixture directory, which here is seven. Where git tracks NOTHING +under it, the cover is gone: as a declared root an empty `test/__fixtures__` refused by the +opened-NOTHING floor whatever git carried, and as an ordinary directory it contributes no entry and +the reconciliation has no expected path to miss. Measured on a scratch repo tracking `src/ok.ts` and +`test/foo.test.ts` only: `ba059a2` exits **2**, this exits **0**. Arguably the more correct answer, +but it is a LOSS and a test now pins it. + +**`scripts` is included, and that is this repository's answer rather than a sibling's.** The +recogniser's patterns, the allow-list the scanner refuses to run without, and the override log it +points a developer at all live there, so the one directory guaranteed to hold PHI-shaped text was the +one nothing enumerated. All nine files were measured against the floor before the root was declared: +zero hits, so the widening lands green on its own bytes rather than on a new carve-out. **Keep it that +way**: `scripts/phi-scan.ts` is now under its own scan, so an example SSN written into a comment there +reds the gate. **That is not hypothetical: the gate pass below caught this worker writing exactly such +a literal into the scanner's own banner, and the sweep reported a hit on itself.** The banner now +names the shapes without spelling them; the literals live in the exempt test file. + +**▶ THE COST OF THE `scripts` ROOT THAT NOBODY WOULD GUESS: THE ALLOW-LIST'S OWN BYTES.** +`phi-allow-list.txt` documents `ID ` as "synthetic id matching an SSN / MRN / member-id shape", +and the dashed-SSN check consults **no allow-list at all**. So an `ID` entry written in that dashed +shape now reds the gate on the allow-list itself: measured, it exits **0** on `ba059a2` and **1** +here. Latent today (the shipped file declares its only id in the `MRN-` form, and a test pins that), +and it stops being latent the moment a worker acts on `scanTarget`'s TODO to add the structured id +detector that consumes `allow.ids`. **The remedy is to spell the id in a shape the floor does not +match.** It is NOT to make the SSN pass consult `allow.ids` (that DELETES a detection the base had), +and it is NOT to exempt the allow-list (that leaves the one file a developer is likeliest to paste a +real value into unswept). **Neither remedy the hit message itself prints works here**: "declare it in +the allow-list" is circular, and `--allow-fixture` routes the invocation down paths mode where the one +named target is then filtered out, so it opens ZERO files and prints `OK, no hits`. That collapse is +**PRE-EXISTING** (identical on `ba059a2`), disclosed and pinned rather than fixed. + +**What is deliberately NOT a root, and TWO OF THE THREE ORIGINAL MEASUREMENTS WERE FALSE.** `vendor/` +(ten `pnpm pack` tarballs; a DEFLATE stream decoded as UTF-8 is not text this gate can say anything +true about, and the em-dash gate's NUL grounding cites them). `docs-content/`, `documentation/` and +`.changeset/`: the first draft said every tracked file under them is `.md` so declaring them would +open not one byte, and that is **wrong** - `documentation/` is 6 of 6, but `docs-content/` is 9 of 10 +(`sidebars.json`) and `.changeset/` is 3 of 4 (`config.json`), so declaring all three opens **two** +files. `.github/`: 8 files, measured clean. **The repository root is NOT clean**, which the first draft +also got wrong: `phi-scan package.json` exits **1** on the brand's own contact address in the `author` +field, so rooting there would red today on a correct value, on top of being the one sibling shape that +got caught enumerating a build transient. **All four are now checks in the suite rather than sentences +a reader has to trust** - this is the third time this repository has been caught writing down a +measurement it did not take. + +**The deliberate-violator exemption, one entry long.** `test/scripts/phi-scan.test.ts` carries the +payload on purpose, so with `test/` a root the sweep would red on the scanner's own suite forever. +Three properties, each pinned by a control that reds when it is removed: + +- **Applied at the SCAN, not the enumeration.** The file is still walked, still READ, and still counts + as observed and reconciled. An unreadable one still refuses at exit 2 rather than passing as exempt. +- **Scoped to the all-mode sweep.** `phi-scan test/scripts/phi-scan.test.ts` still reports every hit + at exit 1. An unscoped exemption would DELETE a detection the base had, which is "instead of" where + this work is only ever "in addition to"; a sibling shipped that mistake before catching it. + Measured: making it unscoped reds exactly the case that asserts the scoping. +- **Per PATH, never a pattern.** An extension rule cannot tell a file carrying violator literals on + purpose from one carrying them by accident. The same payload in `test/scripts/other.test.ts` reds. + +**Allow-listing the values instead was refused, and the reason is the email half.** `EMAILDOMAIN` is +global, so declaring `hospital.org` would switch the email detector off for the whole corpus while +this file's own positive case asserts that exact address IS reported. The dashed-SSN check consults no +allow-list at all, so there is no token-level route for that half either. + +**A NARROWING the widening bought, asserted rather than claimed.** `test/__fixtures__` is no longer a +ROOT, so a live or dangling link AT that path is now an ENUMERATED entry beneath `test` and is refused +by the not-a-regular-file rule, whatever it points at and whatever git tracks. Only the three +top-level roots are still followable. The two residual pins were retargeted one level up for exactly +this reason, and a new case pins the narrowing. + +**🛑 WHAT THIS BOUGHT, AND WHAT IT DID NOT.** It bought the SSN/email floor over 38 more files and +**nothing else**. The structured, field-level detection this scanner needs before it can be called a +PHI gate is still the unimplemented TODO in `scanTarget`, and opening a file does not implement it. A +test pins that limit rather than leaving it as prose: an undashed nine-digit id, a name, a DOB and an +address in the same `PID` segment all go unreported, at exit 0. + +**The recogniser was NOT widened, and that is a measurement rather than an omission.** The companion +defect this class carries is that a recogniser assumes **the file IS the document**, so enumerating a +`.ts` source whose message is an inline string literal buys nothing. That failure mode needs an +ANCHORED detector, and this scanner has none: `scanCommonShapes` is two unanchored `matchAll` passes +over the whole text, so its reach over a `PID|` literal embedded in TypeScript is identical to its +reach over a standalone `.hl7` fixture. Pinned by an anchor-free probe that puts one token in three +placements (a standalone document, an inline HL7 literal with `\r` escapes, a multi-line template +literal) and requires all three to red. + +**The one widening a sibling shipped here was measured and DECLINED: the escape-decoded second view** +(`\x2d` and friends, which hide a token from a raw text pass). Run over every file this widening newly +opens, the decoded view finds nothing the raw view does not and loses nothing either: this +repository's sources spell their messages literally and use `\r`/`\n`/`\t` alone. Porting it would +have been a guard with no measurement behind it. **The measurement is pinned as a TRIPWIRE rather than +asserted once**: the suite runs both views over the whole newly-opened corpus and REDS if they ever +disagree, which is the signal that the next worker should widen (in ADDITION to the raw pass, never +instead of it). A negative control proves the tripwire can see a difference, and the tripwire's +regexes are deliberately a SECOND COPY of the floor's, because one that imported them would go quiet +in exactly the case where the floor itself was narrowed. + +**`--staged` IS UNCHANGED, DELIBERATELY.** Widening the walk changes what CI sweeps; widening +`--staged` changes what a COMMIT is BLOCKED on, which is a hook decision. So `test/*.ts`, +`test/scripts/**` and everything under `scripts/` are swept by the all-mode route and enumerated by +neither of `--staged`'s predicates. The CI sweep is the cover, exactly as it already was for the +non-`.ts` half of `src/`. The two routes now disagree by a lot more than they used to, and the module +header says so rather than leaving it to be inferred. + +**The path-SET escape is unchanged and still open.** The reconciliation compares path sets, not the +bytes git carries at those paths, so a root swapped for a directory mirroring the tracked *names* +still exits 0 over decoy contents. Comparing blobs is a larger rule and is deliberately not taken +here; the widening does not make it worse, it only moves which paths the decoy has to mirror. + +**Re-measured rather than inherited:** both named `PRE-EXISTING` minors (`loadAllowList`/`readdirSync` +throwing the "hits found" code; unmerged `U` entries enumerated by neither `AM` nor `AMT`) are still +**NOT open here**, and their sibling reader is already fixed. `verify.sh cli` now fails **two** +pre-existing steps rather than one: `pnpm audit --prod --audit-level high` (all advisories transitive +under `@modelcontextprotocol/sdk`) and the licenses gate, which reports +`ERR_PNPM_MISSING_PACKAGE_INDEX_FILE` for the vendored `@cosyte/fhir` tarball. **Both reproduce +byte-identically on a base tree** restored by file copy, `package.json` and `pnpm-lock.yaml` are +untouched by this change, and **no CI job runs either command**. The licenses one is newly VISIBLE +rather than newly broken: the umbrella's ladder used to print green on that step without running it. + +**Still live and not this slice's:** `test:fuzz` and `pack:docs` are real gates the umbrella's verify +ladder never names, so they are invisible rather than skipped. And `.github/workflows/ci.yml`'s banner +says the ruleset requires **four** contexts while it requires **seven** - flagged here, not fixed, +because it is its own item. + ### The em-dash brand gate - **Em-dash brand gate armed, and unlike most siblings this repo was NOT clean when it landed.** diff --git a/scripts/phi-scan.ts b/scripts/phi-scan.ts index 8832984..7a6341c 100644 --- a/scripts/phi-scan.ts +++ b/scripts/phi-scan.ts @@ -3,10 +3,10 @@ * `@cosyte/cli` PHI scanner: the CI / pre-commit half of the PHI commit-gate. * * Pure Node. Zero runtime deps. `git` is the only subprocess, always via - * `execFileSync` with array args (never shell-form). Walks the synthetic test - * fixtures (and a conservative text pass over `src/`) and REFUSES anything that - * looks like real PHI, so a developer cannot commit a real-looking fixture by - * accident. + * `execFileSync` with array args (never shell-form). Walks this repository's + * authored corpus (`src/`, `test/` and `scripts/`, see `SCAN_ROOTS`) and REFUSES + * anything that looks like real PHI, so a developer cannot commit a real-looking + * fixture by accident. * * =========================================================================== * ██ STARTER: READ BEFORE YOU RELY ON THIS ████████████████████████████████ @@ -89,30 +89,36 @@ * * 1. A SCAN ROOT THAT IS ITSELF A LIVE LINK IS STILL FOLLOWED, in the all-mode * walk: `existsSync` and `readdirSync` both resolve links. Following is not - * what got fixed; being unable to TELL is. With `test/__fixtures__` - * pointing at a directory outside the repository the walk reads files no - * commit contains and would report their values under a FABRICATED in-repo - * path that holds no such file - a confident wrong provenance, on the same - * channel this banner argues is itself a PHI surface. The observation rule - * now REFUSES that (exit 2, measured) whenever git tracks an in-scope file - * under the root that the link's target does not also carry at the same - * relative path: seven files here, so a link to an UNRELATED directory - * refuses here. STILL OPEN, AND STATED RATHER THAN IMPLIED AWAY, BECAUSE - * THE SHORTER VERSION OF THIS SENTENCE IS FALSE AND WAS MEASURED FALSE: the - * reconciliation compares PATH SETS, not the bytes git carries at those - * paths, so a target directory that mirrors the tracked NAMES satisfies both - * conditions and is followed silently, decoy contents and all - measured at - * exit 0 over this repo's own seven tracked fixture names. A root git tracks - * NOTHING under is the degenerate case of that, not the whole of it. The - * DANGLING direction IS closed outright, whatever is tracked, because it - * opens nothing. + * what got fixed; being unable to TELL is. With a root pointing at a + * directory outside the repository the walk reads files no commit contains + * and would report their values under a FABRICATED in-repo path that holds + * no such file - a confident wrong provenance, on the same channel this + * banner argues is itself a PHI surface. The observation rule now REFUSES + * that (exit 2, measured) whenever git tracks an in-scope file under the + * root that the link's target does not also carry at the same relative + * path, and every one of this repo's three roots tracks many, so a link to + * an UNRELATED directory refuses at each of them. STILL OPEN, AND STATED + * RATHER THAN IMPLIED AWAY, BECAUSE THE SHORTER VERSION OF THIS SENTENCE IS + * FALSE AND WAS MEASURED FALSE: the reconciliation compares PATH SETS, not + * the bytes git carries at those paths, so a target directory that mirrors + * the tracked NAMES satisfies both conditions and is followed silently, + * decoy contents and all - measured at exit 0. A root git tracks NOTHING + * under is the degenerate case of that, not the whole of it. The DANGLING + * direction IS closed outright, whatever is tracked, because it opens + * nothing. WHAT THE WIDENING TO `test/` DID CHANGE HERE, and it is a + * narrowing rather than a fix: `test/__fixtures__` is no longer a ROOT, so + * a live link AT that path is now an ENUMERATED entry beneath `test/` and + * is refused by the not-a-regular-file rule above, whatever it points at + * and whatever git tracks. Only the three top-level roots are still + * followable, and only they are what this shape now describes. * 2. AN ANCESTOR of a scan root is in neither route's scope. Fact 3 below puts - * `test/__fixtures__` and `src` in scope, but not `test`, so staging `test` - * as a link leaves `--staged` at exit 0 (measured) - STILL OPEN, and it is - * the half that gates a commit. The all-mode walk no longer follows it - * quietly: replacing `test` leaves `test/__fixtures__` unopenable, which - * the observation rule refuses. HOW FAR ABOVE A ROOT TO LOOK IS STILL NOT - * DECIDED, and is deliberately not decided here. + * `test/__fixtures__` and `src` in `--staged`'s scope, but not `test` and + * not `scripts`, so staging `test` as a link leaves `--staged` at exit 0 + * (measured) - STILL OPEN, and it is the half that gates a commit. The + * all-mode walk no longer has an ancestor short of the repository root at + * all, because `test` is now a root of its own rather than the directory + * above one. HOW FAR ABOVE A ROOT TO LOOK IS STILL NOT DECIDED, and is + * deliberately not decided here. * 3. PATHS MODE FOLLOWS AN EXPLICITLY NAMED LINK. `buildTargetsForPaths` uses * `statSync`, which resolves, so `pnpm phi-scan ` reads the target's * bytes. UNCHANGED AND STILL OPEN: paths mode has no corpus to reconcile @@ -190,14 +196,57 @@ * * "IN SCOPE" IS A NARROWER THING THAN THE PATH PREFIX, AND THE EXACT BOUNDARY IS * WORTH STATING RATHER THAN LEAVING TO BE INFERRED, BECAUSE THE GAP BETWEEN THE - * TWO IS WHERE THIS DEFECT LIVED. The walk covers everything under - * `test/__fixtures__/` and `src/` except a gitignored entry (the same rule that - * already excludes a gitignored fixture, so links do not get a second, stricter - * boundary of their own) and except a `.md` file. `--staged` covers + * TWO IS WHERE THIS DEFECT LIVED. The walk covers everything under `src/`, + * `test/` and `scripts/` (see `SCAN_ROOTS`) except a gitignored entry (the same + * rule that already excludes a gitignored fixture, so links do not get a second, + * stricter boundary of their own) and except a `.md` file. `--staged` covers * `test/__fixtures__` and everything under it, plus `src` and the `.ts` files * under it, restricted to the staged records git reports as ADDED, MODIFIED or * TYPECHANGED. * + * =========================================================================== + * ▶ 🛑 WHAT WIDENING THE WALK BOUGHT, AND WHAT IT DID NOT. READ BOTH HALVES. + * + * IT BOUGHT THE SSN/EMAIL FLOOR OVER 38 MORE FILES AND NOTHING ELSE. The + * structured, field-level detection this scanner needs before it can be called a + * PHI gate is still the unimplemented TODO in `scanTarget`, and opening a file + * does not implement it. Every one of the 38 was hand-read when the root moved: + * every message literal is a placeholder (`DOE^JANE`, `X^^^H^MR`, `SENDER`, + * `ZZSENTINEL*`) and the only SSN/email shapes are this scanner's own declared + * synthetic payload. So the 89 files the two routes shared no coverage of were an + * ENUMERATION gap, not a live exposure - the defect was that the gate could not + * SEE them, so nothing would have caught a real one if it appeared. + * + * THE RECOGNISER WAS NOT WIDENED, AND THAT IS A MEASUREMENT RATHER THAN AN + * OMISSION. The usual companion defect is that a recogniser assumes THE FILE IS + * THE DOCUMENT, so enumerating a `.ts` source that carries a message as an inline + * string literal buys nothing: the detector anchors on a document it cannot find. + * That failure mode needs an anchored detector to happen, AND THIS SCANNER HAS + * NONE - `scanCommonShapes` is two unanchored `matchAll` passes over the whole + * text, so its reach over a `PID|` literal embedded in TypeScript is identical to + * its reach over a standalone `.hl7` fixture. Pinned by an anchor-free probe in + * `test/scripts/phi-scan.test.ts` rather than left as a claim. + * + * THE ONE WIDENING A SIBLING SHIPPED HERE WAS MEASURED AND DECLINED: an + * ESCAPE-DECODED second view of a source literal (`\x2d`, `-` and friends, + * which hide a token from a raw text pass). Run over every file this widening + * newly opens, the decoded view finds NOTHING the raw view does not and loses + * nothing either - this repository's sources spell their messages literally and + * use `\r`/`\n`/`\t` alone. Porting it would have been a guard with no + * measurement behind it. THE MEASUREMENT IS PINNED AS A TRIPWIRE, not asserted + * once and forgotten: the suite runs both views over the whole newly-opened + * corpus and REDS if they ever disagree, which is the signal that the next worker + * should widen. A negative control proves the tripwire can see a difference. + * =========================================================================== + * + * ▶ THE TWO ROUTES NOW DISAGREE BY A LOT MORE THAN THEY USED TO, AND THAT IS A + * DELIBERATE, STATED ASYMMETRY RATHER THAN AN OVERSIGHT. Widening the walk is a + * change to what CI sweeps; widening `--staged` is a change to what a COMMIT is + * BLOCKED on, which is a hook decision and is not taken here. So `test/*.ts`, + * `test/scripts/**` and every file under `scripts/` are swept by the all-mode + * route in CI and are enumerated by NEITHER of `--staged`'s predicates. The CI + * sweep is the cover, exactly as it already was for the non-`.ts` half of `src/`. + * * Three boundary facts, each measured on this repo rather than inferred, and * each admitting MORE than before rather than less: * @@ -220,10 +269,12 @@ * ("Committing is not possible because you have unmerged files", exit 128), * so no `U` entry has ever been one `git commit` away from landing. It is * `git add` on the resolved path that stages it, and that arrives as `M`. - * - under `src/`, only `.ts` files. The all-mode walk covers every non-`.md` - * file under `src/`, so the two routes disagree there, and the all-mode - * sweep in CI is what covers the difference. Widening the staged half is a - * scope decision and is deliberately not taken here. + * - under `src/`, only `.ts` files; and, since the walk widened, nothing under + * `test/` outside `test/__fixtures__/` and nothing under `scripts/` at all. + * The all-mode sweep in CI is what covers the difference, and the disclosure + * above says exactly how large that difference now is. Widening the staged + * half is a HOOK decision (it changes what a commit is BLOCKED on) and is + * deliberately not taken here. * * A refusal names the entry's own repo-relative path and an engine-owned token * for its kind. IT NEVER REPORTS THE LINK TARGET, which is text off the working @@ -257,24 +308,172 @@ 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 (the real -// fixture dir this repo uses); 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__"); +// Roots walked in "all" mode. +// +// ▶ THE WALK USED TO ROOT AT `test/__fixtures__` AND `src` ONLY, AND THE GAP THAT +// LEFT WAS THE WHOLE OF `test/` OUTSIDE THE FIXTURE DIRECTORY. RE-DERIVED FOR +// THIS REPOSITORY RATHER THAN PORTED FROM A SIBLING, because the sibling shapes +// differ and porting one is the bug: measured on `ba059a2`, 123 tracked files, of +// which the walk opened 34 (7 fixtures + 27 `src/`), leaving 89 scanned by +// NEITHER route and SEVEN of those carrying an inline HL7 `PID|` literal. Back to +// back on that sha: a dashed SSN and an off-domain address written into +// `test/planted.test.ts`, in this repo's own inline-message shape, exited 0 +// "OK, no hits" in all mode while `phi-scan test/planted.test.ts` reported both +// at exit 1 over the same bytes. A file written to `scripts/` did the same. +// +// `scripts` IS INCLUDED, AND THAT IS THIS REPOSITORY'S ANSWER RATHER THAN A +// SIBLING'S. The recogniser's own patterns, the allow-list this scanner refuses +// to run without, and the override log it points a developer at all live under +// `scripts/`, so the one directory guaranteed to hold PHI-shaped text was the one +// nothing enumerated. All nine files there were measured against the floor before +// the root was declared: zero hits, so the widening lands green on its own bytes +// rather than on a new carve-out. KEEP IT THAT WAY - this file is now under its +// own scan, so an example SSN or a real-looking address written into a comment +// HERE reds the gate. That is the intended pressure. The SSN pattern below is +// spelled as a quantified character class and holds no digits in the matched +// arrangement, which is why it does not red on itself. +// +// ▶ THE ONE CONSEQUENCE OF THAT PRESSURE A READER WOULD NOT GUESS, AND IT IS THE +// ALLOW-LIST'S OWN BYTES. `scripts/phi-allow-list.txt` documents `ID ` as +// "synthetic id matching an SSN / MRN / member-id shape", and the dashed-SSN +// check below is UNCONDITIONAL: it consults no allow-list at all. So an `ID` +// entry written in THE DASHED NINE-DIGIT SHAPE THAT CHECK MATCHES now reds the +// gate on the allow-list itself. Measured, not reasoned: appending one exits 0 +// on `ba059a2` and exits 1 here, naming `scripts/phi-allow-list.txt`. Today's +// file declares its only id in the `MRN-` form, which the floor does not match, +// so this is LATENT rather than live - but `scanTarget`'s TODO asks a future +// worker to add the structured id detector that CONSUMES `allow.ids`, which is +// exactly when it stops being latent. The case is pinned in +// `test/scripts/phi-scan.test.ts`, which is where the literal may live: THIS +// FILE MAY NOT SPELL ONE, and a draft of this very paragraph that did was caught +// by this gate reporting a hit on itself. +// +// THE REMEDY IS TO SPELL THE SYNTHETIC ID IN A SHAPE THE FLOOR DOES NOT MATCH +// (`MRN-000123`, as this file already does), AND IT IS DELIBERATELY NEITHER OF +// THE TWO THE HIT MESSAGE PRINTS: +// - "declare it in scripts/phi-allow-list.txt" is CIRCULAR here: the +// declaration is the thing that produced the hit, because the SSN pass reads +// no allow-list. +// - "run with --allow-fixture " does not reach this sweep at all. +// `parseArgs` sends an `--allow-fixture` invocation down PATHS mode, and the +// one named target is then filtered out, so it opens ZERO files and prints +// "OK, no hits" at exit 0. That collapse is PRE-EXISTING (it behaves +// identically on `ba059a2`) and is deliberately not fixed here; it is +// recorded because widening the walk is what makes it the tempting answer. +// AND IT IS NOT `DELIBERATE_VIOLATOR_SOURCES` EITHER. Exempting the allow-list +// would leave the one file a developer is most likely to paste a real value into +// unswept, which is the opposite of what this root was added for. +// +// WHAT IS DELIBERATELY *NOT* A ROOT, each for a measured reason rather than an +// omission: +// - `vendor/`: ten `pnpm pack` tarballs. A DEFLATE stream decoded as UTF-8 is +// not text this gate can say anything true about, and these are third-party +// build artifacts rather than this repository's authored corpus. The em-dash +// gate's NUL-exclusion grounding cites them for the same reason. +// - `docs-content/`, `documentation/` and `.changeset/`: almost every tracked +// file under them is `.md`, which the walk skips by design. RE-COUNTED +// RATHER THAN ROUNDED OFF, because the shorter sentence ("every tracked file +// under them is `.md`, so declaring them would open not one new byte") was +// written here and was FALSE: `documentation/` is 6 of 6 `.md`, but +// `docs-content/` is 9 of 10 (`docs-content/sidebars.json`) and `.changeset/` +// is 3 of 4 (`.changeset/config.json`). Declaring all three would open TWO +// files, not none. They stay out because two JSON manifests are not where +// this package writes messages, not because there is nothing there. +// - `.github/`: 8 tracked files, measured clean under the floor. +// - the repository root: NOT clean, and that is the honest reason rather than +// the one first written here. `phi-scan package.json` exits 1: the `author` +// field carries the brand's own contact address, which is a real address at +// a domain the floor has no allow-list entry for. So rooting at the +// repository root would red the gate TODAY on a value that is correct, on +// top of being the shape exactly one sibling has, which is the one that got +// caught enumerating a build transient. (The address is not spelled here, +// for the reason the `scripts` note gives; the case is pinned in +// `test/scripts/phi-scan.test.ts`.) +// +// THIS LIST IS NOT A CLAIM THAT NOTHING ELSE COULD EVER CARRY PHI. It is a claim +// about where this repository writes messages, which is `src/`, `test/` and +// `scripts/`, re-derived above rather than assumed. const SRC_ROOT = join(REPO_ROOT, "src"); +const TEST_ROOT = join(REPO_ROOT, "test"); +const SCRIPTS_ROOT = join(REPO_ROOT, "scripts"); /** * The declared scan roots, each paired with the repo-relative identity used both * in a refusal and in the `git ls-files` reconciliation below. The pair is here * rather than derived with `normalizePath` so a root's reported name cannot * depend on whether the root currently resolves. + * + * THEY MUST STAY DISJOINT. `buildTargetsForAll` walks each root independently and + * concatenates the results, so a root nested inside another (`test/__fixtures__` + * kept alongside `test`) would enumerate every file beneath it twice and report + * each hit twice. That is why the fixture directory was REPLACED by `test` rather + * than joined by it. + * + * THE FIXTURE DIRECTORY DID NOT STOP BEING WATCHED BY LOSING ITS ROOT STATUS, + * BUT THE COVER IT KEEPS IS CONDITIONAL AND THE CONDITION MUST BE STATED. An + * emptied or missing `test/__fixtures__` is still refused through the OTHER + * condition of the observation rule: git tracks in-scope files under `test` that + * the walk did not open, and the refusal names each one. That holds HERE, where + * git tracks seven files under the fixture directory, and it is what four + * updated assertions in the suite pin. + * + * WHERE GIT TRACKS NOTHING UNDER IT, THE COVER IS GONE, AND THAT IS A REAL + * BEHAVIOUR CHANGE RATHER THAN A RESTATEMENT. As a declared root, an empty + * `test/__fixtures__` refused by the opened-NOTHING floor whatever git carried; + * as an ordinary directory beneath `test`, an empty one contributes no entry and + * there is no expected path for the reconciliation to miss. Measured on a + * scratch repo tracking `src/ok.ts` and `test/foo.test.ts` and nothing under the + * fixture directory: `ba059a2` exits 2 ("test/__fixtures__: opened nothing"), + * this exits 0. Arguably the correct answer, since a directory nothing is filed + * under is not evidence of a starved corpus, but it is a LOSS and is recorded as + * one rather than implied away. */ const SCAN_ROOTS: readonly { abs: string; rel: string }[] = [ - { abs: FIXTURE_ROOT, rel: "test/__fixtures__" }, { abs: SRC_ROOT, rel: "src" }, + { abs: TEST_ROOT, rel: "test" }, + { abs: SCRIPTS_ROOT, rel: "scripts" }, ]; +/** + * Sources whose bytes are a DELIBERATE VIOLATOR CORPUS: they carry PHI-shaped + * literals on purpose, because they are the positive half of this scanner's own + * tests, and sweeping them would red the gate forever. + * + * THIS IS AN EXPLICIT PATH LIST AND MUST STAY ONE. An extension rule cannot tell + * a file that carries violator literals ON PURPOSE from one that carries them BY + * ACCIDENT, and that distinction is the whole reason this gate exists, so the + * exemption is per-path and adding to it is a reviewed act, exactly like adding + * an allow-list token. A blanket `.ts` exclusion would take all of `test/` and + * all of `src/` back out of the scan and close nothing. + * + * ALLOW-LISTING THE VALUES INSTEAD WAS REFUSED, AND THE REASON IS THE EMAIL HALF. + * `EMAILDOMAIN` is GLOBAL: declaring `hospital.org` to green this one file would + * switch the email detector off for the whole corpus, and this file's own + * positive case asserts that exact address IS reported. The dashed-SSN check is + * unconditional and consults no allow-list at all, so there is no token-level + * route for that half either. + * + * THE EXEMPTION IS APPLIED AT THE SCAN, NOT AT THE ENUMERATION, and that is + * load-bearing: the file is still walked, still READ, and therefore still counts + * as observed for the per-root rule and as reconciled against `git ls-files`. + * Skipping it at enumeration would make it look like a file the walk never + * reached, which is the shape both of those rules exist to refuse. + * + * IT IS SCOPED TO THE ALL-MODE SWEEP, AND SCOPING IT IS NOT OPTIONAL. Naming the + * file in paths mode (`pnpm phi-scan test/scripts/phi-scan.test.ts`) still + * reports every hit at exit 1, because a caller naming a path is asking about + * that path. An unscoped exemption would DELETE a detection the base had, which + * is "instead of" where this work is only ever "in addition to". + * + * THE RESIDUAL, stated rather than hidden: a real SSN or email committed into + * this ONE path is not reported by the sweep. It is bounded by the list being + * explicit and one entry long, by the file being the scanner's own suite (read by + * anyone changing the scanner), by paths mode still reporting it, and by the + * value still having to survive review. Widening the list is what would make it + * unbounded. + */ +const DELIBERATE_VIOLATOR_SOURCES: ReadonlySet = new Set(["test/scripts/phi-scan.test.ts"]); + // --------------------------------------------------------------------------- // Types // --------------------------------------------------------------------------- @@ -478,6 +677,14 @@ function validateAllowFixtures(allowFixtures: string[]): void { interface Target { path: string; // forward-slash repo-relative path for reporting read: () => Buffer; + /** + * Set only by `buildTargetsForAll`, only for a member of + * `DELIBERATE_VIOLATOR_SOURCES`. The target is still READ (so an unreadable one + * still refuses) and still counts as observed; only the detectors are skipped. + * Never set by `buildTargetsForPaths`, which is what keeps naming the file + * directly a hit rather than a silent pass. + */ + sweepExempt?: boolean; } /** @@ -717,7 +924,16 @@ function buildTargetsForAll(): Target[] { return files .filter((abs) => !ignored.has(normalizePath(abs))) - .map((abs) => ({ path: normalizePath(abs), read: () => readFileSync(abs) })); + .map((abs) => { + const path = normalizePath(abs); + return { + path, + read: (): Buffer => readFileSync(abs), + // Applied HERE and not in the walk: the file stays enumerated, stays + // read, and stays part of what the observation rule reconciles. + sweepExempt: DELIBERATE_VIOLATOR_SOURCES.has(path), + }; + }); } function buildTargetsForPaths(paths: string[]): Target[] { @@ -888,6 +1104,12 @@ function scanTarget(target: Target, allow: AllowList, hits: Hit[]): void { `could not read ${target.path}: ${err instanceof Error ? err.message : String(err)}`, ); } + // The deliberate-violator exemption, applied AFTER the read so the target still + // counts as observed and an unreadable one still refuses. See + // `DELIBERATE_VIOLATOR_SOURCES` for why it is a path list, why the values + // cannot be allow-listed instead, and why it is scoped to the sweep. + if (target.sweepExempt === true) return; + const text = buf.toString("utf8"); // The format-agnostic floor: dashed SSN + non-test email. This runs on every diff --git a/test/scripts/phi-scan.test.ts b/test/scripts/phi-scan.test.ts index cf85127..7d34da9 100644 --- a/test/scripts/phi-scan.test.ts +++ b/test/scripts/phi-scan.test.ts @@ -204,10 +204,12 @@ describe("phi-scan: the scanner under test is THIS package's", () => { expect(name).not.toBe("@cosyte/dicom"); expect(existsSync(SCANNER_PATH)).toBe(true); // This repo's own scan roots. A sibling's scanner walks `test/fixtures`, not - // `test/__fixtures__`, and none of them walks `src/` as a second root. + // `test/__fixtures__`, and none of them exempts THIS path. const source = readFileSync(SCANNER_PATH, "utf8"); expect(source).toContain("test/__fixtures__"); expect(source).toContain("SRC_ROOT"); + expect(source).toContain("SCRIPTS_ROOT"); + expect(source).toContain("test/scripts/phi-scan.test.ts"); expect(source).not.toContain("PN_TAGS"); }); }); @@ -608,7 +610,7 @@ describe( // can print `OK, no hits` and exit 0 over a corpus nobody opened. Each case // here exited 0 with that message before the observation rule, measured. - it("stays green when both roots are healthy and fully observed", () => { + it("stays green when every root is healthy and fully observed", () => { // The premise, first: a refusal rule that reds the ordinary case teaches // people to disable it, and every case below would pass vacuously against // a scanner that had simply started refusing everything. @@ -624,7 +626,10 @@ describe( const r = runScanner([], root); expect(r.code, `stderr: ${r.stderr}`).toBe(2); - expect(r.stderr).toContain("test/__fixtures__: opened 0 file(s)"); + // Filed under `test`, which is the ROOT, since the widening replaced + // `test/__fixtures__` with its parent. The actionable half is unchanged: + // the refusal still names the tracked file that went unopened. + expect(r.stderr).toContain("test: opened 0 file(s)"); expect(r.stderr).toContain("test/__fixtures__/ordinary.txt"); expect(r.stdout).not.toMatch(/OK/); }); @@ -647,14 +652,40 @@ describe( // stand in for this rule: `existsSync` resolves the link, answers false, // and `walk()` returns before `readdirSync`. Nothing about the entry is // ever inspected, so no kind check can fire on it. + // + // AIMED AT `test` RATHER THAN `test/__fixtures__` SINCE THE WIDENING, and + // the reason is the whole point of the case: the fixture directory is no + // longer a ROOT, so a link there is an ENUMERATED entry and the kind check + // DOES fire on it (pinned separately below). Only a declared root can + // still reach `walk()`'s first line, so only a declared root exercises + // this rule. + const root = makeTrackedRepo(); + rmSync(join(root, "test"), { recursive: true }); + symlinkSync("nowhere-at-all", join(root, "test")); + expect(existsSync(join(root, "test"))).toBe(false); + + const r = runScanner([], root); + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("test: opened 0 file(s)"); + expect(r.stderr).toContain("test/__fixtures__/ordinary.txt"); + expect(r.stdout).not.toMatch(/OK/); + }); + + it("refuses a dangling link AT the old fixture root, by the enumerated-entry rule", () => { + // The narrowing the widening bought, asserted rather than claimed. + // `test/__fixtures__` used to be a declared root, where a dangling link + // reached `walk()`'s `existsSync` and nothing about the entry was ever + // inspected. It is now an ordinary entry BENEATH `test`, so `Dirent`'s + // lstat answer sees it and the not-a-regular-file refusal fires on it + // directly, naming the entry and its kind. const root = makeTrackedRepo(); rmSync(join(root, "test", "__fixtures__"), { recursive: true }); symlinkSync(join("..", "nowhere-at-all"), join(root, "test", "__fixtures__")); - expect(existsSync(join(root, "test", "__fixtures__"))).toBe(false); const r = runScanner([], root); expect(r.code, `stderr: ${r.stderr}`).toBe(2); - expect(r.stderr).toContain("test/__fixtures__/ordinary.txt"); + expect(r.stderr).toContain("test/__fixtures__"); + expect(r.stderr).toContain("a symbolic link"); expect(r.stdout).not.toMatch(/OK/); }); @@ -667,12 +698,12 @@ describe( const outside = realpathSync(mkdtempSync(join(tmpdir(), "cli-phi-scan-outside-"))); repos.push(outside); writeFileSync(join(outside, "unrelated.txt"), "synthetic placeholder\n"); - rmSync(join(root, "test", "__fixtures__"), { recursive: true }); - symlinkSync(outside, join(root, "test", "__fixtures__")); + rmSync(join(root, "test"), { recursive: true }); + symlinkSync(outside, join(root, "test")); const r = runScanner([], root); expect(r.code, `stderr: ${r.stderr}`).toBe(2); - expect(r.stderr).toContain("test/__fixtures__: opened 1 file(s)"); + expect(r.stderr).toContain("test: opened 1 file(s)"); expect(r.stderr).toContain("test/__fixtures__/ordinary.txt"); }); @@ -686,7 +717,7 @@ describe( const r = runScanner([], root); expect(r.code, `stderr: ${r.stderr}`).toBe(2); - expect(r.stderr).toContain("test/__fixtures__: opened 1 file(s)"); + expect(r.stderr).toContain("test: opened 1 file(s)"); expect(r.stderr).toContain("test/__fixtures__/ordinary.txt"); expect(r.stderr).not.toContain("test/__fixtures__/second.txt"); }); @@ -971,12 +1002,18 @@ describe( // exactly this - a root git carries NOTHING under, so the reconciliation // has no expected path to miss and the walk's one hit satisfies the // opened-nothing floor. + // + // THE LINK IS AT `test` RATHER THAN `test/__fixtures__` SINCE THE + // WIDENING. That is not a cosmetic retarget: the fixture directory is no + // longer a declared root, so a link there is refused outright and this + // shape is now reachable ONLY at one of the three top-level roots. The + // escape is one level narrower than it was, and it is still open there. const root = makeRepo(); const outside = realpathSync(mkdtempSync(join(tmpdir(), "cli-phi-scan-outside-"))); repos.push(outside); writeFileSync(join(outside, "real-notes.txt"), SYNTHETIC_PHI); - rmSync(join(root, "test", "__fixtures__"), { recursive: true }); - symlinkSync(outside, join(root, "test", "__fixtures__")); + rmSync(join(root, "test"), { recursive: true }); + symlinkSync(outside, join(root, "test")); const r = runScanner([], root); expect(r.code, `stderr: ${r.stderr}`).toBe(1); @@ -984,9 +1021,9 @@ describe( // RESOLVE, through the link, which is why `existsSync` is true one line // down; what makes it a fabrication is that git tracks no such file and no // commit contains it. The `ls-files` assertion is doing the real work. - expect(r.stderr).toContain("test/__fixtures__/real-notes.txt"); - expect(existsSync(join(root, "test", "__fixtures__", "real-notes.txt"))).toBe(true); - expect(gitOut(root, ["ls-files", "test/__fixtures__/real-notes.txt"]).trim()).toBe(""); + expect(r.stderr).toContain("test/real-notes.txt"); + expect(existsSync(join(root, "test", "real-notes.txt"))).toBe(true); + expect(gitOut(root, ["ls-files", "test/real-notes.txt"]).trim()).toBe(""); }); it("FOLLOWS a root link whose target MIRRORS the tracked names, corpus fully tracked", () => { @@ -999,9 +1036,10 @@ describe( const root = makeTrackedRepo(); const decoy = realpathSync(mkdtempSync(join(tmpdir(), "cli-phi-scan-decoy-"))); repos.push(decoy); - writeFileSync(join(decoy, "ordinary.txt"), "decoy, not the tracked bytes\n"); - rmSync(join(root, "test", "__fixtures__"), { recursive: true }); - symlinkSync(decoy, join(root, "test", "__fixtures__")); + mkdirSync(join(decoy, "__fixtures__")); + writeFileSync(join(decoy, "__fixtures__", "ordinary.txt"), "decoy, not the tracked bytes\n"); + rmSync(join(root, "test"), { recursive: true }); + symlinkSync(decoy, join(root, "test")); // The premise: git really does carry a DIFFERENT blob at that path, so a // pass here is a pass over a corpus that was never opened. expect(gitOut(root, ["show", ":test/__fixtures__/ordinary.txt"])).toContain( @@ -1040,3 +1078,447 @@ describe( }); }, ); + +// --------------------------------------------------------------------------- +// The walk's ROOTS: `src`, `test` and `scripts`, widened from `test/__fixtures__` +// and `src`. +// +// MEASURED BACK TO BACK ON `ba059a2`, THE COMMIT THIS WIDENING LANDED ON, AND +// RE-DERIVED FOR THIS REPOSITORY RATHER THAN PORTED: 123 tracked files, 34 opened +// by the walk, 89 scanned by NEITHER route. A dashed SSN and an off-domain +// address written into `test/planted.test.ts` (in this repo's own inline-message +// shape) and into `scripts/planted.txt` each exited 0 "OK, no hits" in all mode, +// while naming the same file in PATHS mode reported both at exit 1 over the same +// bytes. That gap was ENUMERATION, never detection, which is why the cases below +// assert the two routes AGREE: a narrowing of the roots reds here. +// --------------------------------------------------------------------------- + +describe( + "phi-scan: the walk reaches this repo's whole authored corpus", + { timeout: SLOW_MS }, + () => { + for (const rel of [ + ["test", "planted.test.ts"], // `test/` outside the fixture dir: the item's headline + ["test", "scripts", "planted.test.ts"], // and nested under it + ["scripts", "planted.txt"], // the directory the scanner itself lives in + ["src", "planted.ts"], // the root that was already covered: a control + ["test", "__fixtures__", "planted.txt"], // the old root, still covered after the swap + ]) { + const path = rel.join("/"); + it(`sweeps a violator at ${path}, and paths mode agrees`, () => { + const root = makeRepo(); + mkdirSync(join(root, ...rel.slice(0, -1)), { recursive: true }); + writeFileSync(join(root, ...rel), SYNTHETIC_PHI); + + const sweep = runScanner([], root); + expect(sweep.code, `stderr: ${sweep.stderr}`).toBe(1); + expect(sweep.stderr).toContain(path); + expect(sweep.stderr).toContain(SYNTHETIC_SSN); + + // The two routes must say the same thing about the same bytes. Before the + // widening the first three of these exited 0 here and 1 below. + const named = runScanner([path], root); + expect(named.code, `stderr: ${named.stderr}`).toBe(1); + expect(named.stderr).toContain(SYNTHETIC_SSN); + }); + } + + it("still reports a clean tree as clean over all three roots (exit 0)", () => { + // The premise. A widening that reds the ordinary case is a widening someone + // reverts, and every assertion above would pass against a scanner that had + // simply started refusing everything. + const root = makeRepo(); + const r = runScanner([], root); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + expect(r.stdout).toMatch(/OK, no hits/); + }); + + it("refuses when `scripts` is the starved root, so the new root is reconciled too", () => { + // A root that is declared but not reconciled is the defect this repo closed + // one change ago. Adding a root without extending that rule to it would + // reopen it for the new root alone. + const root = makeRepo(); + git(root, ["add", "scripts/phi-allow-list.txt", "src/ok.ts"]); + rmSync(join(root, "scripts", "phi-allow-list.txt")); + + const r = runScanner([], root); + // The allow-list is gone, so the earlier invocation step refuses first. That + // is the correct order and is asserted rather than worked around: a + // `scripts/` empty enough to starve the observation rule cannot be reached + // without first removing the file this scanner refuses to run without. + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("allow-list not found"); + }); + + it("reconciles `scripts` against git when the allow-list is present but the corpus is not", () => { + const root = makeRepo(); + writeFileSync(join(root, "scripts", "helper.mjs"), "export const h = 1;\n"); + git(root, ["add", "scripts/helper.mjs"]); + rmSync(join(root, "scripts", "helper.mjs")); + + const r = runScanner([], root); + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("scripts/helper.mjs"); + }); + }, +); + +describe( + "phi-scan: what the `scripts` root COSTS, pinned so the disclosure cannot drift", + { timeout: SLOW_MS }, + () => { + it("an `ID` entry spelled as a dashed SSN reds the gate on the allow-list itself", () => { + // The one consequence of rooting at `scripts` that a reader would not + // guess. `phi-allow-list.txt` documents `ID ` as "synthetic id + // matching an SSN / MRN / member-id shape", and the dashed-SSN pass + // consults NO allow-list, so writing that declaration in that shape + // creates the hit. Latent today (the real file carries `MRN-000123`, which + // the floor does not match) and it exits 0 on the base scanner. + // + // THE FIX IF THIS EVER FIRES IS TO SPELL THE ID IN A SHAPE THE FLOOR DOES + // NOT MATCH. It is NOT to make the SSN pass consult `allow.ids`, which + // would DELETE a detection the base had, and it is NOT to add the + // allow-list to the exemption, which would leave the one file a developer + // is most likely to paste a real value into unswept. + const root = makeRepo(); + const allow = join(root, "scripts", "phi-allow-list.txt"); + writeFileSync(allow, `${readFileSync(allow, "utf8")}ID ${SYNTHETIC_SSN}\n`); + + const r = runScanner([], root); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("scripts/phi-allow-list.txt"); + expect(r.stderr).toContain(SYNTHETIC_SSN); + }); + + it("the shipped allow-list carries no id the floor matches, so the trap stays latent", () => { + // The premise of the case above being latent rather than live. If a future + // `ID` entry adopts the dashed shape, this reds first and points at the + // disclosure instead of at a mystery red on the CI sweep. + const declared = readFileSync(join(REPO_ROOT, "scripts", "phi-allow-list.txt"), "utf8") + .split(/\r?\n/) + .filter((l) => l.startsWith("ID ")) + .map((l) => l.slice(3).trim()); + expect(declared.length).toBeGreaterThan(0); + for (const id of declared) expect(id).not.toMatch(/\b\d{3}-\d{2}-\d{4}\b/); + }); + + it("--allow-fixture cannot reach the all-mode sweep, so it is not the remedy either", () => { + // PRE-EXISTING and deliberately not fixed here: an `--allow-fixture` + // invocation is routed down PATHS mode, and the one named target is then + // filtered out, so it opens ZERO files and reports clean. Pinned because + // widening the walk is what makes it the tempting answer to the case + // above, and because a reader would otherwise take the hit message's own + // advice. + const root = makeRepo(); + writeFileSync(join(root, "src", "violator.ts"), SYNTHETIC_PHI); + writeFileSync( + join(root, "phi-scan-overrides.md"), + "# overrides\n\n### src/violator.ts\n\nsynthetic\n", + ); + // The premise: without the flag this really is a hit, so a green below is + // the collapse and not an empty corpus. + expect(runScanner([], root).code).toBe(1); + + const r = runScanner(["--allow-fixture", "src/violator.ts"], root); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + expect(r.stdout).toMatch(/OK, no hits/); + }); + + it("an empty `test/__fixtures__` git tracks nothing under no longer refuses", () => { + // The cover the fixture directory LOST by ceasing to be a declared root, + // asserted rather than implied away. As a root it refused by the + // opened-NOTHING floor whatever git carried; as an ordinary directory it + // contributes no entry and the reconciliation has no expected path to + // miss. This exits 2 on `ba059a2` and 0 here. + // + // Where git DOES track files under it (this repository: seven) the second + // condition still fires, which the observation suite above pins. + const root = makeRepo(); + git(root, ["add", "src/ok.ts", "scripts/phi-allow-list.txt"]); + rmSync(join(root, "test", "__fixtures__", "ordinary.txt")); + writeFileSync(join(root, "test", "foo.test.ts"), "export const t = 1;\n"); + git(root, ["add", "test/foo.test.ts"]); + expect(gitOut(root, ["ls-files", "test/__fixtures__"]).trim()).toBe(""); + + const r = runScanner([], root); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + }); + }, +); + +describe("phi-scan: the NOT-a-root measurements, re-derived rather than quoted", () => { + // The class this repository has now been caught on three times: a measurement + // written into a banner and not taken. Two of the three "deliberately not a + // root" bullets were FALSE as first written, so each is a check here instead + // of a sentence a reader has to trust. A failure means the tree moved and the + // banner needs re-wording, never that the banner should be deleted. + + function tracked(...paths: string[]): string[] { + return gitOut(REPO_ROOT, ["ls-files", "-z", "--", ...paths]) + .split("\0") + .filter((p) => p.length > 0); + } + + it("`documentation/` is all markdown, and `docs-content/` and `.changeset/` are NOT", () => { + const nonMd = (paths: string[]): string[] => + paths.filter((p) => !p.toLowerCase().endsWith(".md")); + expect(nonMd(tracked("documentation"))).toStrictEqual([]); + expect(nonMd(tracked("docs-content"))).toStrictEqual(["docs-content/sidebars.json"]); + expect(nonMd(tracked(".changeset"))).toStrictEqual([".changeset/config.json"]); + }); + + it("`.github/` is clean under the floor, and the repository root is NOT", () => { + // The corrected half. `package.json` carries an off-domain `author` address, + // so "the repository root is measured clean" was false and rooting there + // would red today on a correct value. + const github = runScanner(tracked(".github")); + expect(github.code, `stderr: ${github.stderr}`).toBe(0); + + const manifest = runScanner(["package.json"]); + expect(manifest.code, `stderr: ${manifest.stderr}`).toBe(1); + expect(manifest.stderr).toContain("(email)"); + }); +}); + +describe("phi-scan: the deliberate-violator exemption", { timeout: SLOW_MS }, () => { + // This file carries the payload on purpose, and `test/` is now a scan root, so + // without the exemption the sweep would red forever on its own test suite. + + it("the sweep of THIS repository is green, over this file's real payload", () => { + // Not a fixture: the actual `pnpm phi-scan` CI invocation, in this tree. + const r = runScanner([]); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + expect(r.stdout).toMatch(/OK, no hits/); + }); + + it("names the file in the scanner's own source, so the exemption is reviewable", () => { + const source = readFileSync(SCANNER_PATH, "utf8"); + expect(source).toContain("DELIBERATE_VIOLATOR_SOURCES"); + expect(source).toContain("test/scripts/phi-scan.test.ts"); + }); + + it("is SCOPED TO THE SWEEP: naming this file in paths mode still reports every hit", () => { + // The half that keeps this a widening rather than a trade. An unscoped + // exemption would DELETE a detection the base had, and a sibling shipped + // exactly that mistake before catching it. + const r = runScanner(["test/scripts/phi-scan.test.ts"]); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain(SYNTHETIC_SSN); + expect(r.stderr).toContain("jane.doe@hospital.org"); + }); + + it("is PER PATH, not a pattern: the same payload in a sibling test file still reds", () => { + // An extension or directory rule could not tell a file that carries violator + // literals on purpose from one that carries them by accident, which is the + // whole distinction this gate exists to draw. + const root = makeRepo(); + mkdirSync(join(root, "test", "scripts"), { recursive: true }); + writeFileSync(join(root, "test", "scripts", "other.test.ts"), SYNTHETIC_PHI); + + const r = runScanner([], root); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain("test/scripts/other.test.ts"); + }); + + it("an exempt file is still READ and still counts as observed", () => { + // The exemption is applied after the read, so it cannot be mistaken for a + // file the walk never reached: an unreadable one still refuses (exit 2) + // rather than passing as exempt. + // + // EACH BRANCH ASSERTS ITS OWN PREMISE, and `hasAssertions` alone does NOT + // close the hole its first draft claimed it did. That draft skipped on + // `expect(typeof readable).toBe("boolean")`, which is TRUE OF EVERY VALUE: + // under a uid that ignores mode bits (root in a container) the case went + // green having asserted nothing at all about the exemption, while + // `hasAssertions` was satisfied by the tautology. So the skip branch now + // asserts the environment really is the one it is skipping for, and the real + // branch asserts the file really did become unreadable. Neither can pass + // vacuously. (The same tautology sits in two PRE-EXISTING cases above, + // unchanged here: they are not this change's to rewrite.) + expect.hasAssertions(); + const root = makeRepo(); + mkdirSync(join(root, "test", "scripts"), { recursive: true }); + const exempt = join(root, "test", "scripts", "phi-scan.test.ts"); + writeFileSync(exempt, SYNTHETIC_PHI); + let r: RunResult; + try { + spawnSync("chmod", ["000", exempt], { encoding: "utf8", shell: false }); + let readable = true; + try { + readFileSync(exempt, "utf8"); + } catch { + readable = false; + } + if (readable) { + // The premise of the SKIP: mode bits really are being ignored here, so + // the unreadable branch is unreachable rather than untested by choice. + expect(readable).toBe(true); + return; + } + expect(readable).toBe(false); + r = runScanner([], root); + } finally { + spawnSync("chmod", ["644", exempt], { encoding: "utf8", shell: false }); + } + expect(r.code, `stderr: ${r.stderr}`).toBe(2); + expect(r.stderr).toContain("could not read test/scripts/phi-scan.test.ts"); + }); +}); + +describe("phi-scan: the floor is ANCHOR-FREE, which is what the widening rests on", () => { + // ENUMERATING A `.ts` SOURCE BUYS NOTHING IF THE DETECTOR ASSUMES THE FILE *IS* + // THE DOCUMENT. That is the companion defect this class carries, and it is a + // property of an ANCHORED recogniser. This scanner has none: `scanCommonShapes` + // is two unanchored passes over the whole text. These cases assert that rather + // than leaving it as a claim in a banner, by putting one token in three + // placements and requiring all three to red. + + const PLACEMENTS: [string, string, string][] = [ + ["a standalone document", "doc.txt", `patient ssn ${SYNTHETIC_SSN} on file\n`], + [ + "an inline HL7 literal inside TypeScript", + "inline.ts", + `const M =\n "MSH|^~\\\\&|A|B|C|D|20240101||ADT^A01|1|P|2.5\\rPID|1||X^^^H^MR||DOE^JANE||19800101|F|||||||||${SYNTHETIC_SSN}\\r";\n`, + ], + [ + "a multi-line template literal", + "template.ts", + "const M = `\n line one\n ssn " + SYNTHETIC_SSN + "\n`;\n", + ], + ]; + + for (const [label, name, body] of PLACEMENTS) { + it(`catches the same token in ${label}`, () => { + const r = scan(name, body); + expect(r.code, `stderr: ${r.stderr}`).toBe(1); + expect(r.stderr).toContain(SYNTHETIC_SSN); + }); + } + + it("KNOWN LIMIT, pinned: the floor is SSN + email and the widening did not change that", () => { + // The disclosure "enumerating the files buys the SSN/email floor and NOTHING + // else" is asserted here rather than only written down. An undashed id, a + // name, a date of birth and an address in the very same PID segment go + // unreported, because the structured detector is still the unimplemented TODO + // in `scanTarget`. If one of these ever starts failing, a real detector + // landed: delete the case and the disclosure together. + const r = scan( + "unreported.ts", + 'const M =\n "PID|1||MRN00042^^^H^MR||DOE^JANE||19800101|F|||42 SYNTHETIC ST^^METROPOLIS^NY^10001||555-0100|||||123456789";\n', + ); + expect(r.code, `stderr: ${r.stderr}`).toBe(0); + expect(r.stdout).toMatch(/OK, no hits/); + }); +}); + +describe("phi-scan: the escape-decoded view was MEASURED and DECLINED, and stays measured", () => { + // A sibling widened its recogniser to a second, ESCAPE-DECODED view of a source + // literal, because a `.ts` file can spell a token through `\x2d` and hide it + // from a raw text pass. Measured over every file THIS widening newly opens, the + // decoded view finds nothing the raw view does not, so porting it here would + // have been a guard with no measurement behind it. + // + // THE MEASUREMENT IS PINNED RATHER THAN ASSERTED ONCE. If a source ever lands + // that does hide a token behind an escape, this reds and tells the next worker + // to widen. The regexes are deliberately a SECOND COPY of the scanner's floor: + // a tripwire that imported them would go quiet in exactly the case where the + // floor itself was narrowed. + + const SSN_RE = /\b\d{3}-\d{2}-\d{4}\b/g; + const EMAIL_RE = /\b[A-Za-z0-9._%+-]+@([A-Za-z0-9.-]+\.[A-Za-z]{2,})\b/g; + const ALLOWED_DOMAINS = new Set( + readFileSync(join(REPO_ROOT, "scripts", "phi-allow-list.txt"), "utf8") + .split(/\r?\n/) + .filter((l) => l.startsWith("EMAILDOMAIN ")) + .map((l) => l.slice("EMAILDOMAIN ".length).trim().toLowerCase()), + ); + + const SIMPLE: Record = { + n: "\n", + r: "\r", + t: "\t", + v: "\v", + f: "\f", + "0": "\0", + "'": "'", + '"': '"', + "\\": "\\", + "`": "`", + }; + + /** The escape-decoded view of a source literal: `\x2d`, `\u002d`, `\r` and friends. */ + function decode(text: string): string { + return text.replace( + /\\(u\{[0-9a-fA-F]{1,6}\}|u[0-9a-fA-F]{4}|x[0-9a-fA-F]{2}|[nrtvf0'"\\`])/g, + (whole, esc: string) => { + if (esc.startsWith("u") || esc.startsWith("x")) { + const hex = esc.startsWith("u{") ? esc.slice(2, -1) : esc.slice(1); + const cp = Number.parseInt(hex, 16); + return Number.isNaN(cp) ? whole : String.fromCodePoint(cp); + } + return SIMPLE[esc] ?? whole; + }, + ); + } + + function floorTokens(text: string): Set { + const out = new Set(); + for (const m of text.matchAll(SSN_RE)) out.add(`ssn:${m[0]}`); + for (const m of text.matchAll(EMAIL_RE)) { + if (!ALLOWED_DOMAINS.has((m[1] ?? "").toLowerCase())) out.add(`email:${m[0]}`); + } + return out; + } + + /** + * The TRACKED, non-`.md` files under the three roots. On a clean tree that is + * the same set the walk opens, and it is asserted to be non-empty below, but + * the two are NOT identical by construction and the shorter comment that said + * "exactly what the widened walk opens" was wrong: the walk enumerates the + * WORKING TREE, so it also opens an untracked, non-gitignored file under a + * root, which `git ls-files` never reports. The difference is one-directional + * (the walk is the superset), so this sweep can under-cover but never + * over-claim, and a stray untracked file cannot make it red. + * + * `-z` because `git ls-files` C-quotes a path holding a space, a quote or a + * non-ASCII byte, and a quoted name is a path no file has. + */ + function walkedCorpus(): string[] { + return gitOut(REPO_ROOT, ["ls-files", "-z", "--", "src", "test", "scripts"]) + .split("\0") + .filter((p) => p.length > 0 && !p.toLowerCase().endsWith(".md")); + } + + it("the tripwire can see a difference (negative control on the comparison itself)", () => { + // Without this, a decode() that silently stopped decoding would make the + // sweep below pass for the wrong reason, which is the vacuity class this + // suite has already sprung twice. + const hidden = 'const s = "123\\x2d45\\u002d6789";'; + expect([...floorTokens(hidden)]).toStrictEqual([]); + expect([...floorTokens(decode(hidden))]).toStrictEqual([`ssn:${SYNTHETIC_SSN}`]); + }); + + it("the corpus it sweeps is the one the walk opens, and it is not empty", () => { + const corpus = walkedCorpus(); + expect(corpus.length).toBeGreaterThan(50); + expect(corpus).toContain("test/scripts/phi-scan.test.ts"); + expect(corpus).toContain("scripts/phi-scan.ts"); + expect(corpus).toContain("src/index.ts"); + }); + + it("finds no token the raw view misses, across every file the walk opens", () => { + const divergent: string[] = []; + for (const rel of walkedCorpus()) { + const text = readFileSync(join(REPO_ROOT, rel), "utf8"); + const raw = floorTokens(text); + const decoded = floorTokens(decode(text)); + for (const t of decoded) if (!raw.has(t)) divergent.push(`${rel}: only decoded: ${t}`); + for (const t of raw) if (!decoded.has(t)) divergent.push(`${rel}: only raw: ${t}`); + } + // A failure here is NOT a defect in this change. It means a source landed + // that spells a PHI-shaped token through an escape, so the escape-decoded + // view now earns its place: widen `scanCommonShapes` (in ADDITION to the raw + // pass, never instead of it) and re-derive this measurement. + expect(divergent).toStrictEqual([]); + }); +});