fix(phi-hook): v1.5 recall-hardening — close 7 false-negatives + ship scan-history - #3
Merged
Merged
Conversation
… scan-history Review found the gate was clean code but weaker than it looked: a heuristic with real false-negatives. Each fix was confirmed empirically (the gap now CATCHES, legit cases still PASS) and is covered by new tests. Suite 61 -> 86, ruff clean. 1. Pattern 5: structured-data gate (.csv/.tsv/.psv) outside allowlists — a patient line-list is the likeliest clinician leak and evaded both the binary gate and the label heuristics. 2. Pattern 2 also matches value-shaped record rows (name + DOB-value + id, e.g. a CSV line), so the literal words DOB/MRN need not appear. 3. Pattern 1 now scans staged file PATHS, not just diff content. 4. Disease allowlist requires BOTH slug tokens be clinical (was: any single disease word exempted the whole slug, so johnson-asthma-r passed). 5. NAME_SHAPE catches middle initials and apostrophes; all-caps acronyms still excluded. 6. Bracket-placeholder strip narrowed to literal placeholder vocab. 7. CRED_FILE catches prefixed secrets (config-secrets.yaml) and *secrets.json. Also: shipped hooks/scan-history.py (claimed in v1.4, was never committed) — a read-only retroactive full-history scanner. All doc/comment examples use placeholders so the hook never flags its own source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rdmgator12
added a commit
that referenced
this pull request
Jun 22, 2026
The hook code shipped in #3 but the repo-level docs didn't reflect it. - CHANGELOG.md: add [1.2.1] entry (Security: 7 false-negatives closed; Added: scan-history.py + 86-test suite). - CONTRIBUTING.md: wire the "don't include personal data" rule to the enforcing pre-commit hook + scan-history.py; add install/audit/test commands. - README.md: list hooks/ in Project Structure; add a Privacy & Safety section. Docs-only; no code change. Hook patterns/tests/ruff unchanged and still green. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
A review of
hooks/phi_hook.pyfound it was clean code but weaker than it looked — a heuristic PHI gate with real false-negatives. This closes all seven, each confirmed empirically (the gap now catches, legit cases still pass) and covered by new tests.Suite 61 → 86 · ruff clean ·
scan-history.pyreports the repo clean across full history.Fixes
patient_list.csvevaded both the binary gate and the label heuristics.csv/.tsv/.psvgate outside allowlistsName,2015-03-01,00123…), not the literal words DOB/MRNjohnson-asthma-rpassed)[Jane Doe, DOB …]was stripped and immunized by the placeholder allowlistJohn A. Smith) and apostrophes (O'Brien) evaded NAME_SHAPEconfig-secrets.yaml/*secrets.jsonslippedhooks/scan-history.py— it was never committedVerification
python -m unittest discover -s hooks/tests→ 86 tests OK (25 new regression tests)ruff check hooks/→ All checks passed!python hooks/scan-history.py→ clean (248 text blobs / 30 commits, no PHI)<First>/<Last>/YYYY-MM-DDso the hook never flags its own source.🤖 Generated with Claude Code