Raise the fast-uri override past the 4.x vulnerable range - #624
Merged
Conversation
`npm Audit (TypeScript SDK)` has been failing on `main` — GHSA-7p8r-x3mc-p8w7, high severity, host confusion via a backslash authority introducer. Every PR opened against main inherits the red check, so this blocks the v0.13.0 freeze. The override already existed at `>=3.1.3`, which is why this reads as a regression rather than a gap. The advisory patches three separate lines: < 2.4.4 -> 2.4.4 >= 3.0.0, < 3.1.5 -> 3.1.5 >= 4.0.0, < 4.1.2 -> 4.1.2 `>=3.1.3` satisfied none of them and, worse, left npm free to resolve 4.1.1 — inside the third vulnerable range. Bumping to `>=3.1.5` alone reproduces exactly that: the lockfile resolves 4.1.1 and the audit still fails. Only `>=4.1.2` excludes all three ranges. Verified rather than reasoned: override >=3.1.3 -> resolved 4.1.1 -> audit REAL_EXIT=1 override >=3.1.5 -> resolved 4.1.1 -> audit REAL_EXIT=1 override >=4.1.2 -> resolved 4.1.2 -> audit REAL_EXIT=0, "found 0 vulnerabilities" Ran with `--audit-level=high`, the same threshold CI uses. The lesson for the other three overrides in that block: a floor written as `>=x.y.z` constrains only the low end, so a later major can walk back into a newer vulnerable range without the override appearing stale. Worth revisiting whether these should be bounded rather than open-ended. fast-uri is a transitive dependency of the TypeScript toolchain, not a runtime dependency of the SDK, so there is no consumer-visible change. Verified anyway: make ts-check REAL_EXIT=0 79 files, 1280 tests passed make conformance-typescript REAL_EXIT=0 210 passed, 2 skipped (pre-existing)
There was a problem hiding this comment.
🟡 Not ready to approve
The lockfile regeneration unintentionally strips libc fields from 8 optional @oxlint/binding-linux-* native packages, contradicting the npm-11.17 preservation convention established in #616 and risking incorrect binary selection on musl systems.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR raises the fast-uri npm overrides floor in the TypeScript SDK from >=3.1.3 to >=4.1.2 to remediate GHSA-7p8r-x3mc-p8w7, whose 4.x-line fix is 4.1.2. The prior open-ended >=3.1.3 floor left npm free to resolve the vulnerable 4.1.1, failing npm audit --audit-level=high on main and turning every downstream PR check red. fast-uri is a toolchain-only transitive dependency, so there is no runtime/consumer-visible change.
Changes:
- Set the
fast-urioverride to>=4.1.2intypescript/package.json. - Re-resolve
typescript/package-lock.jsonsofast-urimoves4.1.1→4.1.2. - Lockfile also (unintentionally) drops
libcfields from 8@oxlint/binding-linux-*optional native packages.
File summaries
| File | Description |
|---|---|
| typescript/package.json | Raises the fast-uri override floor to >=4.1.2 to force the patched release. |
| typescript/package-lock.json | Re-resolves fast-uri to 4.1.2; also strips libc constraints from @oxlint native-binary entries (regression). |
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
Review details
Files not reviewed (1)
- typescript/package-lock.json: Generated file
- Files reviewed: 1/2 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
902
to
903
| "dev": true, | ||
| "libc": [ | ||
| "glibc" | ||
| ], | ||
| "license": "MIT", |
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
The conformance runner's `pretest` shelled into typescript/ and ran
`npm install`, so every `make check` reconciled and rewrote the lockfile as a
side effect. `npm ci` installs from the lockfile without writing it back, which
is what a check was always supposed to do.
The churn was reported as macOS-vs-Linux. It isn't — it is npm-version vs
npm-version, and the platform correlation is an artifact of who runs which npm.
Same package.json, resolved four ways:
macOS npm 10.9.8 fresh -> 034075df11e3
Linux npm 10.9.8 fresh -> 034075df11e3
macOS npm 11.19.0 fresh -> 3c96ea13247b
Linux npm 11.19.0 fresh -> 3c96ea13247b
Byte-identical across platforms within an npm major, divergent across majors.
npm >= 11.19 records a `libc` array on Linux-only optional dependencies; every
older npm drops it. The repo pins node 22 (npm 10.9.8), CI uses node 22 and 24
(npm 11.5.1), and Dependabot runs a current npm — so the writers straddle the
threshold. #624 lost eight such arrays to a local reconcile; dc5f17e, a
Dependabot bump, had added them the same way.
Chasing a toolchain pin does not fix that, because Dependabot's npm is not ours
to pin. Reconciling both candidate lockfiles under all three npms in play:
stripped (no libc) npm 10.9.8 / 11.5.1 / 11.19.0 -> fixed point under all three
libc-bearing npm 10.9.8 / 11.5.1 -> stripped back out
The committed stripped form is the only fixed point, so it stays. Restoring the
`libc` arrays would re-arm the identical 24-line churn for every npm below
11.19, including the one this repo pins. Revisit when the node floor moves past
that threshold; until then the lockfile matches what the pinned toolchain
produces, which is the rule worth keeping.
check-npm-lockfile-readonly enforces the invariant this fix claims: no
package.json lifecycle script and no Makefile recipe may use a lockfile-writing
npm subcommand. CI runs on Linux with `npm ci` and so cannot observe the symptom
itself; the gate is static and catches the reintroduction instead. Restoring the
old `pretest` fails it at scripts.pretest; a probe recipe fails it at the
Makefile line. scripts/bump-version.sh is exempt by name — a reviewed release
step, not reachable from `make check`.
Also lower the fast-uri override from `>=4.1.2` to `>=3.1.5 <4`. Both escape
GHSA-7p8r-x3mc-p8w7, but `@redocly/ajv` declares `fast-uri: ^3.0.1`, so the 4.x
floor forced a transitive major outside its declared range; 3.1.5 is the patched
release on the line it actually asks for. Measured at CI's threshold:
>=3.1.5 <4 -> 3.1.5 -> npm audit --audit-level=high exit 0, 0 vulnerabilities
3.1.5 -> 3.1.5 -> exit 0
>=4.1.2 -> 4.1.2 -> exit 0
3.1.4 -> 3.1.4 -> exit 1, GHSA-7p8r-x3mc-p8w7 (gate is live)
Closes #612.
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
The conformance runner's `pretest` shelled into typescript/ and ran
`npm install`, so every `make check` reconciled and rewrote the lockfile as a
side effect. `npm ci` installs from the lockfile without writing it back, which
is what a check was always supposed to do.
The churn was reported as macOS-vs-Linux. It isn't — it is npm-version vs
npm-version, and the platform correlation is an artifact of who runs which npm.
Same package.json, resolved four ways:
macOS npm 10.9.8 fresh -> 034075df11e3
Linux npm 10.9.8 fresh -> 034075df11e3
macOS npm 11.19.0 fresh -> 3c96ea13247b
Linux npm 11.19.0 fresh -> 3c96ea13247b
Byte-identical across platforms within an npm major, divergent across majors.
npm >= 11.19 records a `libc` array on Linux-only optional dependencies; every
older npm drops it. The repo pins node 22 (npm 10.9.8), CI uses node 22 and 24
(npm 11.5.1), and Dependabot runs a current npm — so the writers straddle the
threshold. #624 lost eight such arrays to a local reconcile; dc5f17e, a
Dependabot bump, had added them the same way.
Chasing a toolchain pin does not fix that, because Dependabot's npm is not ours
to pin. Reconciling both candidate lockfiles under all three npms in play:
stripped (no libc) npm 10.9.8 / 11.5.1 / 11.19.0 -> fixed point under all three
libc-bearing npm 10.9.8 / 11.5.1 -> stripped back out
The committed stripped form is the only fixed point, so it stays. Restoring the
`libc` arrays would re-arm the identical 24-line churn for every npm below
11.19, including the one this repo pins. Revisit when the node floor moves past
that threshold; until then the lockfile matches what the pinned toolchain
produces, which is the rule worth keeping.
check-npm-lockfile-readonly enforces the invariant this fix claims: no
package.json lifecycle script and no Makefile recipe may use a lockfile-writing
npm subcommand. CI runs on Linux with `npm ci` and so cannot observe the symptom
itself; the gate is static and catches the reintroduction instead. Restoring the
old `pretest` fails it at scripts.pretest; a probe recipe fails it at the
Makefile line. scripts/bump-version.sh is exempt by name — a reviewed release
step, not reachable from `make check`.
Also lower the fast-uri override from `>=4.1.2` to `>=3.1.5 <4`. Both escape
GHSA-7p8r-x3mc-p8w7, but `@redocly/ajv` declares `fast-uri: ^3.0.1`, so the 4.x
floor forced a transitive major outside its declared range; 3.1.5 is the patched
release on the line it actually asks for. Measured at CI's threshold:
>=3.1.5 <4 -> 3.1.5 -> npm audit --audit-level=high exit 0, 0 vulnerabilities
3.1.5 -> 3.1.5 -> exit 0
>=4.1.2 -> 4.1.2 -> exit 0
3.1.4 -> 3.1.4 -> exit 1, GHSA-7p8r-x3mc-p8w7 (gate is live)
Closes #612.
jeremy
added a commit
that referenced
this pull request
Aug 4, 2026
* Stop `make check` rewriting typescript/package-lock.json
The conformance runner's `pretest` shelled into typescript/ and ran
`npm install`, so every `make check` reconciled and rewrote the lockfile as a
side effect. `npm ci` installs from the lockfile without writing it back, which
is what a check was always supposed to do.
The churn was reported as macOS-vs-Linux. It isn't — it is npm-version vs
npm-version, and the platform correlation is an artifact of who runs which npm.
Same package.json, resolved four ways:
macOS npm 10.9.8 fresh -> 034075df11e3
Linux npm 10.9.8 fresh -> 034075df11e3
macOS npm 11.19.0 fresh -> 3c96ea13247b
Linux npm 11.19.0 fresh -> 3c96ea13247b
Byte-identical across platforms within an npm major, divergent across majors.
npm >= 11.19 records a `libc` array on Linux-only optional dependencies; every
older npm drops it. The repo pins node 22 (npm 10.9.8), CI uses node 22 and 24
(npm 11.5.1), and Dependabot runs a current npm — so the writers straddle the
threshold. #624 lost eight such arrays to a local reconcile; dc5f17e, a
Dependabot bump, had added them the same way.
Chasing a toolchain pin does not fix that, because Dependabot's npm is not ours
to pin. Reconciling both candidate lockfiles under all three npms in play:
stripped (no libc) npm 10.9.8 / 11.5.1 / 11.19.0 -> fixed point under all three
libc-bearing npm 10.9.8 / 11.5.1 -> stripped back out
The committed stripped form is the only fixed point, so it stays. Restoring the
`libc` arrays would re-arm the identical 24-line churn for every npm below
11.19, including the one this repo pins. Revisit when the node floor moves past
that threshold; until then the lockfile matches what the pinned toolchain
produces, which is the rule worth keeping.
check-npm-lockfile-readonly enforces the invariant this fix claims: no
package.json lifecycle script and no Makefile recipe may use a lockfile-writing
npm subcommand. CI runs on Linux with `npm ci` and so cannot observe the symptom
itself; the gate is static and catches the reintroduction instead. Restoring the
old `pretest` fails it at scripts.pretest; a probe recipe fails it at the
Makefile line. scripts/bump-version.sh is exempt by name — a reviewed release
step, not reachable from `make check`.
Also lower the fast-uri override from `>=4.1.2` to `>=3.1.5 <4`. Both escape
GHSA-7p8r-x3mc-p8w7, but `@redocly/ajv` declares `fast-uri: ^3.0.1`, so the 4.x
floor forced a transitive major outside its declared range; 3.1.5 is the patched
release on the line it actually asks for. Measured at CI's threshold:
>=3.1.5 <4 -> 3.1.5 -> npm audit --audit-level=high exit 0, 0 vulnerabilities
3.1.5 -> 3.1.5 -> exit 0
>=4.1.2 -> 4.1.2 -> exit 0
3.1.4 -> 3.1.4 -> exit 1, GHSA-7p8r-x3mc-p8w7 (gate is live)
Closes #612.
* Fix the shared-node_modules race, invert the gate to an allowlist
Three review findings from #631.
1. `npm ci` in the runner's pretest raced ts-check under `make -j`.
The pretest shelled into typescript/ and installed there, and typescript/
node_modules is shared with ts-check. `npm install` merely rewrote the lockfile;
`npm ci` deletes node_modules before installing, so swapping one for the other
traded a dirty file for a target that can delete dependencies out from under a
sibling mid-run — intermittent, load-dependent, and worse than the bug.
Fixed in the dependency graph rather than the command. conformance-typescript
(and -live) now depend on ts-build, which routes the install through the
ts-install stamp that make already serialises. The runner's own `npm ci` touches
only its private node_modules. pretest keeps a read-only existence check so a
bare `npm test` in the runner directory still says what to do instead of failing
on a missing import.
$ make -n conformance-typescript
cd typescript && npm ci
touch typescript/node_modules/.install-stamp
cd typescript && npm run build
cd conformance/runner/typescript && npm ci && npm test
2. The writer denylist false-greened four ordinary spellings.
npm accepts any unambiguous prefix, so `npm in` and `npm ins` are `npm install`;
and flags may sit between the command and its subcommand, so
`npm --prefix ../x install` and `npm --prefix=../x install` never matched an
`npm install` pattern at all. All four write the lockfile. A denylist has to
enumerate every spelling of every writer forever, which is the wrong shape.
Inverted: ALLOWED_SUBCOMMANDS is a fail-closed inventory of invocations that
cannot write a lockfile, and anything else fails with instructions to extend it.
The parser skips flags, consumes value-taking flags with their arguments, and
resolves the subcommand, so abbreviations and flag-prefixed forms land in the
failing branch by construction. `npm audit` passes; `npm audit fix` does not.
scripts/test-check-npm-lockfile-readonly drives the gate from outside with
synthetic single-commit repos — 20 cases, including all four bypasses, the #612
pretest verbatim, the bump-version.sh exemption, and a control proving the same
content is rejected under any other name. Shown to be non-vacuous: three
mutations of the gate each fail it (allow `install` → 9 cases, drop the
value-flag skip → 1, drop the path form → 20). The value-flag case fails on the
message fragment rather than the exit status, which is the reason the assertions
check both.
Two false positives surfaced while wiring it and are pinned as accept-cases: a
subcommand can arrive wearing the quote that closed the string it sat in
(`npm ci'`), and "node/npm is required" is prose, not a path to npm.
3. The `libc` threshold is exactly npm 11.11.0, bisected rather than asserted.
npm 11.6.0 fresh -> 0 libc entries
npm 11.10.0 fresh -> 0
npm 11.11.0 fresh -> 18
npm 11.12.0 fresh -> 18
The PR body's macOS-vs-Linux framing is corrected separately; the mechanism is
npm-version dependent and byte-identical across operating systems within a
version.
* Gate: run in-process, reject unparsable npm, assert the SDK build is fresh
Three more review findings from #631.
1. The gate cost 234 seconds, against a Makefile comment claiming 0.2.
check_text reached scan_line through a `< <(…)` process substitution, which
forks a subshell per call, and it was called on every tracked line — 15,431
under scripts/ alone. Now scan_line writes to a REASONS global and runs
in-process, and both file walks pre-filter with `grep -n npm`, since only lines
mentioning npm can carry a violation. 234s -> 0.588s measured. The Makefile
comment now says ~0.6s and why.
2. A backslash continuation walked through the allowlist.
npm --prefix \
typescript install
Each half alone looks harmless: the first line has no subcommand, the second has
no npm. The parser ran off the end, left `sub` empty, and the empty case fell
through to silent acceptance — the one branch that was not fail-closed. An npm
invocation whose subcommand cannot be resolved is now reported as such, so the
rejection rests on "could not read this", not on recognising a writer.
Two cases pin it, and the mutant that restores the silent-accept fails exactly
those two and nothing else:
accept unparsed npm -> exit 1, 2 failing cases
FAIL npm --prefix \ <newline> install (line continuation)
FAIL a bare npm with its subcommand off the end of the line
3. pretest accepted any dist/index.js, however stale.
The runner resolves the SDK through its package exports, which point at
typescript/dist, so an existence check let `npm test` in the runner directory
report green against code no longer in the tree. The previous pretest could not:
it rebuilt every run. That rebuild is what raced ts-check, so it cannot come
back.
conformance/runner/typescript/assert-sdk-built.mjs replaces the existence check
with a read-only freshness assertion — newest mtime under typescript/src, plus
tsconfig.json and package.json, against dist/index.js. It writes nothing and
touches no shared state, and `make ts-build` satisfies it by construction:
freshly built -> exit 0
source touched after -> exit 1, "SDK build is stale"
dist absent -> exit 1, "SDK is not built"
rebuilt -> exit 0
Self-test is now 22 cases. Gate, self-test, actions lint, runner-test
reachability and its self-test, and conformance-typescript (214 passed,
2 skipped) all green.
* Split attached shell operators; catch deleted sources in the freshness check
Two more review findings from #631, both bypasses of the guards added earlier.
1. `npm ci;npm install` walked through the allowlist.
Shell operators need no surrounding whitespace, so `read -a` left `ci;npm` as a
single token. Its `ci` prefix was allowlisted and the outer scan never saw the
embedded second `npm` as a token at all. Same for `&&`, `|`, and an install
inside `$(…)`.
Every operator is now padded before tokenising, so each becomes its own word and
any npm following one starts a fresh command. Four cases pin it, and the mutant
that removes the padding fails exactly those four:
drop operator padding -> exit 1, 4 failing cases
FAIL npm ci;npm install (no space around ;)
FAIL npm ci&&npm install (no space around &&)
FAIL npm ci|npm install (no space around |)
FAIL an install inside a command substitution
2. The freshness assertion still passed after a source was DELETED.
It compared the newest *file* mtime under src against dist. A deletion leaves
every surviving file older than dist while dist still carries the removed
module, so nothing looked stale. Directory mtimes are the only record of a
deletion, and they are now walked too — which also covers additions and renames.
scripts/test-assert-sdk-built drives the assertion at synthetic SDK trees
through a new CONFORMANCE_SDK_ROOT override; the deleted-source case cannot be
staged in the real checkout without destroying it. 9 cases. The mutant that
restores file-only mtimes fails exactly the three that need directories:
file mtimes only -> exit 1, 3 failing cases
FAIL a source DELETED after the build
FAIL a source RENAMED after the build
FAIL a whole source DIRECTORY removed
3. A stale CI comment claimed a guard that does not exist.
test.yml justified using `npx vitest run` over `npm test` by saying "the
runner's dist-freshness globalSetup still fails loudly if dist/ were stale."
There is no such globalSetup anywhere in the runner, and never was — which is
precisely why a stale dist/ could have been tested silently. The comment now
says what is actually true, and names assert-sdk-built.mjs as the guard that was
missing.
Self-tests: npm gate 26 cases, freshness 9. Gate runs in 0.457s. Local gates,
actions lint, runner-test reachability + self-test, and conformance-typescript
(214 passed, 2 skipped) all green.
* Fail closed on npm options the parser cannot classify
Sixth review finding on #631, and the same shape as the last three: npm accepts
any config key as `--key value`, so an option this parser does not recognise
cannot be assumed valueless.
npm --shell ci install --package-lock-only
`--shell` takes a value, so `ci` is that value and the real subcommand is
`install` — but the parser read `ci`, found it allowlisted, and passed the line.
The invocation writes package-lock.json.
Options are now allowlisted the same way subcommands are, in three cases:
--anything=value one token, unambiguous, skipped whatever it is
VALUE_FLAGS consumes the next token too
VALUELESS_FLAGS consumes nothing
Anything else is reported as unclassifiable rather than guessed at. `--shell` is
also added to VALUE_FLAGS, but that is not what protects the case — mutating it
back out leaves the self-test green, because the fail-closed rule catches it as
an unrecognised option. Only the true pre-fix state fails:
drop --shell, keep fail-closed -> self-test green (general rule holds)
drop --shell AND assume valueless -> exit 1, 2 failing cases
FAIL npm --shell ci install (value swallows a fake subcommand)
FAIL an option this parser cannot classify
Four cases added, two rejecting and two guarding against over-rejection: known
valueless options before the subcommand, and an unknown option in `--key=value`
form, both still accepted. Self-test is 30 cases; the gate still runs in under a
second.
* Unwrap fully-quoted npm tokens, without reading inside quoted strings
Seventh review finding on #631: the shell strips quoting before it resolves the
command, so `"npm" install` and `'npm' install` run exactly as the bare spelling
does — but `read -a` keeps the quotes attached and the token stopped looking
like npm.
The first attempt removed quote characters wherever they appeared. That fixed
the bypass and immediately broke the other direction, on a real line in this
repo:
grep -Eq 'npm (run )?test' <<<"$ts_recipe" \
With quotes gone, that reads as an npm token followed by `(` — reported as an
invocation with no parsable subcommand. Stripping quotes globally means reading
inside quoted strings, and quoted strings are data.
So a token is unwrapped only when the quotes enclose it entirely. `"npm"` is a
command; `'npm` opens a string and is left alone. A trailing escape backslash is
dropped too, which also keeps the line-continuation case working.
Both directions are pinned, and the mutants fail on opposite sides — which is
the point, since either fix alone regresses the other:
no dequoting at all -> exit 1, 3 failing
FAIL a double-quoted executable: "npm" install
FAIL a single-quoted executable: 'npm' install
FAIL an escaped space: npm\ install
strip quotes anywhere -> exit 1, 1 failing
FAIL an npm mention inside a quoted regex — gate exited 1; it must accept this
Self-test is 34 cases. Gate, freshness self-test, actions lint, and runner-test
reachability plus its self-test all green.
* Scan whole multiline scripts; require proof the SDK build completed
Eighth and ninth review findings on #631.
1. A package.json script's second line was never scanned.
JSON permits a literal newline in a script value and npm runs every line, but
the scan lost them twice over: `jq -r` emitted the value across several output
records while the reader took only the first as `name<TAB>cmd`, and `read -a`
stops at a newline regardless. So this scanned clean:
"pretest": "echo ok\nnpm install --package-lock-only"
Records are NUL-terminated now (`jq -j` with an explicit NUL escape), which
keeps each script whole however many lines it spans, and a newline is rewritten
to `;` before tokenising — it is a command separator, and `;` is one the operator
padding already understands. The mutant that restores newline framing fails
exactly the two multiline cases.
2. dist/index.js was treated as proof of a successful build.
typescript/tsconfig.json does not set noEmitOnError, so tsc can emit a partial
dist and still exit non-zero — which skips postbuild, leaving dist/generated
missing or stale while the entry point looks newer than every source. A bare
`npm test` in the runner then accepted it.
postbuild now writes dist/.build-complete once tsc has emitted AND src/generated
has been copied, and the assertion requires that marker and compares its mtime
rather than the entry point's. A build that died halfway leaves the entry point
without the marker, which is the state the guard now names:
SDK build did not complete — .../dist/.build-complete is missing. dist/
exists, so a build started; tsc emitting and then failing leaves exactly
this state.
The mutant that ignores the marker fails exactly the partial-build case.
Self-tests: npm gate 36 cases, freshness 10. Local sweep green — gate, both
self-tests, actions lint, runner-test reachability and its self-test, ts-check
(1290 passed) and conformance-typescript (214 passed, 2 skipped).
* Resolve quote-split command names; clean dist before rebuilding
Two more review findings on #631, plus a scope statement the gate was missing.
1. `n"p"m install` bypassed both pre-filters.
The shell removes quoting before it resolves a command, so that runs npm — but
the line carries no literal `npm` substring, and both cheap filters rejected it
before the parser ran: `grep -n npm` in the file walks, and the `*npm*` test in
check_text.
Quoting is now resolved by balance rather than by position. A token whose quote
characters all close what they open is the command the shell would run, so the
quotes come off: `n"p"m` and `"npm"` both become `npm`. A token that leaves one
open begins a string whose contents are data — `'npm` in
`grep -Eq 'npm (run )?test'` — and is left alone. That distinction is
load-bearing in both directions, and the mutants prove it:
strict `npm` grep filter -> the two split-name cases pass through
whole-token-only unwrap -> same two
unconditional quote removal -> the quoted regex is reported again
The grep pre-filter is correspondingly looser (a couple of non-letters allowed
between n, p and m). Over-matching costs a parse; under-matching was the bypass.
Gate still runs in 0.29s.
2. tsc does not clean outDir, so a successful rebuild could still serve a
deleted module.
Delete src/oauth/index.ts, run `npm run build`, and dist/oauth/index.js survives
— still reachable through the package's `./oauth` export. The freshness marker
is refreshed by that build, so the assertion called it good. The guard was
making a stale build look verified, which is worse than not having it.
`prebuild` now removes dist before tsc runs, so the tree cannot outlive its
sources. Measured on the real build rather than a fixture:
build with a probe source -> dist/stale-probe.js present
delete the source, rebuild -> dist/stale-probe.js gone
assertion after clean rebuild -> exit 0
3. The gate now says what it is.
Its header claimed an invariant without bounding it. It is a regression guard
against an accidental reintroduction — #612 was one well-meant line — and it
reads text with a shell-shaped parser, not a shell. Review has driven that parser
through prefixes, flags before the subcommand, unknown value-taking options,
unpadded operators, continuations, multiline values and split command names, and
each is pinned. But a parser that is not a shell can always be out-argued by
`eval` on an assembled string, and chasing that is not the job. A green run means
nobody reintroduced this by accident; it is not a security boundary.
Self-tests: npm gate 38 cases, freshness 10. Local sweep green — both gates and
self-tests, actions lint, runner-test reachability and its self-test, ts-check
(1290 passed) and conformance-typescript (214 passed, 2 skipped).
* One option-skipper, so the audit lookahead cannot disagree with the scan
Tenth review finding on #631, and the cause is that the same job was done twice,
differently.
`npm audit fix` writes the lockfile, and `fix` is a positional action that may
sit behind options. The subcommand scan consumed `--key value` pairs properly;
the audit lookahead skipped option NAMES only. So this passed:
npm audit --audit-level high fix
The lookahead skipped `--audit-level`, stopped on `high`, and never reached the
`fix`. `--audit-level` was also missing from VALUE_FLAGS, which is what made the
value visible as a positional in the first place.
Both call sites now share one `skip_options`, and `--audit-level` is declared
value-taking. Two mutants, and they fail differently in a way worth reading:
audit lookahead skips names only -> exit 1, 1 failing
FAIL npm audit --audit-level high fix (value hides the action)
--audit-level not value-taking -> exit 1, 4 failing
the writer is still rejected, but as an unclassifiable option rather than
as `npm audit fix`, and `npm audit --audit-level high` starts failing too
The second is the fail-closed design working: getting the option table wrong
costs a false alarm, not a miss.
Three cases added — `--audit-level high fix`, `--json fix`, and the
`--audit-level high` with no action that must still pass. Self-test is 41 cases;
gate runs in 0.318s.
Local sweep green — both gates and self-tests, actions lint, runner-test
reachability and its self-test, ts-check (1290 passed) and conformance-typescript
(214 passed, 2 skipped).
* Scan the argument of sh -c as code, not as a quoted string
Eleventh review finding on #631, and the one place the balance rule was wrong
rather than merely incomplete.
sh -c 'npm install --package-lock-only'
The quotes here are syntax: `-c` takes a program, and the shell runs its
contents. But `'npm` leaves a quote open, and the rule that protects
`grep -Eq 'npm (run )?test'` — an unbalanced quote begins a string whose
contents are data — declined to normalise it. So the gate certified a command
that rewrites the lockfile. `sh -c` is an ordinary way to write a lifecycle
script, not an exotic spelling.
A shell wrapper followed by `-c` now switches the scan into code mode, where
quoting is normalised unconditionally, and an operator ends both the command and
that mode. Data stays data on the far side of the `&&`:
sh -c 'npm ci' && grep -Eq 'npm (run )?test' f
Both directions are pinned, and the mutants fail on opposite sides:
no shell-wrapper handling -> exit 1, 3 failing
FAIL sh -c 'npm install ...'
FAIL bash -c "npm install"
FAIL /bin/sh -c with a writer
wrapper context never reset -> exit 1, 2 failing
FAIL wrapper context ends at the operator before a quoted regex
Five cases added, three rejecting and two guarding the scope of the mode.
Self-test is 46 cases; gate runs in 0.503s.
Local sweep green — both gates and self-tests, actions lint, runner-test
reachability and its self-test, ts-check (1290 passed) and conformance-typescript
(214 passed, 2 skipped).
* Classify --workspaces as the boolean it is, not a value-taking flag
`npm config ls -l` reports `workspace = []` against `workspaces = null`:
the singular names one workspace and takes a value, the plural is a bare
toggle meaning "every workspace". The option-skipper had the plural in
VALUE_FLAGS, so it consumed the token behind it — `npm --workspaces ci`
parsed to no subcommand at all and was reported as unparsable.
Fail-closed throughout, so this was never a missed writer: the rejection
message was wrong, not the verdict. But it is a false alarm on a
read-only invocation, and the gate's whole claim is that a rejection
means something.
Pinned in the self-test from both directions — the plural accepted, the
singular still eating its value so `npm --workspace <name> install` is
still caught as the writer it is rather than mistaken for a subcommand.
49 cases.
* Join backslash-newlines; count the lockfile as a build input
Two more review findings on #631.
1. A continuation hid the audit action.
npm audit --audit-level high \
fix
Rewriting the newline to `;` without consuming the backslash left `\` standing
where the action should be, so the lookahead never reached `fix`. A
backslash-newline is a continuation — the shell removes both and runs one
command — so those are now joined before any newline is rewritten.
Two guards, not one: the audit lookahead also reports when something stands in
the action's position that is not a word, so even with the join removed the
writer is still caught, just named differently. The mutant shows both:
no backslash-newline join -> exit 1, 2 failing
FAIL npm audit ... \ <newline> fix — rejected, but no 'npm audit fix' in output
FAIL: cannot resolve the `npm audit` action from `\`
2. The freshness assertion ignored the lockfile.
A dependency bump can change the TypeScript compiler itself, and so the emitted
JavaScript and declarations, without any source file being touched — dist then
looks current while having been built by a different tsc. typescript/
package-lock.json now counts among the compared build inputs, alongside src,
tsconfig.json and package.json. The mutant that drops it fails exactly the new
case.
Self-tests: npm gate 50 cases, freshness 11. Local sweep green — both gates and
self-tests, actions lint, runner-test reachability and its self-test, ts-check
(1393 passed) and conformance-typescript (221 passed, 2 skipped).
* Stop at inline comments, scan delegated Makefiles, fail on missing build inputs
Three more review findings on #631. Two of them are the same shape as the defect
this gate exists to catch — a check that silently passes.
1. Only the root Makefile was scanned.
`make check` delegates into go/Makefile and swift/Makefile through
`$(MAKE) -C`, so a writer in a delegated recipe was invisible. A gate that
reports success over the file it never opened is exactly the failure mode of
#612. Every tracked Makefile is walked now.
2. An inline comment was reported as a writer.
echo ok # npm install is for local setup
Nothing after an unquoted `#` runs, but the tokenizer read straight past it.
This one is a false alarm rather than a miss, and it had no honest escape —
allowlisting `install` would have gutted the invariant — so the scan stops at a
comment marker.
3. A deleted build input made the build look fresher.
`newestMtime()` reports 0 for a missing path, so deleting typescript/
tsconfig.json LOWERED the computed source time and left the completion marker
looking current: output built against a configuration no longer in the tree.
Absence is staleness, not freshness. The three file inputs must now exist, and
the SDK root's own mtime — which is what records a deletion — is folded in.
Each shown failing against the un-fixed artefact, with the real exit status
written to its own file and grepped back rather than read off a combined log:
1 continuation join removed MY_REDPROOF_1_EXIT_IS 1
FAIL npm audit ... \ <newline> fix (continuation hides the action)
2 comment stop removed MY_REDPROOF_2_EXIT_IS 1
FAIL an inline shell comment mentioning npm install
FAIL a trailing comment after a permitted command
3 root Makefile only MY_REDPROOF_3_EXIT_IS 1
FAIL a writer in a delegated Makefile
4 lockfile not an input MY_REDPROOF_4_EXIT_IS 1
FAIL the lockfile changed after the build
5 missing input tolerated MY_REDPROOF_5_EXIT_IS 1
FAIL tsconfig.json deleted after the build
FAIL package-lock.json deleted after the build
The delegated-Makefile case builds its own synthetic repo — root `check`
invoking `$(MAKE) -C go check`, nested recipe running
`npm install --package-lock-only` — rather than relying on the reported one.
Self-tests: npm gate 54 cases, freshness 13. Local sweep green — both gates and
self-tests, actions lint, runner-test reachability and its self-test, ts-check
(1393 passed) and conformance-typescript (221 passed, 2 skipped).
* Treat a multi-word quoted span as data, except inside sh -c
Twelfth review finding on #631, and the last of the false-alarm class.
echo "please npm install here"
The shell hands that to `echo` as one argument, but `read -a` splits it into
words and `npm` looked like an executable. Like the inline-comment case, this had
no honest remedy — allowlisting `install` would have gutted the invariant — and
it would have blocked ordinary diagnostics and documentation strings.
Open quoted spans are now tracked across words and their contents skipped. The
exception is `sh -c '…'`, where the quoted text is a program rather than an
argument: code mode wins, and the span is scanned as code. Both directions are
covered by cases, since either rule alone gets the other wrong:
echo "please npm install here" accepted (data)
echo 'run npm install by hand' accepted (data)
sh -c 'npm install --package-lock-only' rejected (code)
Red proof against the un-fixed gate, with span tracking removed:
MY_REDPROOF_6_EXIT_IS 1
FAIL npm install inside a quoted echo argument — gate exited 1; it must accept this
FAIL the same in single quotes, in a Makefile recipe — gate exited 1; it must accept this
Self-test is 57 cases. Local sweep green — both gates and self-tests, actions
lint, runner-test reachability and its self-test, ts-check (1393 passed) and
conformance-typescript (221 passed, 2 skipped).
* Decode embedded escapes; only classify words in command position
Two more review findings on #631 — one a miss, one a false alarm, and the fix
for the second is what the parser had been missing all along.
1. An embedded escape ran npm without looking like it.
np\m install --package-lock-only
Bash removes the backslash before resolving the command, so that runs the real
npm. Normalisation dropped only a TRAILING backslash, so the matcher never saw
it. Escapes are decoded now.
2. A quoted argument was read as a command.
echo "npm" install
The shell passes `"npm"` to echo. The balanced-quote rule unwrapped it wherever
it appeared and reported a forbidden invocation — another false alarm with no
allowlist remedy.
The missing concept was COMMAND POSITION. Only the first word of a command can
be the executable, and the position is restored by an operator. Words that run
other commands pass it along, so `npx --yes npm@11 install` still resolves —
`npx` holds the position open across its flags. Everything else consumes it.
This subsumes a lot: `echo checking npm install support` is now accepted for the
right reason rather than by luck, and `echo $(npm install)` is still rejected
because `(` restores the position.
Both shown failing against the un-fixed gate, real exit status written to its own
file and grepped back:
7 escapes not decoded MY_REDPROOF_7_EXIT_IS 1
FAIL an embedded escape: np\m install — gate exited 0; it must reject this
8 no command-position tracking MY_REDPROOF_8_EXIT_IS 1
FAIL a quoted npm as an ARGUMENT, not a command — gate exited 1; it must accept this
FAIL npm named in the middle of a command line — gate exited 1; it must accept this
Self-test is 60 cases, and all 57 earlier ones still pass — including the npx
pin, which is what proves the command-position rule did not over-tighten. Local
sweep green: both gates and self-tests, actions lint, runner-test reachability
and its self-test, ts-check (1393 passed), conformance-typescript (221 passed,
2 skipped).
* Drop npm exec from the allowlist; assignments keep the command position
Two more review findings on #631, both the same shape: a writer reached through
something the parser approved without looking at what it wrapped.
1. `npm exec -- npm install --package-lock-only`
`exec` was on the allowlist, so the outer invocation was approved and the nested
`npm install` never examined. `exec` runs an arbitrary delegated command, and
"approve the wrapper, ignore what it wraps" is not something an allowlist entry
can express — so it is off the list rather than special-cased. Nothing in this
repo uses it; anything that needs it has to parse the delegated command.
2. `CI=1 npm install --package-lock-only`
An environment assignment prefix was treated as the executable, clearing the
command position, so the `npm` that followed was skipped. Assignments now keep
the position, as do flags and command-running prefixes.
Both reproduced here rather than taken on report, and both shown failing against
the un-fixed gate — real exit status written to its own file and grepped back:
9 exec back on the allowlist MY_REDPROOF_9_EXIT_IS 1
FAIL npm exec -- npm install (writer behind a wrapper) — gate exited 0
10 assignment consumes position MY_REDPROOF_10_EXIT_IS 1
FAIL CI=1 npm install (assignment prefix) — gate exited 0
FAIL two assignments then a writer — gate exited 0
Four cases added, three rejecting and one guarding the other direction
(`CI=1 npm ci` must still pass). Self-test is 64 cases. Local sweep green: both
gates and self-tests, actions lint, runner-test reachability and its self-test,
ts-check (1393 passed), conformance-typescript (221 passed, 2 skipped).
* Observe the invariant instead of only predicting it
Review has now found nine spellings the static gate's parser missed, across four
rounds: prefixes, flags before the subcommand, unknown value-taking options,
unpadded operators, continuations, multiline values, quoting that splits the
command name, `sh -c`, `npm exec` delegation, assignment prefixes. Every one was
real and each got a rule. The pattern is the finding, not any single case.
The suggested inversion — treat anything unclassifiable as a failure — does not
work at the layer where the misses happen. The parser IS already fail-closed
wherever it classifies: an unknown subcommand, an unrecognised option and an
unresolvable subcommand are all reported. The misses are one step earlier, in
deciding whether a word is a command at all, and that question is shell parsing.
Inverting THERE means failing on every quoted mention of npm — review reported
two such false alarms itself (`echo ok # npm install …` and
`echo "please npm install here"`), neither with an allowlist remedy, because
prose and diagnostics mention npm constantly. A gate that fails on those gets
switched off within a week.
So rather than keep predicting, `make check` now also observes.
scripts/assert-lockfiles-unchanged hashes every tracked lockfile before the
checks run and again afterwards; any difference fails. `check` is a thin wrapper
around `check-targets` so the two hashes bracket the whole run. It parses
nothing. `eval` on an assembled string, a subcommand held in a variable, a writer
three delegated Makefiles deep, a spelling nobody has thought of yet — all of
them change bytes, and none can talk a byte comparison out of noticing.
The two are complementary, and neither is redundant:
* the static gate fails at the commit that introduces the writer, naming file
and line, and works even on a machine where the write would be a no-op —
which is the common case, since the committed lockfile is a fixed point
under every npm in play;
* the tripwire cannot say what went wrong, only that something did, and only
where the write actually changes bytes — but it cannot be out-spelled.
Proven rather than asserted. The self-test records, mutates, verifies — 9 cases,
including the two mechanisms no text parser resolves:
ok an npm lockfile rewritten — rejected (exit 1)
ok Gemfile.lock rewritten — rejected (exit 1)
ok uv.lock rewritten — rejected (exit 1)
ok a lockfile deleted — rejected (exit 1)
ok written via eval of an assembled string — rejected (exit 1)
ok written by a command held in a variable — rejected (exit 1)
And against the real repository, driving the exact sequence `check` runs:
MY_TRIPWIRE_LIVE_EXIT_IS 1
FAIL: the checks modified a dependency lockfile.
4c69c0050607… typescript/package-lock.json
7bf293d5e87b… typescript/package-lock.json
Restored, it reports "Lockfiles unchanged by the checks (4 files,
byte-identical)" and exits 0 — which is what every `make check` run now prints.
It covers Gemfile.lock and uv.lock too. Nothing writes those today; the guard is
the same invariant and costs nothing to extend.
* Tripwire: watch the filesystem, and report even when the checks fail
Two review findings against the tripwire added in a7e1fff — both holes in the
thing meant to have no holes, which is the right place to be strict.
1. A lockfile created where none existed was in neither snapshot.
The walk used `git ls-files`, which lists what is already tracked. A check that
created a root package-lock.json therefore appeared on neither side of the
comparison and verification passed over it, leaving the tree dirty. It now walks
the filesystem for lockfile names, pruning node_modules/.git/dist/build, so a
file that did not exist before and does now is exactly what it notices.
2. Verification never ran when the checks failed.
`@$(MAKE) check-targets` aborting the recipe meant a target that wrote a lockfile
and THEN failed produced only its own error — the dirty tree went unexplained,
which is precisely the case where the diagnostic is worth most. `--verify` now
runs regardless, and the sub-make's exit status is preserved rather than
overwritten.
Both reproduced here. For (1), the self-test driven at an index-based copy:
MY_REDPROOF_11_EXIT_IS 1
FAIL a NEW untracked lockfile created at the repo root — exited 0
FAIL a new lockfile in a directory that had none — exited 0
For (2), the two recipe shapes run against a synthetic Makefile whose
`check-targets` writes a lockfile and exits 7:
PRE-FIX (abort on failure) exit 2, tripwire diagnostic shown: False
FIXED (verify regardless) exit 2, tripwire diagnostic shown: True
and the self-test's wrapper case pins the status contract itself —
WRAPPER_EXIT=7, not 1, so a lockfile failure cannot mask what actually broke.
Self-test is 12 cases. Local sweep green: three gates and their self-tests,
actions lint, runner-test reachability and its self-test, ts-check (1393 passed),
conformance-typescript (221 passed, 2 skipped).
* Tripwire: cover the Go checksum files too
`go` rewrites go.sum as readily as `npm` rewrites a lockfile, and `make check`
runs Go targets — so `go/go.sum`, `conformance/runner/go/go.sum` and
`go.work.sum` belonged in the snapshot from the start. Nothing writes them today;
the guard is the same invariant and costs one line.
Red proof against the un-fixed tripwire, with the Go names dropped from the walk:
MY_REDPROOF_13_EXIT_IS 1
FAIL go/go.sum rewritten — exited 0; it must reject this
FAIL the conformance runner's go.sum rewritten — exited 0; it must reject this
FAIL go.work.sum rewritten — exited 0; it must reject this
Self-test is 15 cases.
* Keywords keep the command position; one snapshot per check invocation
Two more review findings, and the second is a live hazard in this repo rather
than a hypothetical.
1. Shell keywords consumed the command position.
if false; then npm install --package-lock-only; fi
`then` was treated as the command, clearing the position, so the `npm` after it
was skipped. Same for `while`/`do`, `else`, and `!`. Keywords are syntax, not
commands, and the position belongs to the word after them.
This one matters beyond the parser: a conditional that is false in CI but true on
a developer's machine is exactly the case where the static gate has to catch what
the runtime tripwire would only notice after that developer's lockfile had
already been rewritten.
2. Concurrent runs shared one snapshot path.
The tripwire keyed its baseline on the user, so two `make check` runs — this repo
is worked in from ~30 worktrees at once, and a sibling worktree was running one
while this was being written — overwrote each other's baseline. `check` now
allocates the path with mktemp per invocation and threads it through both calls,
and the bare-run fallback is keyed on the repo root.
Both shown failing against the un-fixed artefact:
14 keywords consume position MY_REDPROOF_14_EXIT_IS 1
FAIL if false; then npm install; fi — gate exited 0
FAIL while ...; do npm install ...; done — gate exited 0
FAIL ! npm install (negated) — gate exited 0
FAIL a writer inside an else branch — gate exited 0
15 one snapshot path per user MY_REDPROOF_15_EXIT_IS 1
FAIL default-path baselines crossed — clean=1 dirty=1
Getting 15 to bite took fixing the test first: the two fixture repos were
byte-identical, so a crossed baseline was undetectable. They now differ by
content, which is what makes the case real.
Self-tests: npm gate 68 cases, tripwire 17, freshness 13.
* Make the byte comparison authoritative in CI, on every job
The static parser has now been patched five rounds running — continuations,
comments, delegated Makefiles, `exec` delegation, assignment prefixes, control
keywords. Writing a shell parser by increments is not a winnable game, and the
right response is to stop relying on grammar for the guarantee.
`--verify-clean` is the CI form of the tripwire. A CI checkout starts pristine,
so the committed tree is already the baseline and no snapshot is needed: it
asserts that no dependency manifest differs from HEAD and that none has appeared.
It runs at the end of all 21 jobs that install dependencies — 12 in test.yml,
9 in security.yml — with `if: always()`, so a job cannot both fail and hide what
it dirtied. It parses nothing, so no spelling reaches it.
It covers untracked APPEARANCE, not only tracked mutation. That was the gap
worth closing: a script creating a lockfile where none was tracked is invisible
to anything built on `git ls-files`. `git status --untracked-files=all` sees it.
MY_REDPROOF_16_EXIT_IS 1 (--verify-clean restricted to tracked files)
FAIL a NEW untracked lockfile created — exited 0; it must reject this
FAIL a new lockfile in a new directory — exited 0; it must reject this
On demoting the static gate: not done, deliberately, and the reason is measured
rather than a preference. The committed lockfile is a fixed point under npm
10.9.8, 11.5.1 AND 11.19.0 — every npm in play — which is exactly why it was
chosen. So a reintroduced `npm install` running in CI writes back the SAME bytes,
and the dynamic check is silent for it. The dynamic check is ground truth for
"did this run dirty the tree", which is the release-blocking property; it is not
a detector of a reintroduced writer until some npm's output diverges from the
committed file. The static gate is the only thing that catches that beforehand.
So the division of labour, now stated in both scripts:
* dynamic, authoritative, unevadable — did anything change? Cannot be
out-spelled, but is silent while writes are byte-neutral.
* static, best-effort, specific — is anything ABLE to write? Catches the
reintroduction at the commit, with a file and line, before the bytes ever
diverge; and every miss review has found was a false negative in that
early-warning role, never a hole in the clean-tree guarantee.
Demoting the static gate to non-gating would leave the reintroduction case
covered by nobody until an npm release moves the fixed point. Keeping both
costs 0.5s.
Self-tests: tripwire 24 cases, npm gate 68, freshness 13.
* CI: run the manifest check from the repo root
Jobs with a `working-directory` default resolved `./scripts/...` against that
subdirectory, so the step exited 127 rather than running. `working-directory: .`
on the step; the script cds to the repo root itself, which is what the git
pathspecs need.
* CI: skip the manifest check in the aggregator job
The `conformance` job has no checkout — it only reads the other jobs' results —
so the step exited 127 there. Every job that actually installs dependencies keeps
it: 11 in test.yml, 9 in security.yml.
* Demote the static parser to a diagnostic; the byte comparison is the gate
Six rounds of review found eleven spellings the parser missed: unambiguous
prefixes, flags before the subcommand, unknown value-taking options, unpadded
operators, line continuations, multiline script values, quoting that splits the
command name, `sh -c`, `npm exec` delegation, assignment prefixes, control
keywords — and this round, a recipe split mid-token, `env -C`, and make's own
`@` prefix. Every one was a SILENT hole, which is the worst failure mode a gate
can have. Separating code from data is shell parsing; this is not a shell, and
patching grammar does not converge.
Codex said it plainly in the mid-token thread: "otherwise only the post-check
tripwire notices." The tripwire already catches all of them. So make that the
architecture rather than leaving it implicit.
scripts/check-npm-lockfile-readonly -> scripts/lint-npm-lockfile-writes
It prints and exits 0. `--strict` still exits non-zero, which is how its own
self-test observes the rejections. A spelling it misses is now a weaker message,
never a shipped hole. The Makefile target and CI step are renamed to match, and
both scripts state the division of labour in their headers:
* assert-lockfiles-unchanged — the guarantee. Parses nothing. Compares the
bytes of every dependency manifest before and after the checks, and in CI
against the committed tree. Cannot be out-spelled. Silent only while a write
is byte-neutral.
* lint-npm-lockfile-writes — best-effort. Reads source, so it can be fooled.
Worth keeping for the one thing the comparison cannot do: name the file, the
line and the reason at the commit that reintroduces a writer — which today it
would not even notice, since the committed lockfile is a fixed point under
npm 10.9.8, 11.5.1 and 11.19.0.
The three live findings are fixed anyway, because a diagnostic that misses the
standard silent-recipe form is not worth much:
* `@npm install` / `+npm update` — make strips `@`, `-` and `+` before the
shell sees them.
* `np\` + newline + `m install` — Makefile recipe lines are joined on a
trailing backslash before filtering, so a token split across the break is
seen whole. This also lets the earlier `npm --prefix \` case resolve all the
way to `npm install` rather than merely being unreadable.
* `env -C . npm install` — command-running prefixes now consume their own
value-taking options, so `.` is not mistaken for the command.
Self-test is 73 cases.
Two items from earlier are already in: the snapshot path is allocated per
invocation with mktemp and keyed on the repo root for bare runs (a673d3e), and
the untracked-appearance case is covered in both modes, with `find` locally and
`git status --untracked-files=all` in CI (5f34e2b, 918dc3b).
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
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.
npm Audit (TypeScript SDK)is failing onmain, so every PR opened against it inherits a red check. This blocks the v0.13.0 freeze.GHSA-7p8r-x3mc-p8w7 — high severity,
fast-urihost confusion via a backslash authority introducer.Why this reads as a regression, not a gap
An override already existed at
>=3.1.3. The advisory patches three separate lines:< 2.4.4>= 3.0.0, < 3.1.5>= 4.0.0, < 4.1.2>=3.1.3satisfies none of them — and, worse, leaves npm free to resolve 4.1.1, which sits inside the third range. An open-ended floor constrains only the low end.Bumping to
>=3.1.5alone reproduces exactly that failure. Measured, not reasoned:Run with
--audit-level=high, the same threshold CI uses. 4.1.2 is the current latest.Worth a look beyond this fix
Three sibling overrides in that block are written the same open-ended way:
A
>=x.y.zfloor cannot stop a later major from walking into a newer vulnerable range, and the override will not look stale while it happens — which is precisely what occurred here. Worth deciding whether these should be bounded rather than open-ended. Not changed in this PR: one advisory, one fix, and the others are not currently failing.Related:
brace-expansionneeded the same treatment two days ago (#616), which is a second data point for the same pattern.Verification
fast-uriis a transitive dependency of the TypeScript toolchain, not a runtime dependency of the SDK, so there is no consumer-visible change. Verified regardless:Only
typescript/package.jsonandtypescript/package-lock.jsonare touched. Committed bytes verified against the working tree.Summary by cubic
Raises the
fast-urioverride to>=4.1.2and updates the lockfile sonpm auditpasses onmain(GHSA-7p8r-x3mc-p8w7). This fixes the failing TypeScript SDK audit check and unblocks the v0.13.0 freeze.fast-urioverride to>=4.1.2; previous>=3.1.3allowed vulnerable4.1.1.fast-uri@4.1.2.npm audit --audit-level=highnow reports 0 vulnerabilities; toolchain-only change, no runtime impact.Written for commit 634cc2e. Summary will update on new commits.