From c8eb2538d0ca416de41a4406d1f3da1750019b01 Mon Sep 17 00:00:00 2001 From: Noah Schatz Date: Fri, 7 Aug 2026 19:22:42 +0000 Subject: [PATCH 1/2] fix(phi-scan): widen the walk to src, test and scripts The all-mode walk was rooted at `test/__fixtures__` and `src` only, so 89 of this repository's 123 tracked files were scanned by NEITHER of the scanner's two routes: PHI-SCAN-WALK-ROOT-SCOPE, the other half of the item `#38`/`#39` closed. It now roots at `src`, `test` and `scripts`, opening 72 tracked files instead of 34. RE-DERIVED FOR THIS REPOSITORY, NEVER PORTED. Measured on `ba059a2`: 123 tracked, 34 opened (7 fixtures + 27 `src/`), 89 in neither route, 7 of those carrying an inline HL7 `PID|` literal. RED before, GREEN after, back to back on that sha: a dashed SSN and an off-domain address written into `test/planted.test.ts` in this repo's own inline-message shape (a whole HL7 message as one `.ts` string literal with `\r` escapes between segments) exited 0 `OK, no hits` in all mode, while `phi-scan test/planted.test.ts` reported both at exit 1 over the same bytes. A file under `scripts/` behaved the same. 14 of the suite's 68 cases red against `ba059a2`'s scanner. ALL 38 NEWLY OPENED FILES WERE HAND-READ. Every message literal is a placeholder (`DOE^JANE`, `X^^^H^MR`, `SENDER`, `ZZSENTINEL*`) and the only SSN/email shapes anywhere are the scanner's own declared synthetic payload. So the 89 were an ENUMERATION gap, not a live PHI exposure: the defect is that the gate could not see those files, so nothing would have caught a real one. `test` REPLACES `test/__fixtures__` rather than joining it. Roots must stay disjoint: each is walked independently and the results concatenated, so a nested root would enumerate every file beneath it twice and report each hit twice. The fixture dir is still watched, through the observation rule's other condition; only the root a refusal is filed under moved, which is what the four updated assertions are. `scripts` is in because the allow-list, the override log and this scanner live there, so the one directory guaranteed to hold PHI-shaped text was the one nothing enumerated. All nine files were measured against the floor before the root was declared: zero hits. Not roots, each measured: `vendor/` (DEFLATE streams), `docs-content`/`documentation`/`.changeset` (all `.md`, which the walk skips), `.github` and the repo root. A NARROWING CAME WITH IT: `test/__fixtures__` is no longer a root, so a live or dangling link there is now an enumerated entry and is refused outright, whatever it points at. Only the three top-level roots are still followable. The two residual pins were retargeted one level up and a new case pins the narrowing. `test/scripts/phi-scan.test.ts` is the ONE exempt path. Applied at the SCAN, never the enumeration (still read, still observed, still reconciled; unreadable still exits 2); SCOPED to the all-mode sweep (naming it still reports every hit, because an unscoped exemption DELETES a detection the base had); and PER PATH, never a pattern. Each property has a control that reds when it is removed. Allow-listing the values was refused: `EMAILDOMAIN` is global. WHAT THIS BOUGHT AND WHAT IT DID NOT. The SSN/email floor over 38 more files and nothing else; structured field-level detection is still the unimplemented TODO, and a test pins what is still missed. The recogniser was NOT widened, on measurement: this floor is anchor-free, so it never had the "the file IS the document" defect, and an escape-decoded second view finds nothing the raw view does not over the whole newly-opened corpus. That measurement ships as a TRIPWIRE with a negative control, not a one-time claim. `--staged` is unchanged, deliberately: widening it is a hook decision about what a commit is BLOCKED on. The path-SET escape is unchanged and still disclosed. Both named PRE-EXISTING minors were re-measured NOT open here. verify.sh cli: 12 steps ran, 10 green. `pnpm audit --prod --audit-level high` and the licenses gate fail, both reproduced byte-identically on a base tree restored by file copy; `package.json` and `pnpm-lock.yaml` are untouched here and no CI job runs either command. 424 tests, 97.7% statements. --- .changeset/phi-scan-widen-the-walk-roots.md | 32 ++ CHANGELOG.md | 35 ++ CLAUDE.md | 85 ++--- documentation/agent-notes.md | 125 +++++++ scripts/phi-scan.ts | 251 ++++++++++--- test/scripts/phi-scan.test.ts | 372 +++++++++++++++++++- 6 files changed, 800 insertions(+), 100 deletions(-) create mode 100644 .changeset/phi-scan-widen-the-walk-roots.md 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..2b14879 --- /dev/null +++ b/.changeset/phi-scan-widen-the-walk-roots.md @@ -0,0 +1,32 @@ +--- +"@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. `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. + +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..e5ec061 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,41 @@ 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; only the root a refusal is filed under changed. + - **`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. + - **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. + - **`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..e6413ad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,10 +1,10 @@ # @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**; that file's own header records when and why, and +> 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 +33,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 +88,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 +111,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 +176,18 @@ 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. +- **🛑 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 +204,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 +284,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,8 +342,7 @@ 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 @@ -386,10 +390,9 @@ 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: diff --git a/documentation/agent-notes.md b/documentation/agent-notes.md index 68e67cd..ff4a8ce 100644 --- a/documentation/agent-notes.md +++ b/documentation/agent-notes.md @@ -216,6 +216,131 @@ 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. +**Fourteen of the suite's 68 cases red against `ba059a2`'s scanner.** + +**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. + +**`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. + +**What is deliberately NOT a root, each measured rather than omitted.** `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/` (every +tracked file under them is `.md`, which the walk skips by design, so declaring them would add +reconciliation surface and open not one byte); `.github/` and the repository root (measured clean, and +neither is where this package writes messages - and rooting at the repository root is the one sibling +shape that got caught enumerating a build transient). + +**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..a870ffa 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,117 @@ 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. +// +// 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/`: every tracked file +// under them is `.md`, which the walk skips by design, so declaring them +// would add reconciliation surface and open not one new byte. +// - `.github/` and the repository root: measured clean under the floor, and +// neither is where this package's PHI-shaped literals live. Rooting at the +// repository root is a shape exactly one sibling has, and it is the one that +// got caught enumerating a build transient. +// +// 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, and + * that is worth knowing rather than rediscovering. 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. What changes is only which root the + * message is filed under. */ 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 +622,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 +869,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 +1049,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..f55286e 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,303 @@ 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: 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. `hasAssertions` because a uid that ignores + // mode bits would otherwise make the early return a silent pass. + 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; + } + expect(typeof readable).toBe("boolean"); + if (readable) return; + 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; + } + + /** Exactly what the widened walk opens: tracked, non-`.md`, under the three roots. */ + function walkedCorpus(): string[] { + return gitOut(REPO_ROOT, ["ls-files", "--", "src", "test", "scripts"]) + .split("\n") + .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([]); + }); +}); From 5160e18698b9e196d22d62f4cd3f40bc1ad7220d Mon Sep 17 00:00:00 2001 From: Noah Schatz Date: Fri, 7 Aug 2026 22:16:37 +0000 Subject: [PATCH 2/2] fix(phi-scan): correct two false not-a-root measurements, disclose what the scripts root costs Gate pass 1 (conformance-refuter) returned REFUTED with two majors and three minors. No blocker: no detection the base had is deleted, the detector is byte-identical, and no PHI escapes. Every finding was reproduced by hand before being acted on. MAJOR 1, the scripts root costs the allow-list its own bytes. phi-allow-list.txt documents `ID ` as a synthetic id "matching an SSN / MRN / member-id shape", and the dashed-SSN check consults no allow-list, so an `ID` entry in that dashed shape now reds the gate on the allow-list itself. Measured: exit 0 on ba059a2, exit 1 here. Latent today (the shipped file declares its only id in the `MRN-` form) and it stops being latent the moment someone acts on scanTarget's TODO to add the structured id detector that consumes allow.ids. Disclosed, not "fixed": making the SSN pass read allow.ids would DELETE a detection the base had, and exempting the allow-list would leave the likeliest place for a real value unswept. Neither remedy the hit message prints works either, which is now written down: "declare it in the allow-list" is circular, and --allow-fixture routes the invocation down paths mode where the one named target is filtered out, so it opens zero files and prints OK (PRE-EXISTING, identical on ba059a2). MAJOR 2, two of the three "deliberately NOT a root" measurements were false. "every tracked file under docs-content/, documentation/ and .changeset/ is .md, so declaring them would open not one new byte" is wrong: documentation/ is 6 of 6, but docs-content/ is 9 of 10 (sidebars.json) and .changeset/ is 3 of 4 (config.json). ".github/ and the repository root: measured clean" is half wrong: .github/ is clean over 8 files, and the repository root is NOT, because package.json's author field carries a real off-domain contact address. All four are now checks in the suite rather than sentences a reader has to trust. This is the third time this repository has written down a measurement it did not take. MINOR 3, the vacuity guard on the exemption's most safety-relevant case did not guard: the skip branch asserted `typeof readable === "boolean"`, true of every value, so under a uid that ignores mode bits the case went green having asserted nothing while hasAssertions was satisfied by the tautology. Each branch now asserts its own premise. The two PRE-EXISTING cases carrying the same shape are left alone. MINOR 4, the fixture directory's remaining cover is CONDITIONAL and was stated flat. It holds where git tracks files under test/__fixtures__ (here, seven). Where git tracks nothing under it, an empty one no longer refuses: ba059a2 exits 2, this exits 0. A real loss, now pinned and disclosed rather than implied away. MINOR 5, walkedCorpus() claimed to be "exactly what the widened walk opens". It is the TRACKED files; the walk also opens an untracked, non-gitignored file under a root. One-directional, so the tripwire can under-cover but never over-claim. Corrected, and given -z so a C-quoted path cannot become a name no file has. The gate caught this remediation red-handed on its own rule: a first draft of the new banner SPELLED an example SSN and a real address, scripts/ is now under its own scan, and phi-scan reported a hit on itself. The banner now names the shapes without spelling them, the literals stay in the exempt test file, and CLAUDE.md carries the one-line imperative. Re-derived rather than carried: 16 of the suite's 74 cases red against ba059a2's scanner (a first draft read 14 of 68, before these cases existed), measured by swapping that scanner under the head suite. CLAUDE.md is 31,449 of its 31,451 budget. Room was made by relocating two rationale clauses already verbatim in documentation/agent-notes.md and by cutting meta-text. No trap was deleted. verify: twelve steps ran, 430 tests pass, ten green. The two red steps are pre-existing and their inputs cannot have moved: pnpm audit (transitive advisories under @modelcontextprotocol/sdk) and pnpm licenses (ERR_PNPM_MISSING_PACKAGE_INDEX_FILE on the vendored @cosyte/fhir tarball), with package.json and pnpm-lock.yaml byte-identical to base. --- .changeset/phi-scan-widen-the-walk-roots.md | 14 +- CHANGELOG.md | 18 ++- CLAUDE.md | 19 +-- documentation/agent-notes.md | 51 +++++-- scripts/phi-scan.ts | 81 ++++++++-- test/scripts/phi-scan.test.ts | 158 +++++++++++++++++++- 6 files changed, 298 insertions(+), 43 deletions(-) diff --git a/.changeset/phi-scan-widen-the-walk-roots.md b/.changeset/phi-scan-widen-the-walk-roots.md index 2b14879..beb590d 100644 --- a/.changeset/phi-scan-widen-the-walk-roots.md +++ b/.changeset/phi-scan-widen-the-walk-roots.md @@ -18,9 +18,17 @@ 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. `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. +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 diff --git a/CHANGELOG.md b/CHANGELOG.md index e5ec061..24b97aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,10 +34,22 @@ still do. Each entry was assigned to the release whose tag first contains it, re - **`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; only the root a refusal is filed under changed. + 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. @@ -51,6 +63,10 @@ still do. Each entry was assigned to the release whose tag first contains it, re 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 diff --git a/CLAUDE.md b/CLAUDE.md index e6413ad..912571a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,8 +3,7 @@ > **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.** 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**; that file's own header records when and why, and -> nothing was deleted. +> 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 @@ -181,6 +180,9 @@ Why: [agent-notes § The pre-commit PHI gate and git mv](documentation/agent-not 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 @@ -348,9 +350,8 @@ return 0`, so no `--profile`, `--ignore-rules` or config setting reaches that ea 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. @@ -395,10 +396,10 @@ Mirrors the three disciplines in the meta-repo's `documentation/conventions.md`. 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 ff4a8ce..afb4100 100644 --- a/documentation/agent-notes.md +++ b/documentation/agent-notes.md @@ -238,7 +238,9 @@ address written into `test/planted.test.ts`, in this repo's own inline-message s 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. -**Fourteen of the suite's 68 cases red against `ba059a2`'s scanner.** +**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 @@ -248,21 +250,50 @@ still refused, through the observation rule's OTHER condition (git tracks in-sco 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. - -**What is deliberately NOT a root, each measured rather than omitted.** `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/` (every -tracked file under them is `.md`, which the walk skips by design, so declaring them would add -reconciliation surface and open not one byte); `.github/` and the repository root (measured clean, and -neither is where this package writes messages - and rooting at the repository root is the one sibling -shape that got caught enumerating a build transient). +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. diff --git a/scripts/phi-scan.ts b/scripts/phi-scan.ts index a870ffa..7a6341c 100644 --- a/scripts/phi-scan.ts +++ b/scripts/phi-scan.ts @@ -333,19 +333,62 @@ const OVERRIDE_LOG_PATH = join(REPO_ROOT, "phi-scan-overrides.md"); // 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/`: every tracked file -// under them is `.md`, which the walk skips by design, so declaring them -// would add reconciliation surface and open not one new byte. -// - `.github/` and the repository root: measured clean under the floor, and -// neither is where this package's PHI-shaped literals live. Rooting at the -// repository root is a shape exactly one sibling has, and it is the one that -// got caught enumerating a build transient. +// - `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 @@ -366,12 +409,24 @@ const SCRIPTS_ROOT = join(REPO_ROOT, "scripts"); * 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, and - * that is worth knowing rather than rediscovering. 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. What changes is only which root the - * message is filed under. + * 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: SRC_ROOT, rel: "src" }, diff --git a/test/scripts/phi-scan.test.ts b/test/scripts/phi-scan.test.ts index f55286e..7d34da9 100644 --- a/test/scripts/phi-scan.test.ts +++ b/test/scripts/phi-scan.test.ts @@ -1163,6 +1163,123 @@ describe( }, ); +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. @@ -1206,8 +1323,18 @@ describe("phi-scan: the deliberate-violator exemption", { timeout: SLOW_MS }, () 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. `hasAssertions` because a uid that ignores - // mode bits would otherwise make the early return a silent pass. + // 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 }); @@ -1222,8 +1349,13 @@ describe("phi-scan: the deliberate-violator exemption", { timeout: SLOW_MS }, () } catch { readable = false; } - expect(typeof readable).toBe("boolean"); - if (readable) return; + 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 }); @@ -1338,10 +1470,22 @@ describe("phi-scan: the escape-decoded view was MEASURED and DECLINED, and stays return out; } - /** Exactly what the widened walk opens: tracked, non-`.md`, under the three roots. */ + /** + * 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", "--", "src", "test", "scripts"]) - .split("\n") + return gitOut(REPO_ROOT, ["ls-files", "-z", "--", "src", "test", "scripts"]) + .split("\0") .filter((p) => p.length > 0 && !p.toLowerCase().endsWith(".md")); }