fix: scrub real client and personal data from a public repo - #144
Merged
Conversation
Owner
Author
Post-audit: four regressions this branch introduced, now fixedAn adversarial audit (six lenses, every finding independently refuted) raised 77
Plus:
|
app/__tests__/windfallImport.test.ts carried a genuine Windfall export row -- full name, personal email, customer code, net worth and a boat-owner flag. The comment said it matched the real sample CSV, and it did. Windfall is a wealth-screening service, so that row is a named private individual's finances, published on a public repository since the initial commit. Nothing in the test depended on any of it. The parser is being tested for column-NAME resilience; every assertion passes on any well-formed row. The values are invented now, and a comment says so plainly so nobody restores them from the real CSV to reproduce something. First of several -- the same pattern reaches other fixtures -- but this one is a private individual rather than a business, so it goes on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The repo is public. Beyond the identity leak, three of these were the same bug as CLAUDE.md 61-63: one deployment's naming compiled into matching logic, so a second deployment silently mis-routes rather than failing. - reportRouter matched `Saybrook_Home_*` / `SH_*` report filenames. The discriminator was never the org name -- it is "has an org prefix at all", which is what separates the org's inbound-items export from the vendor's unprefixed standard report of the same name. Now ORDORITE_REPORT_PREFIX, defaulting to any prefix. All 25 existing routing assertions pass unchanged; re-hardcoding the prefix fails the 6 new ones. - isReturnOrder hardcoded the store codes SB/GT/CH/BB/WS/RS. The vendor's convention is the A/M suffix; the codes are ours. Now ORDORITE_STORE_CODES, same permissive default. - Staff emails, a customer's phone, a vendor's phone, and the buyer identity in four parser fixtures replaced with invented values at the same widths -- the fixed-width column offsets are the parsers' contract. 3,442 unit tests pass; tsc clean.
The repo is public and carried one retailer's production data as test fixtures. Removed: 19 real people's names (staff and customers, across 21 files), a customer's and a vendor's phone number, staff email local-parts, the company's email domain, its three store towns, and four real customer street addresses. Replaced with invented values at the same shapes -- the fixed-width column offsets are the parsers' contract. Two guards were the same bug rather than only a leak: - The seed's target check was a BLOCKLIST of database names. It failed open: any name nobody had listed seeded silently, and the list only ever grew after someone lost data. It is now an allowlist -- only a name that says it exists to be seeded (seed/demo/scratch/sandbox/sample) runs unattended. env.example's default moves holt_dev -> holt_demo to match, and setup.sh uses the same rule so it fails before migrating rather than after. This had no test at all; __tests__/seedTargetGuard.test.ts now covers it, and the cases that matter are the unfamiliar names. - restore-drill.sh and apply-preset already gated on allowlists, so their deny-lists were redundant as well as identifying. __tests__/clientDataTripwire.test.ts greps every tracked file for the identifiers found here and fails on a hit. The exemption list is checked in both directions, so a stale entry cannot quietly cover a new leak. Two applied migrations are exempt with the reason stated: editing them breaks their Prisma checksum on live deployments. CLAUDE.md rule 59 and docs/domains/seed-data.md rewritten to describe the allowlist, so the rule and the code say the same thing. 3,457 unit tests pass; tsc and prettier clean.
Both were case or partial variants the replacement missed, and both failed only in the integration suite: - salesPersonFkBackfill asserted a case-insensitive match between a staff displayName and a differently-cased salesperson string; renaming the displayName alone made the pair stop matching, which is what the test is for. - runSalesImport asserted the surname the CSV fixture no longer carried. Full integration suite passes.
CI creates a database called `ci` and runs `npm run setup` against it to prove the first-run path works. The allowlist did not recognise that name, so the boot-and-smoke job failed at the first step -- a name that declares disposability as plainly as "scratch" does was being treated as if it might hold real data. The gap underneath: the allowlist and the database CI actually creates were two facts nothing made agree, so the mismatch could only surface remotely, in the one job that boots the app. seedTargetGuard.test.ts now reads POSTGRES_DB out of .github/workflows/ci.yml and asserts the guard accepts every value it finds, and checks setup.sh's shell copy of the rule still carries the same tokens. Removing "ci" from the allowlist fails both. 3,459 unit tests pass; tsc and prettier clean.
An adversarial audit of this branch raised 77 findings; six survived
independent refutation, and I reproduced each by executing the shipped code.
Four were regressions this branch created.
1. ORDORITE_REPORT_PREFIX could not express the two prefixes a real deployment
uses. It was one escaped scalar, so `A|B` became the literal `A\|B` and
matched nothing -- pinning the prefix silently unrouted every report filed
under the other one (stock, PO lines, the 100K-row product catalog), logged
as "skipped" with no alert. Now a comma-separated list, the convention
ORDORITE_STORE_CODES already used.
2. The org-report routes were matching `.+_<name>` UNANCHORED, so files that
previously returned null -- the loud unrouted-file path -- drove master-data
imports instead: Deleted_Customers.csv and Vendor_Stock_by_Item.csv both
routed. `.` matched `/`, so a directory component counted as a prefix.
Now anchored on the base name, with the prefix optional. Unconfigured, the
router matches bare report names only; it refuses a look-alike rather than
guessing. Only Inbound_Items requires a prefix, because a bare
Inbound_Items is a different report with a different runner.
3. Generalising `/^RS\d/i` to `/^R[A-Z]\d/i` looked source-neutral and widened
it 26x. An unrelated R-series -- RA for a rug account, RX for exchanges --
would import as RETURNED and be subtracted from revenue, with no
configuration able to narrow it back. Now ORDORITE_RETURN_PREFIXES, empty by
default: there is no safe universal guess, so a deployment that uses such a
series names it.
4. ORDORITE_STORE_CODES="," was truthy, split to nothing, and joined to "" --
an empty alternation matching a zero-length store code, BROADER than the
default it replaced. isReturnOrder("A1") became true. A misconfiguration
must never widen a match; it now falls back to the default.
Also: setup.sh's globs were unanchored substrings against guard.ts's
word-bounded regex, so setup.sh accepted `holt_samples` and `demolition_prod`,
ran migrate and seed:roles into them, and only THEN did the seed refuse --
leaving a half-set-up database. The shell arm is now word-bounded, and
seedTargetGuard.test.ts compares the two implementations' DECISIONS across 21
boundary names instead of checking they contain the same words. The old test
passed the whole time this was broken.
And a stale COMPANY_EMAIL_DOMAIN restore left the pre-scrub stem in
ordoriteShared.test.ts, where the next test appended to that describe would
have inherited it.
3,466 unit tests pass; import integration suites pass; tsc, prettier, lint clean.
…pwire 53 findings from the branch audit never got judged -- their verifiers died mid-run. Re-verifying them confirmed 31, and the leaks among them were worse than the first pass caught. People and addresses, all still live at HEAD: - a customer's name beside her home delivery address, in source, two tests and a doc comment - a named customer's spend profile, customer number and return record across four files - a former employee named with her order count, and a departed seller's unattributed revenue - a customer bound to an outage, an order number and a $91,151 revenue gap - a customer and two staff first names in a transfer anecdote - staff names paired with specific orders and commission amounts in a runbook - the deployment's town ZIP, in two ZIP+4 examples Half-renames the first pass left behind, each of which is why a scrub needs a tripwire rather than a careful grep: "Erin Kelly" fixed in one test but left in another and in the parser it documents; "Karen Dwyer" fixed where the name was one string and missed where it was split across firstName/lastName; an "alex robertson" map key orphaned when its sibling was renamed. Seven vendor fixtures published confidential dealer pricing -- one printed Wholesale beside MSRP on the same row, another stated the vendor's entire catalog cost ceiling in a source comment. Every price is now invented, with the arithmetic each parser actually keys on preserved (qty x price == extension, extensions summing to the printed total) and the fixed-width caret alignments in two parser sources unchanged. Real order, PO and reference numbers went with them, since a document number is what ties an invented price back to a real deal. The tripwire now covers all of it: surnames, whole names where the surname is too common to match alone, addresses, the ZIP, vendor document numbers, the specific dealer figures, and the catalog-ceiling phrasing. It found five sites this commit would otherwise have missed, including two where a name wrapped across comment lines. A rep-group company was deliberately dropped from the list -- naming a business is not the harm, publishing its trade terms is. 3,474 unit tests pass; six integration suites pass; tsc and prettier clean.
… renames
Tripwire, which guards everything else and was the weakest part of it:
- It scanned tracked files only, and content only. A file added but not yet
committed, or a file whose CONTENTS are clean but whose NAME is a client's,
both passed. Now scans untracked-but-not-ignored files and every path.
- The catch block adopted any stdout the error carried. A maxBuffer overflow
throws with status null and TRUNCATED stdout, so a partial walk would have
been reported as a clean one -- the exact silent pass the test exists to
prevent. Only a clean exit-1 with no output is a pass now; everything else
throws.
- The stale-exemption check, the suite's only proof it is scanning at all,
exempted the test file itself and so could pass vacuously. That exemption is
gone: the file always hits, because it contains the pattern sources, which
makes it the liveness canary.
- The domain pattern matched only the correct spelling of a name the codebase
itself documents as arriving with typos. Narrowed to the stem.
ORDORITE_STORE_CODES defaulted to `[A-Z]{2,}`, which in front of `[A-Z]*A\d`
matches any letter run whose last letter before a digit is "A" -- SOFA1,
MEGA1234, VIA3 all classified as RETURNED and were subtracted from revenue. No
test could fail on it because every fixture used a real store code. It now
fails closed like ORDORITE_RETURN_PREFIXES; genuine returns are still caught by
the negative-net-total check, so declining to guess costs nothing.
Repairs to the mechanical renames, each of which still compiled and still
passed:
- `akritos` -> `northgate` was collateral. It is the maker's OWN second
deployment, named in NOTICE, SECURITY.md and ARCHITECTURE.md; renaming it in
three files left two names for one thing. Reverted.
- restore-drill.sh's deny-list had been emptied of the non-client names too,
while its comment still promised they were covered.
- apply-preset lost its sentence's subject; journalEntry turned one
deployment's timezone into a false claim about US retailers; four docs were
left with "the the pilot deployment" and lower-cased sentence starts.
- A guard.ts doc comment still named a list this branch deleted.
- The router docs described neither the router nor each other, mixing `SH_` and
`Company_` placeholders. Both are now `<Org>_`, with the env var documented
and the one prefix-required route called out.
Rule 59 and env.example now state that the token must be delimited, so
`holt-demo` and `holt_samples` are refused -- near-misses on purpose.
3,474 unit tests pass; tsc, prettier, lint and markdownlint clean.
The tripwire caught these the moment this branch rebased onto the merged #143, which is the whole reason it exists: the scrub was correct when written, and a merge from another branch put the name straight back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
goetchstone
force-pushed
the
fix/scrub-client-data
branch
from
August 26, 2026 18:29
faf7f0f to
eaf94e0
Compare
…' twins A fresh-eyes sweep -- five hunters told nothing about what the earlier passes had already removed, so they could not be anchored by my assumptions -- raised 60 hits and confirmed 23. The worst was untouched since the initial public release: "Penny/Steve Sigal", a real customer couple, in a quarantined it.skip block. Both earlier passes walked that file and edited lines above and below it. The row's phone and order number are fictional placeholders, which is exactly why it read as synthetic -- but the file's history shows the author's habit was real names beside sanitized numbers, so the fake phone vouched for nothing. The rest is one pattern: the earlier passes scrubbed the FIXTURES and missed their twins. Every vendor's dealer prices survived in the parser source doc comment that documents the fixture, in the home-accessory normalizer fixture that consumes it, or both -- so a comment and the fixture it described no longer even agreed. Two files neither pass had opened at all, nuorderPrintout and kkOrderParser, still carried real PO numbers and wholesale prices beside suggested retail. All of it now uses the same invented values as the corresponding fixture, with each parser's arithmetic preserved (qty x price == extension, extensions summing to the printed total, cancelled-section totals restated) and the aligned caret comments in two parser sources kept to width. Also gone: a real ZIP as the worked example in the address parser, and the last "Erin Kelly" in a JSDoc. The tripwire now covers the document numbers and the specific dealer figures, not just names -- it caught the Sigal line the moment the pattern went in. Its one miss in this pass is worth recording: a bulk substitution applied a short replacement before a longer one that contained it, leaving a fixture line arithmetically inconsistent and its test asserting nothing. Tests caught that, not the tripwire. 3,478 unit tests pass; integration suites pass; tsc, prettier and lint clean.
… repo An independent sweep over docs, config, scripts, prisma and file names came back clean except the two applied migrations already carried as exemptions -- which is the convergence signal worth having. But it surfaced something else by accident: the guard itself. The tripwire listed, in plain text and in one public file, the surnames, the company/town stem, both store towns, the town ZIP, the staff email local-parts and the phone numbers of a real business. It concentrated in a single place exactly what every other file had been scrubbed of, and search engines index it. The guard against publishing a company's identifying data was publishing it. Patterns are now base64. That is not secrecy -- anyone determined decodes it in a second, and the header says so. It stops the repo being a plain-text index of a real company's identifiers, which is the actual harm. `what` stays in plain English: the REASON an entry exists has to remain readable, only the identifier is encoded, and the header carries the one-liner for adding a new one. Removing the plaintext also removed an accident the suite had been leaning on: the file used to match its own patterns, so it always appeared in the hit set and doubled as proof the scan was live. That crutch is gone, so the canary is now deliberate -- a positive control asserting the scan reaches known content and known paths, and it runs first. Every other assertion here passes on an empty result, so a scan that silently reaches nothing would otherwise read as a spotless repo. Verified both ways: planting a scrubbed identifier still fails two cases, and the file no longer contains any of the identifiers it guards. 3,479 unit tests pass; tsc and prettier clean.
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.
This repository is public. It began as one retailer's internal system, and their
production data came along as test fixtures.
What was in here
company's email domain
All replaced with invented values at the same character widths — the
fixed-width column offsets are what the parsers actually key on, so a
same-meaning-different-width substitution would have silently changed what the
tests prove.
Three of these were bugs, not only leaks
The seed's target check was a blocklist and failed open. It named specific
databases; anything nobody had listed seeded silently, and the list only ever
grew after someone lost data. It is now an allowlist — only a name that says it
exists to be seeded runs unattended.
env.examplemovesholt_dev→holt_demoto match, andsetup.shuses the same rule so it fails beforemigrating rather than after. This had no test at all;
__tests__/seedTargetGuard.test.tsnow covers it, and the cases that matter arethe unfamiliar names — refused precisely because nobody thought of them.
The import router matched the client's report filenames. The discriminator
was never the org name — it is "has an org prefix at all", which is what
separates the org's
<Org>_Inbound_Itemsexport from the vendor's unprefixedstandard report of the same name. Now
ORDORITE_REPORT_PREFIX, defaulting toany prefix. All 25 existing routing assertions pass unchanged; re-hardcoding the
prefix fails the 6 new ones.
Return detection hardcoded the client's store codes. The vendor's convention
is the
A/Msuffix; the codes are the deployment's. NowORDORITE_STORE_CODES, same permissive default. (CLAUDE.md 61–63.)Keeping it out
__tests__/clientDataTripwire.test.tsgreps every tracked file for theseidentifiers and fails on a hit. Its exemption list is checked in both
directions, so a stale entry cannot quietly cover a new leak. It found 57
mentions I had missed on its first run.
Two applied migrations are exempt with the reason stated: editing them breaks
their Prisma checksum on every live deployment. They contain narrative comments
only — no contact details, pricing or personal data.
Not addressed here
These identifiers are still in this repo's git history and in any existing
fork. Removing them there means rewriting history, which is a separate call.
Verification
tscclean, prettier clean, 0 lint errors🤖 Generated with Claude Code