Repository navigation
Pin Node to the releases that block the eMudhra intermediates - #10
Conversation
Node 18 (EOL 2025-04-30) and 20 (EOL 2026-04-30) get no security backports, including the TLS SPKI blocklist for the compromised eMudhra intermediates (nodejs/node#66871). Declare the supported floor in engines so consumers are warned on an unpatched-forever runtime. npm also synced the lockfile's stale root license field to MIT.
Drop end-of-life 18 and 20 to match engines.node >=22 and cover every supported line. check-latest makes setup-node resolve the newest patch from nodejs.org instead of the runner's cached toolchain, so CI and releases pick up the eMudhra blocklist fix as soon as it is released.
engines.node still admits any Node 22+, including releases that accept chains through the compromised eMudhra intermediates. Pin the expected floors (first release per line containing nodejs/node#66871) and check package.json, the lockfile and the README against them.
These are the first releases on each supported line whose TLS stack rejects chains containing the blocked eMudhra public keys (nodejs/node#66871; changelog commits 4342503fd8, 01acf0ee74, a2caddd20e). Consumers' OTLP/HTTPS exporters validate TLS with their own Node, so the package should not claim support for unpatched releases. Odd lines 23 and 25 are end-of-life and stay excluded.
Pinning a version number only proves intent; the probe proves the runtime actually rejects chains through the compromised eMudhra intermediates (CERT_REVOKED, as nodejs/node#66871 reports it) while still connecting to a normally trusted host. Unpatched v24.22.0 fails it.
check-latest picked whatever patch nodejs.org listed, which gave no guarantee the eMudhra blocklist was present. Pin the exact first fixed release per line (matching engines.node), release on 24.22.1, and run the TLS probe on each, which also logs process.version. The test now fails if the workflows drift from the engines floors. Kept as the last commit because it edits .github/workflows.
reviewer9io
left a comment
There was a problem hiding this comment.
The engine floors and exact CI/release versions agree, and ordinary certificate/network failures are distinguished correctly. Reviewed the full seven-file diff and its library, test, packaging and workflow consumers at ea06eccd8f6299095a6a928dc34dd475856e5019.
P2
[P2] Bound the TLS requests before gating CI and releases on them — scripts/tls-probe.mjs:12. A connection that never produces a response/error leaves both the CI and release acceptance step waiting without a deadline. Add a finite request deadline, destroy the timed-out request and report failure. The inline thread includes the runnable regression: both stalled-endpoint cases failed twice; four controls passed each time, and all six passed with a minimal local deadline control.
Fresh Node 24.22.1 verification passed: strict locked install, all 17 existing tests, lint and build. Current-head CI passed on 22.23.4, 24.22.1 and 26.12.1; those successful responses do not cover the stalled-dependency case. Local probes mocked HTTPS; no live TLS, npm publication or GitHub release creation was performed. Advisory rollout: consumers still need to run the updated package on a patched Node runtime; merging does not update existing installations.
Coverage
- Ran: Intent, Correctness and safety, Reliability, Cost and resource, Contract and rollout, Tests
- Skipped: Maintainability and design (Small version/workflow/probe change with no structural redesign.), Guidance compliance (No applicable repository guidance resolved.), Prior discussion (No prior reviews or discussion comments exist.)
A host that accepts the connection and never answers leaves the probe's promise unsettled, so the CI and release steps that gate on it wait until GitHub cancels the job instead of failing.
The probe gates CI and releases on two external hosts. Without a deadline, a host that accepts the connection and never responds hangs the job; now the request is destroyed with ETIMEDOUT and the probe exits non-zero.
|
reviewer9io
left a comment
There was a problem hiding this comment.
The 15-second deadlines fix the previously reported CI/release stall, with timeout errors visible in the probe output. The original six-case regression now passes on 48078ea2814b5080e2e578fb6cbb43e8dac39c2b; no actionable findings remain.
Reassessed the full eight-file PR and every line of the two-file update. Reused the complete review at ea06eccd8f6299095a6a928dc34dd475856e5019 only for Git-identical source/configuration, with matching package, lockfile and runtime identities.
Fresh isolated Node 24.22.1 verification: 11 affected tests passed, and all six original regression cases passed. Replacing only the script with the previous version made both stall assertions fail while four controls passed, confirming the new tests detect the fix. Prior unchanged library tests, lint and build were not repeated. Current-head CI passed on 22.23.4, 24.22.1 and 26.12.1. Local HTTPS was mocked; no live TLS, release or deployment was exercised.
Advisory rollout: consumers must still upgrade their own Node runtime; merging this package change does not update existing installations.
Coverage
- Ran: Intent, Correctness and safety, Reliability, Cost and resource, Contract and rollout, Tests, Prior discussion
- Skipped: Maintainability and design (Small version/workflow/probe change without structural redesign.), Guidance compliance (No applicable repository guidance in the resolved manifest.)
Review only the commits after
f2d864f. This branch reused the merged #9 branch, so its old commits show up in the list but add nothing to the diff.What
test: require Node releases with the eMudhra blocklist: addstest/node-versions.test.ts, which pins the patched floors and checkspackage.json,package-lock.jsonand the README against them.build: floor Node at 22.23.4, 24.22.1 and 26.12.1: setsengines.nodeto^22.23.4 || ^24.22.1 || >=26.12.1(package.json and lockfile) and updates the README requirement.build: add TLS probe for the eMudhra SPKI blocklist: addsscripts/tls-probe.mjs. With default TLS verification it logsprocess.version, requiresCERT_REVOKEDfromhttps://dv-g3b-emudhra.we-have-a-problem.com/and requires a successful connection tohttps://nodejs.org/. The script is not in the publishedfiles.ci: pin CI and release to the patched Node releases: CI matrix22.23.4 / 24.22.1 / 26.12.1and release24.22.1, both withoutcheck-latest, and both run the probe. The test also fails if the workflows drift fromengines.node. This is the last commit because it edits.github/workflows.Why
Node's embedded root store accepted chains through the compromised eMudhra intermediates until nodejs/node#66871.
engines.node >=22andcheck-lateststill allowed unpatched releases. The fix ships in these releases, checked against Node's changelogs:4342503fd8)01acf0ee74)a2caddd20e)Each entry is "tls: reject chains containing blocked public keys". Odd lines 23 and 25 are end-of-life and never got the fix, so they are excluded.
Node execution surfaces in this repo:
ci.yml, matrix)release.yml, which creates a GitHub release only; there is no npm publish)engines.node)engines.node)There is no
.nvmrc/.node-version, no Dockerfile and no deployed runtime, so there is no image digest to pin and nothing to redeploy.Tests
test/node-versions.test.tsfailed 3/3 before the engines change and passes after. Changing the CI matrix to26.12.0makes it fail.jest(17/17),npm run lintandnpm run buildpass at each of the 4 new commits, except the intentionaltest:commit.process.versionand theCERT_REVOKED/trusted-host result for 22.23.4, 24.22.1 and 26.12.1. That CI run is the acceptance evidence.The probe depends on the incident's public test host. Once that host is retired, drop the probe step; the version pins and the test stay.
Fixes #8
Agent usage
claude-opus-5-5· $2.81 · 158 input · 40,411 output · 3,536,168 cache-read · 161,714 cache-write-1hclaude-sonnet-5-5· $0.24 · 26 input · 3,675 output · 353,852 cache-read · 42,530 cache-write-1hOpened by coder9io. Review comments and CI results on this PR are picked up automatically.