Skip to content

fix(deps): clear js-yaml DoS advisory via read-yaml-file override#57

Open
lukaso-bot wants to merge 1 commit into
mainfrom
fix/js-yaml-advisory-override
Open

fix(deps): clear js-yaml DoS advisory via read-yaml-file override#57
lukaso-bot wants to merge 1 commit into
mainfrom
fix/js-yaml-advisory-override

Conversation

@lukaso-bot

Copy link
Copy Markdown
Collaborator

What

pnpm audit was reporting 1 moderate — js-yaml quadratic-complexity DoS in merge-key handling (GHSA-h67p-54hq-rp68, patched ≥4.2.0).

The existing js-yaml@4: ^4.2.0 override (from 29e3498, which set out to clear exactly this) only constrains the 4.x line. The advisory was actually firing on js-yaml 3.14.2, pulled transitively:

@changesets/cli → @manypkg/get-packages@1.1.3 → read-yaml-file@1.1.0 → js-yaml 3.14.2

read-yaml-file@1.1.0 requires js-yaml ^3.x and calls the removed safeLoad API, so js-yaml can't simply be forced to 4 underneath it. js-yaml never shipped a patched 3.x — the only fix is ≥4.2.0.

Fix

One pnpm override: read-yaml-file@1 → ^2.1.0.

  • read-yaml-file@2.1.0 depends on js-yaml ^4.0.0 (pinned to 4.2.0 by the existing override) → advisory cleared.
  • It's still CJS (strip-bom@^4, not the ESM strip-bom@5 of read-yaml-file 3.x), so @manypkg/get-packages@1.1.3 can still require() it.
  • pnpm changeset status runs clean — the changesets package-discovery path (the only consumer of this chain) still works.

Verification

  • pnpm auditNo known vulnerabilities found (was 1 moderate).
  • All js-yaml instances now 4.2.0; all read-yaml-file now 2.1.0.
  • Full pre-push gate green: build, typecheck, test, a11y-contrast (chromium), biome, shellcheck, actionlint, gitleaks, publint, osv (no High/Critical).
  • Lockfile diff is a net removal (13 insertions / 47 deletions): drops the js-yaml@3.14.2 chain (argparse@1, esprima, sprintf-js, pify@4, strip-bom@3, read-yaml-file@1.1.0); adds read-yaml-file@2.1.0 + strip-bom@4.

Scope

Dev-dependency only. The published git-released package is unaffected → no changeset.

🤖 Generated with Claude Code

The `js-yaml@4: ^4.2.0` override (29e3498) only constrained the 4.x line,
but the GHSA-h67p-54hq-rp68 advisory (quadratic-complexity DoS) was firing
on **js-yaml 3.14.2**, pulled transitively by
`@changesets/cli → @manypkg/get-packages@1.1.3 → read-yaml-file@1.1.0`
(which requires js-yaml ^3.x). js-yaml has no patched 3.x release — the fix
is only ≥4.2.0 — and read-yaml-file@1.1.0 calls the removed `safeLoad` API,
so js-yaml can't simply be forced to 4 under it.

Fix: override `read-yaml-file@1 → ^2.1.0`. 2.1.0 depends on js-yaml ^4.0.0
(pinned to 4.2.0 by the existing override) and is still CJS (`strip-bom@^4`),
so `@manypkg/get-packages@1.1.3` can still `require()` it. `pnpm changeset
status` confirms the changesets package-discovery path still works.

`pnpm audit` is now clean (was: 1 moderate). Lockfile diff is a net removal:
drops the js-yaml@3.14.2 chain (argparse@1, esprima, sprintf-js, pify@4,
strip-bom@3), adds read-yaml-file@2.1.0 + strip-bom@4. Dev-dep only; the
published `git-released` package is unaffected (no changeset).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@lukaso-bot lukaso-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In-house adversarial review (Copilot quota-blocked → no automated review landed)

Reviewed the full diff and independently re-ran the one risk the PR's CI does not cover.

Coverage gap I checked: the 8 PR checks (a11y, osv, gitleaks, shell/workflow lint, test matrix) never invoke the changesets package-discovery path — changesets/action only runs in release.yml on push to main. Swapping read-yaml-file out from under @manypkg/get-packages@1.1.3 is exactly the kind of thing that passes CI and then breaks the Version-Packages PR after merge. So I re-ran it in a clean worktree off this branch (pnpm install --frozen-lockfile):

  • pnpm changeset statusexit 0. This drives @manypkg/get-packagesread-yaml-file on pnpm-workspace.yaml; it loads all workspace packages cleanly, so the CJS require() of read-yaml-file@2.1.0 works under @manypkg/get-packages@1.1.3.
  • pnpm auditNo known vulnerabilities found (was 1 moderate).
  • pnpm why confirms every read-yaml-file resolves to 2.1.0 and every js-yaml to 4.2.0 — no js-yaml@3.x remains.

Diff correctness:

  • Override targeting is right: read-yaml-file@1 is a versioned override, so it only rewrites requesters in the 1.x range (@manypkg/get-packages@1.1.3 asks for ^1.1.0) and leaves any 2.x+ consumer untouched. No collateral.
  • read-yaml-file@2.1.0 deps are js-yaml ^4.0.0 (pinned to 4.2.0 by the existing override) + strip-bom@4 (still CJS) — no ESM trap from the 3.x line.
  • Lockfile change is a net removal (drops the js-yaml@3.14.2 chain: argparse@1, esprima, sprintf-js, pify@4, strip-bom@3); no incidental version bumps.
  • Dev-dependency only; published git-released unaffected → no changeset needed (correct).

Verdict: no blocking issues; the residual not-covered-by-CI risk is verified clear. Mergeable. (Posted as a comment, not an Approve — GitHub won't let the PR author self-approve.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant