* test(#4651): failing-first coverage for final-extension classification
Phase 1 of epic #4636, absorbing #4580. Tests only; no fix. These MUST fail.
The guard classifies a name by comparing everything after `.env.` as one
token against a set whose members are FINAL EXTENSIONS. So `.env.local.example`
yields suffix `local.example`, which is not a member, and a committed
secret-free template is refused. That is a category error, not strictness.
Two arms are covered because the same classification is hand-rolled twice in
one file: `isSecretBasename` for Read/Bash, and `globAltSelectsSecret`
(`lit.startsWith('.env.')`) for Grep globs. Fixing one alone would ship a
guard that allows `cat .env.local.example` while refusing
`Grep --glob '.env.local.example'` — the same file, the same hook, opposite
answers. A cross-arm parity loop over one shared list asserts the two cannot
drift.
Rows that exist because they are the ones nobody enumerates:
- `.env.example.local` must stay BLOCKED. Final extension is `local`; this is
dotenv's documented local-override convention and a real secret. Any fix
shaped as "contains example" admits it.
- `.env.local.` must stay BLOCKED — empty final extension is not a member.
- `.env.` must stay ALLOWED. Note #4580's proposed patch adds
`if (suffix === '') return true;`, which flips it to blocked; that breaks the
existing `allows` assertion in this suite and broadens the protected set,
which epic #4636's non-goals forbid. Not applied.
- `.env.local.exam*` (partial glob literal) must stay BLOCKED — it can select
`.env.local`, and a partial literal cannot be classified.
- `*.example` and `*` must stay ALLOWED — regression protection on the arm
that already works.
Local behavioral repro of the current guard, confirming the tests fail for the
right reason rather than by construction:
.env.local.example rc=2 (blocked) <- the defect
.env.example rc=0 (allowed)
.env.local rc=2 (blocked)
.env.example.local rc=2 (blocked)
.env. rc=0 (allowed)
glob .env.local.example rc=2 <- the second arm
Regressions are folded into the owning module's suite rather than a new
tests/fix-NNNN-*.test.cjs file, per scripts/lint-regression-test-names.cjs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#4651): classify by final extension so .env.<name>.example is readable
Phase 1 of epic #4636, absorbing #4580. Implements ADR-4650 decision 5.
The guard compared everything after `.env.` as ONE token against a set whose
members are FINAL EXTENSIONS. `.env.local.example` yielded `local.example`,
which is not a member, so a committed, secret-free template was refused — the
guard blocked the one file that exists so nobody has to open the real `.env`.
That is a category error, not strictness. The fix is not "add local.example to
the set"; it is to compare the right token. hooks/lib/filename-classification.js
now owns that distinction and is the only place it is expressed.
Both arms are fixed, because the same classification was hand-rolled twice in
this one file:
- isSecretBasename (Read/Bash) now tests finalExtension(suffix).
- globAltSelectsSecret (Grep --glob) split its first branch. With no
wildcard the alternative IS a whole filename, so it is classified exactly
via isSecretBasename. With a wildcard present the literal is only a
PARTIAL prefix (`.env.local.exam*` can still select `.env.local`) and
cannot be classified, so the original conservative rule stays.
Fixing only the first would have shipped a self-contradicting guard: `cat
.env.local.example` allowed while `Grep --glob '.env.local.example'` refused —
same file, same hook, opposite answers. A cross-arm parity loop over one shared
list now asserts the two cannot drift.
Two deliberate departures from #4580's suggested patch, both verified:
- Its `if (suffix === '') return true;` is NOT applied. That flips `.env.`
from allowed to blocked, breaking an existing assertion in this suite and
broadening the protected set, which epic #4636's non-goals forbid.
- `fullSuffix` was drafted alongside finalExtension and removed before
commit: zero production consumers, and none planned (Phases 2-4 are
containment, duplicate draining and the path-join ratchet, none of which
classify filenames). A zero-caller export is dead code. The distinction is
pinned instead by a test asserting finalExtension('local.example') is
'example' and explicitly NOT 'local.example'.
The protected set is unchanged. `.env.example.local` stays BLOCKED — its final
extension is `local`, dotenv's local-override convention and a real secret;
any fix shaped as "contains example" admits it.
Scoped out by measurement, not assumption: src/validate.cts:395 and
src/phase.cts:1674 also hand-roll lastIndexOf('.'), but both parse phase
identifiers (`3.2` -> parent `3`), owned by the phase-id.cts seam. Folding
them in would repeat this same category error in the opposite direction.
Checkpoint 1 (prove RED) on the tests-only commit 91d3d6e1: outcome=failed,
26 failures / 45330, all 26 in the two new test files, zero pre-existing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#4651): document the widened template exemption and cover the Bash arm
Two findings from the isolated adversarial review, both fixed in place.
1. The header's "Stated cost" passage named only the four literal template
names, but since this change the exemption keys on the FINAL EXTENSION, so
the trusted set is `.env.<anything>.{example,sample,template,dist}` — an
unbounded family. The reviewer demonstrated it: `.env.prod-real-secrets.example`
is allowed. That is the deliberate and necessary cost of fixing #4580, but
it was materially larger than what the header disclosed, and a silent
expansion of a security guard's trusted set is not acceptable. The passage
now states the family, the concrete bypass, and that it applies across
Read, Grep and Bash alike.
2. The cross-arm parity loop asserted Read and the exact-literal Grep glob but
not Bash, whose `namesSecret` -> `isSecretBasename` path is genuinely
distinct. The Bash arm was covered only by two one-off tests outside the
shared table, so the table could not have caught a drift there. The loop now
drives all three arms from the same TEMPLATES/SECRETS arrays.
No classification logic changed. The Read-arm behavioral table is byte-identical
before and after: rc=0 for .env.local.example / .env.example / .env. ; rc=2 for
.env.local / .env.example.local / .env / .secrets.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#4651): treat trailing dots and spaces as aliases of the protected file
Closes a Windows path-alias bypass surfaced by the isolated adversarial review
of this phase. Maintainer-approved as in scope.
Win32 strips trailing dots and spaces from every path component, so `.env.`,
`.env..`, `.env `, `.env. `, `.env .`, `.secrets.` and `.secrets ` all resolve
to the real `.env` / `.secrets` on Windows. The guard allowed every one of them
— a bypass of a file it already protects, reachable from Read, Grep and Bash
alike. `isSecretBasename` now normalizes the basename before classifying.
The whole class is fixed, not the reported name. `.env.` alone would have left
`.secrets.` and the trailing-space forms open, which is the same
one-cause-explains-every-failure trap this epic exists to close.
Two consequences, both measured rather than assumed:
- `.env.example.` flips blocked -> ALLOWED. It aliases the already-trusted
`.env.example` template, so this is correct; it was previously blocked only
because the trailing dot broke final-extension parsing.
- A Bash token that is exactly `.env` plus trailing whitespace flips
allowed -> BLOCKED. Verified this is CONSISTENCY, not a new false-positive
class: the bare `.env` token was ALREADY blocked as an operand in the same
position before this change, so the alias now simply behaves like the thing
it aliases.
The header's "No whitespace trimming" guarantee is preserved and now stated
precisely: leading and interior whitespace is still never trimmed, so prose
like a commit message mentioning `.env` in a sentence stays prose and stays
allowed. Only TRAILING dots and spaces are stripped. Two tests pin that.
This lands at the same behavior #4580's proposed `if (suffix === '') return
true;` would have produced for `.env.`, which this phase earlier rejected. The
rejection was correct on its stated grounds — that line broadens the protected
set, which epic #4636's non-goals forbid. The Windows framing is different:
normalizing an alias of an already-protected file is not a broadening, and the
fix is reached by normalization rather than by special-casing an empty suffix,
so it generalizes to `.secrets.` and the space forms.
Cannot be reproduced on this host — the remote matrix is Linux-only and Windows
coverage arrives from CI — so this ships on the Win32 path-normalization
contract plus the CI lane, and that limitation is stated rather than implied.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#4651): one owner for path segmentation, closing a Read/Grep divergence
Four findings from the two-axis review, all fixed in place.
The real one: the guard had TWO path-segmentation rules. `lastSegment` (used
by Read and Bash via `namesSecret`) splits on both `/` and `\`, while
`classifyGrepGlob` hand-rolled its own on `/` only. Measured:
Read of `config\.env` rc=2 BLOCKED
Grep --glob 'config\.env' rc=0 ALLOWED
Same logical file, opposite answers — precisely the divergence this epic
exists to remove, sitting inside the file this phase was already fixing.
`lastSegment` now lives in hooks/lib/filename-classification.js and both arms
call it. All five path-bearing cases (both separators) now agree.
Note on how this was nearly missed: the first measurement of it reported
"both allow", which looked like the reviewer was wrong. That reading was a
measurement artifact — `config\.env` inside a printf'd JSON payload is an
invalid escape, so the hook fails open at rc=0 and the test was observing
JSON breakage rather than the predicate. Re-measured with correct escaping,
the divergence is real. The tests added here use properly escaped literals
and were verified by running, not by reasoning about the escaping.
Also fixed:
- Both fast-check properties were satisfied by a degenerate
always-return-'' implementation: "never contains a dot / is a suffix" and
"never ends with dot-or-space / is a prefix" are both trivially true of
the empty string. They now additionally pin content preservation — the
removed tail must match /^[. ]*$/, and a name with nothing to strip must
come back unchanged.
- The cross-arm parity loop used only bare basenames, so it could not have
caught the divergence above. It now covers path-bearing names with both
separators.
- That loop's description overclaimed: Read and Bash BOTH route through
`namesSecret`, so they are not independent paths; only the Grep glob arm
is genuinely separate. The description now says so rather than implying
three-way independence.
- `normalizeWindowsBasename` runs on every platform, not only Windows. Its
doc now states that explicitly: the guard must answer identically
everywhere, and a name is judged by what Win32 would resolve it to.
No classification logic changed; the 12-name regression sweep is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(#4651): regenerate install-tree goldens, correct the guard's user-facing docs
Three things, all consequences of the fix rather than new behavior.
1. Install-tree goldens. `hooks/lib/filename-classification.js` is a SHIPPED
file — package.json `files` includes `hooks` — so every per-runtime install
tree gains a path. Checkpoint 2 failed on exactly this: 11 failures, all in
tests/golden-install-tree.test.cjs, against 45356 passing. Regenerated via
scripts/gen-install-tree-fixtures.cjs; 11 goldens changed, matching the 11
failures one-for-one.
This ripple was identified at design time and then not acted on. Fleet's
impact preview named golden-install-tree.test.cjs before any code was
written, and 40-design.md records it under "Ripples identified". Writing a
risk down is not the same as discharging it, and a full matrix run was spent
discovering something already known.
2. docs/USER-GUIDE.md made a precise and now-false claim about the guard's
protected set: it named `.env.example` / `.sample` / `.template` / `.dist`
as the four exempt names. The exemption keys on the FINAL EXTENSION, so the
exempt set is the unbounded family `.env.<anything>.{example,sample,template,dist}`.
The page now states that family, the widened residual, that order matters
and only the last segment counts (`.env.example.local` is a secret), and
that trailing dots and spaces are stripped because Windows resolves them to
the protected file. A wrong user-facing model of what a security guard
protects is worth correcting even though Fixed/Security changesets are
exempt from the required-docs rule.
docs/ARCHITECTURE.md and docs/INVENTORY.md say "templates such as
`.env.example` exempt" — non-exhaustive, still true, deliberately left
alone. Same for the ja-JP / zh-CN / ko-KR / pt-BR rows, which carry the same
hedged phrasing; hand-translating a security description unreviewed is not
something to do silently.
3. Two changeset fragments, not one. A refusal corrected is `Fixed`; a bypass
closed is `Security`. Folding the second into the first would under-report
it in the release notes. Both carry `pr: 0` for backfill once the PR exists.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(#4651): backfill changeset PR number to 4659
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
241646a43a