diff --git a/.changeset/silly-hens-relax.md b/.changeset/silly-hens-relax.md deleted file mode 100644 index 9ca0e888d..000000000 --- a/.changeset/silly-hens-relax.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Security -pr: 4552 ---- -**Pinned the transitive `hono` dependency to `>=4.13.5`** — fixes a moderate-severity path-traversal/DoS advisory chain (GHSA-gqvv-2mrq-wpjv, GHSA-g6gw-c38x-mqfc, GHSA-crvj-82cr-hjcx) in `hono <4.13.5`, pulled in transitively via `@anthropic-ai/claude-agent-sdk` -> `@modelcontextprotocol/sdk`. Discovered blocking `tests/npm-integrity-gate.test.cjs` while verifying #4460; fixed inline per this repo's no-defer policy rather than left for a separate PR. (#4460) diff --git a/.changeset/tame-hens-jump.md b/.changeset/tame-hens-jump.md index 0ffd02483..6e8519b93 100644 --- a/.changeset/tame-hens-jump.md +++ b/.changeset/tame-hens-jump.md @@ -1,5 +1,5 @@ --- type: Fixed -pr: 4552 +pr: 4572 --- -**`npm run check:env`'s npm-version check no longer times out under CI contention, and no longer misreports a timeout as a missing binary** — the check hand-rolled its own `spawnSync` call with a 10s timeout, duplicating (imperfectly) this repo's canonical OS-shell-projection seam. Discovered live: an unrelated PR's Windows CI shard failed this check twice under heavy concurrent test load. Now routed through `execNpm` (the same seam other npm-invoking code already uses), which gives it the canonical 15s timeout and the canonical, cross-platform-correct timeout detection — a real timeout is now reported as one, distinct from npm genuinely being absent. (#4460) +**`npm run check:env`'s npm-version check no longer misreports a timeout as a missing binary** — every `spawnSync` failure mode (ENOENT, a signal-killed timeout under load, a non-zero exit) used to collapse into one message, "npm binary not found on PATH." Discovered live: an unrelated PR's Windows CI shard failed this check twice under heavy concurrent test load, and the message made a real timeout indistinguishable from npm genuinely being absent. The reason is now reported accurately, and the check's own timeout was raised from 10s to 15s to match this repo's other npm-subprocess calls. (#4460) diff --git a/scripts/check-env.cjs b/scripts/check-env.cjs index 3e6e33779..525d331e0 100644 --- a/scripts/check-env.cjs +++ b/scripts/check-env.cjs @@ -196,6 +196,9 @@ function main() { // than inventing a new number. Confirmed via real Windows CI: a 10s // window was insufficient twice under ~51-file concurrent test load. const NPM_VERSION_TIMEOUT_MS = 15_000; + // No try/catch needed: spawnSync's documented contract routes ENOENT and a + // timeout-triggered kill through the RETURNED result's `.error` field, not + // a thrown exception -- there is nothing here for a catch to intercept. const npmVersionSpawn = spawnSync(npmCmd, ['--version'], { encoding: 'utf8', timeout: NPM_VERSION_TIMEOUT_MS, shell: process.platform === 'win32' }); const npmVersionResult = { exitCode: npmVersionSpawn.status ?? 1, diff --git a/scripts/lib/npm-version-check-diagnosis.cjs b/scripts/lib/npm-version-check-diagnosis.cjs index 676cfd2a5..8dfbb45d8 100644 --- a/scripts/lib/npm-version-check-diagnosis.cjs +++ b/scripts/lib/npm-version-check-diagnosis.cjs @@ -46,6 +46,13 @@ function describeNpmVersionCheckFailure(result) { if (result.exitCode !== 0) { return `npm --version exited ${result.exitCode} with no usable output`; } + if (result.exitCode === 0) { + // npm ran and exited cleanly but printed nothing -- distinct from every + // case above (which all involve a failed/absent spawn), so it gets its + // own message rather than falling through to "not found on PATH", which + // would misdescribe a real npm binary that simply produced no output. + return 'npm --version exited 0 but produced no output'; + } return 'npm binary not found on PATH'; } diff --git a/tests/npm-version-check-diagnosis.test.cjs b/tests/npm-version-check-diagnosis.test.cjs index 83cdb3c91..929be27be 100644 --- a/tests/npm-version-check-diagnosis.test.cjs +++ b/tests/npm-version-check-diagnosis.test.cjs @@ -61,11 +61,14 @@ describe('describeNpmVersionCheckFailure (#4460)', () => { ); }); - test('exitCode 0 with no stdout (defensive default) falls back to the original message', () => { + test('exitCode 0 with no stdout gets its own message, not the "not found" fallback', () => { + // npm ran and exited cleanly but printed nothing -- distinct from every + // other branch (all of which involve a failed/absent spawn); must not be + // misdescribed as "not found on PATH" when the binary plainly ran. const result = { exitCode: 0, stdout: '', stderr: '', signal: null, error: null, timedOut: false }; assert.equal( describeNpmVersionCheckFailure(result), - 'npm binary not found on PATH', + 'npm --version exited 0 but produced no output', ); }); });