From a5b5860b93fd5c3f5edeac69dcb68cd5badd5ab3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 9 Aug 2026 11:39:40 -0400 Subject: [PATCH] fix(#3238): bump js-yaml to the patched 4.3.1 (high-severity devDep advisory) (#3246) * chore(#3238): bump js-yaml 4.3.0 -> 4.3.1 (GHSA-5p4m-2wfm-xmqj) Dependabot alert 14. js-yaml 4.3.0 sits inside the vulnerable range >=4.0.0 <4.3.1 of GHSA-5p4m-2wfm-xmqj -- high, CVSS 7.5 (AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H), CWE-407 Inefficient Algorithmic Complexity. resolveYamlOmap() enforces `!!omap` key uniqueness with a linear objectKeys.indexOf() scan inside the per-element loop, so resolution is O(n^2) in entry count. `!!omap` is registered in the DEFAULT schema, so a plain yaml.load(untrustedInput) with no options is affected. The loop is synchronous, so it blocks the event loop -- amplification is per-process, not per-request. Same weakness as CVE-2026-59870, fixed in 5.x at 5.2.1 and only now backported to the 3.x/4.x lines. Reproduced locally against the installed 4.3.0, advisory PoC, default schema: n= 5000 load= 22ms - n=10000 load= 57ms 2.59x n=20000 load= 199x 3.49x n=40000 load= 768ms 3.86x <- ~4x per doubling = quadratic After the bump, same machine, same PoC: n= 5000 load= 33ms - n=10000 load= 33ms 1.00x n=20000 load= 49ms 1.48x n=40000 load= 96ms 1.96x <- ~2x per doubling = linear Duplicate-key rejection is preserved (YAMLException still raised), so the upstream indexOf -> Set swap kept the semantics it was guarding. Scope is development-only and stays that way: js-yaml is a devDependency and is absent from package.json's `files` allowlist, so it never ships to consumers. `npm audit --omit=dev` reported 0 vulnerabilities before this change and still does; `npm audit` went 1 high -> 0. Targeted install rather than `npm audit fix`, so the blast radius is auditable: npm reports "changed 1 package", and the lockfile diff is 4 insertions / 4 deletions touching only js-yaml. The declared floor moves ^4.2.1 -> ^4.3.1 so a future resolution cannot land back on a vulnerable 4.3.x -- the operative fix is the lockfile, since npm ci is lockfile-driven. Stayed on the v4-legacy line (4.3.1) rather than jumping to `latest` 5.2.3: 5.x is a rewritten module layout and a separate change with its own blast radius. The v4-legacy dist-tag exists precisely so 4.x consumers can take this patch. Regression guard mirrors tests/issue-2765-brace-expansion-lockfile.test.cjs -- the repo's existing precedent for a dev-scope lockfile bump against a high-severity DoS advisory. It walks `npm ls --json --all` so a transitive copy left behind still fails, and carries a vacuity guard so an empty version list cannot pass silently. No timing assertion: wall-clock assertions are barred by the clock-seam rule and would be load-sensitive on shared benches, so the measurements live in the diagnosis artifact instead. Refs #3238 * fix(#3238): require 5.2.1 on the 5.x line; add the release-notes fragment Three review findings, all fixed inline. SPEC AXIS (the serious one): the guard's `(maj > 4)` clause accepted ANY 5.x. GHSA-5p4m-2wfm-xmqj names only the 3.x and 4.x ranges, so the isolated adversarial pass -- checking strictly against that advisory -- rated 5.0.0 as correctly accepted. But the advisory's own body records that the SAME weakness in the 5.x line is CVE-2026-59870 / GHSA-724g-mxrg-4qvm, fixed in 5.2.1. A guard whose purpose is "this tree has no quadratic !!omap resolver" must require 5.2.1 there too, or an accidental major bump to 5.0.0 silently reintroduces the exact bug the test exists to prevent. The two reviewers disagreed and the disagreement was load-bearing: taking only the adversarial verdict would have shipped the hole. ISOLATED ADVERSARIAL (item 4): Number('4.3.1-beta.1') produced NaN, and NaN comparisons made the predicate return false. That failed safe, but by accident rather than design, and majors outside {3,4} were reported vulnerable despite being outside every advertised range. The predicate is now explicit -- build metadata stripped, prerelease fails CLOSED (4.3.1-beta.1 sorts below 4.3.1 and may predate the fix), unparseable fails closed, maj<3 accepted as predating the affected lines. Validated by a standalone harness over 27 version strings (12 accepted, 14 rejected, 1 build-metadata): all 27 agree with the advisory ranges. Boundary rows on all three affected lines -- 3.15.0/3.15.1, 4.3.0/4.3.1, 5.2.0/5.2.1. STANDARDS AXIS: the precedent this change mirrors, 54cb4145b (#2765, identical dev-scope lockfile security bump), shipped `fix(#2765)` plus a `.changeset/*.md` typed Fixed. This change was typed `chore` with no fragment. Per CONTRIBUTING.md, `chore` is omitted from user-facing release notes entirely, so a security fix would have merged invisibly. Adds the fragment (type Fixed, pr:0 to be backfilled) and types this commit `fix`. Note the changeset gate does not require a fragment for a package.json/tests diff (`ok_no_user_facing_changes`) -- it is added because users should be told they received a security patch, not to satisfy a gate. Not fixed, recorded as not-a-defect: the near-duplication of tests/issue-2765-brace-expansion-lockfile.test.cjs. The reviewer that raised it also argued the repo's one-self-contained-guard-per-advisory convention is the better call -- each advisory guard stays independently auditable and removable, and a shared helper would couple unrelated advisories. Extracting it would also be drive-by refactoring of a file this change only appends beside. Refs #3238 * chore(#3238): backfill changeset PR number 3246 --------- Co-authored-by: sim --- .changeset/vivid-pumas-jump.md | 5 ++ package-lock.json | 8 +-- package.json | 2 +- tests/issue-3238-js-yaml-lockfile.test.cjs | 75 ++++++++++++++++++++++ 4 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 .changeset/vivid-pumas-jump.md create mode 100644 tests/issue-3238-js-yaml-lockfile.test.cjs diff --git a/.changeset/vivid-pumas-jump.md b/.changeset/vivid-pumas-jump.md new file mode 100644 index 000000000..30c6020a0 --- /dev/null +++ b/.changeset/vivid-pumas-jump.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3246 +--- +**Dev-dependency `js-yaml` bumped to the patched 4.3.1, resolving a high-severity quadratic-CPU advisory** — the lockfile now pins the backported `!!omap` fix (GHSA-5p4m-2wfm-xmqj, CVSS 7.5), reachable via eslint. A non-breaking in-range bump (no overrides, no major bump, one package moved); production `npm audit --omit=dev` is unaffected (devDependency only). (#3238) diff --git a/package-lock.json b/package-lock.json index d7681c04f..47b165dcc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,7 +28,7 @@ "eslint-plugin-no-only-tests": "^3.4.0", "fast-check": "^4.8.0", "globals": "^16.5.0", - "js-yaml": "^4.2.1", + "js-yaml": "^4.3.1", "typescript": "^6.0.3", "typescript-eslint": "^8.60.0" }, @@ -3842,9 +3842,9 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "4.3.0", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.0.tgz", - "integrity": "sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q==", + "version": "4.3.1", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.1.tgz", + "integrity": "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==", "dev": true, "funding": [ { diff --git a/package.json b/package.json index 62f236cab..c0e22fbd4 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "eslint-plugin-no-only-tests": "^3.4.0", "fast-check": "^4.8.0", "globals": "^16.5.0", - "js-yaml": "^4.2.1", + "js-yaml": "^4.3.1", "typescript": "^6.0.3", "typescript-eslint": "^8.60.0" }, diff --git a/tests/issue-3238-js-yaml-lockfile.test.cjs b/tests/issue-3238-js-yaml-lockfile.test.cjs new file mode 100644 index 000000000..69d1ec669 --- /dev/null +++ b/tests/issue-3238-js-yaml-lockfile.test.cjs @@ -0,0 +1,75 @@ +// allow-test-rule: structural-implementation-guard (#3238) +'use strict'; + +// Regression guard for #3238: the lockfile must pin a patched js-yaml (>=4.3.1 on the +// 4.x line, >=3.15.1 on the 3.x line) to resolve GHSA-5p4m-2wfm-xmqj — a high-severity +// (CVSS 7.5, CWE-407) quadratic-CPU DoS in `!!omap` resolution, vulnerable range +// `>=4.0.0 <4.3.1`. `!!omap` is in the DEFAULT schema, so a plain yaml.load() is +// affected. This is a lockfile-only devDependency bump (direct, plus an +// @eslint/eslintrc dedupe); production (npm audit --omit=dev) was already clean. +// The test pins every installed copy so the bump can't silently regress. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { execFileSync } = require('node:child_process'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); + +// `npm` is not process.execPath, git, or a bash script/hook, so this does not +// route through tests/helpers/process-seam.cjs (whose runNode/runGit/runHook +// primitives cover exactly those three shapes and forward no `shell` option) +// — `npm` needs `shell: true` on Windows (npm.cmd), which the seam has no +// surface for. Bounding this directly with an explicit `timeout` is the +// documented alternative in eslint-rules/no-unbounded-spawn.cjs. +const NPM_LS_TIMEOUT_MS = 30000; + +function npmLs(pkg) { + // `npm ls --json --all` lists every installed copy with its version. Collect + // the version of every node whose key is `pkg` (not the parent packages). + const out = execFileSync('npm', ['ls', pkg, '--json', '--all'], { + cwd: ROOT, encoding: 'utf8', shell: true, stdio: ['ignore', 'pipe', 'ignore'], + timeout: NPM_LS_TIMEOUT_MS, + }); + const versions = []; + const walk = (node) => { + if (!node || !node.dependencies) return; + for (const [k, v] of Object.entries(node.dependencies)) { + if (k === pkg && v && v.version) versions.push(v.version); + walk(v); + } + }; + walk(JSON.parse(out)); + return versions; +} + +// GHSA-5p4m-2wfm-xmqj names only the 3.x (<3.15.1) and 4.x (<4.3.1) lines. The SAME +// weakness in the 5.x line is CVE-2026-59870 / GHSA-724g-mxrg-4qvm, fixed in 5.2.1 — +// so a guard against this bug CLASS must require 5.2.1 there too rather than waving +// every 5.x through, or an accidental major bump to 5.0.0 would reintroduce the exact +// quadratic `!!omap` resolution this test exists to prevent. +function isPatched(version) { + const core = String(version).split('+')[0]; // drop build metadata + // A prerelease of the patched version (e.g. 4.3.1-beta.1) sorts BELOW it in semver + // and may predate the fix — fail closed rather than guess. + if (core.includes('-')) return false; + const [maj, min, pat] = core.split('.').map(Number); + if (![maj, min, pat].every(Number.isInteger)) return false; // unparseable — fail closed + if (maj < 3) return true; // predates the affected lines + if (maj === 3) return min > 15 || (min === 15 && pat >= 1); // 3.x >= 3.15.1 + if (maj === 4) return min > 3 || (min === 3 && pat >= 1); // 4.x >= 4.3.1 + if (maj === 5) return min > 2 || (min === 2 && pat >= 1); // 5.x >= 5.2.1 (CVE-2026-59870) + return true; // >5.x +} + +test('all installed js-yaml copies are patched (>=4.3.1 / >=3.15.1 / >=5.2.1) — #3238', () => { + const versions = npmLs('js-yaml'); + // Vacuity guard: an empty list would make every assertion below trivially true. + assert.ok(versions.length > 0, 'js-yaml must be installed (devDependency) to guard'); + for (const v of versions) { + assert.ok(isPatched(v), + `js-yaml@${v} is not a patched version — the quadratic \`!!omap\` resolution bug is ` + + 'present in 3.x <3.15.1 (GHSA-5p4m-2wfm-xmqj), 4.x <4.3.1 (same), and 5.x <5.2.1 ' + + '(CVE-2026-59870). Re-apply: npm install js-yaml@^4.3.1'); + } +});