chore(deps): bump js-yaml to 4.3.1 for GHSA-5p4m-2wfm-xmqj - #1084
Conversation
Resolves a high-severity advisory where js-yaml's `!!omap` resolution consumes quadratic CPU on a crafted document. Affects >=4.0.0 <4.3.1, and 13 dependency paths were resolving to 4.3.0. - Refresh the lockfile onto 4.3.1 and raise the spec floor to ^4.3.1 in lib/cli (runtime dep), lib/ts-sdk (dev dep), and website (runtime dep). The existing ^4.3.0 ranges already permitted 4.3.1, so no override was needed to force the upgrade. - Retarget the guard override in pnpm-workspace.yaml from the superseded `<4.3.0` range to `<4.3.1` so it covers the current advisory. - Drop the `overrides` block from website/package.json. pnpm only honors overrides at the workspace root, so it had no effect on resolution, but it pinned the vulnerable floor.
github-actions
Bot
added
cli
|
🚀 Website Preview Deployed! Preview your changes at: https://cg-pr-1084.billy-daly.workers.dev This preview will be automatically deleted when the PR is closed. |
| fast-uri@>=3.0.0 <3.1.5: '>=3.1.5 <4' | ||
| form-data@>=4.0.0 <4.0.6: '>=4.0.6' | ||
| immutable@<4.3.9: '>=4.3.9 <5' | ||
| js-yaml@>=4.0.0 <4.3.0: '>=4.3.0' |
There was a problem hiding this comment.
Should this override have an upper bound? Asking bc dependencies right above provide a cap (-- e.g. fast-uri: >=3.1.5 <4). Is the difference intentional, or should this be >=4.3.1 <5 so a breaking v5 can't sneak in later?
There was a problem hiding this comment.
This is a good question, and based on the initial research that I did, here's what I found:
- By default running
pnpm audit --fixadds the>=X.X.Xwithout the limit - Whether the limit makes a difference in terms of dependency resolution isn't totally clear. There is a v5.2.3 of
js-yamlpublished, but transitive dependencies that had^4.1.1didn't get bumped to that version. - It seems like there may be a risk of that happening if we were to completely delete and recreate the
pnpm-lock.ymlbut I was having trouble re-creating it myself though.
More importantly though, all of my testing made me realize we actually don't need the override at all though, so I removed it here: build: removes unnecessary override
I created this ticket for follow-up investigation and cleanup though
karinamzalez
left a comment
karinamzalez
left a comment
There was a problem hiding this comment.
I added some non-blocking comments!
We don't need a patch release for the SDK because js-yaml was just a dev dependency
2 participants
Summary
Resolves a high-severity advisory for
js-yamlwhich is failing other PRs.js-yamlvulnerability #1083Changes proposed
js-yamlin:@common-grants/sdk@common-grants/cli@common-grants/sdk@common-grants/cliContext for reviewers
Notes
Additional information
See this check for current failure and see the CI checks below for the new state.