feat(investigate): add yarn Berry remediation path (#122) - #123
Conversation
npm and yarn share Dependabot's npm_and_yarn ecosystem; until now the bump path was npm-only (npm audit fix + package-lock.json), so yarn repos (e.g. mozilla/fxa) got no remediation. Route by lockfile: detect_js_package_manager() picks npm/yarn/pnpm, and an npm alert on a yarn.lock repo now takes a yarn_bump path that runs 'yarn up -R <pkg>@npm:^<version>' (Berry v2+) and commits yarn.lock via the existing verified-commit + PR helpers (scripts/yarn-fix.sh, yarn-bump.sh), mirroring the npm and pip paths. MVP/handoff (see PR): Berry-only (classic v1 skipped), audit verification is best-effort, PnP files not committed. Not yet tested end-to-end locally.
Berry v4 rejects a version/range with -R ('Ranges aren't allowed when
using --recursive') — it takes the package name only and re-resolves each
occurrence to the newest version satisfying its existing range. Validated
live against mozilla/fxa (yarn@4.9.2): one run bumped every vulnerable
brace-expansion line to its patched in-range version, yarn.lock-only,
node_modules untouched. Gate 'fixed' on yarn.lock actually changing; an
unchanged lock means the patch is out-of-range and needs a resolutions
override (still #122).
d766261 to
a027f25
Compare
| if pkg_manager == "yarn": | ||
| print(" yarn ecosystem — deferring to yarn_bump workflow step.") | ||
| action = "yarn_bump" | ||
| write_output("yarn_package", package_name) | ||
| write_output("yarn_version", patched_version) | ||
| write_output("alert_number", str(alert_number)) | ||
| elif pkg_manager == "npm": | ||
| print(" npm ecosystem — deferring to npm_bump workflow step.") | ||
| action = "npm_bump" | ||
| write_output("npm_package", package_name) | ||
| write_output("npm_version", patched_version) | ||
| write_output("alert_number", str(alert_number)) |
There was a problem hiding this comment.
quibble (non-blocking): These code blocks duplicate a lot of code that could just use f"{pkg_manager}" string interpolation instead.
There was a problem hiding this comment.
Done in 9e3f667 — collapsed to f"{pkg_manager}" interpolation.
There was a problem hiding this comment.
suggestion (non-blocking): Do a quick check with an LLM to see if any of this shell code could be put into the pr-lib.sh script so it can be re-used across scripts.
I find that LLMs don't seem to have seen a lot of shell script code that uses library script files, so they tend to duplicate code more often in shell scripts than other languages.
AND/OR - in this repo when I've noticed a shell script or scripts growing more advanced or sophisticated, I've also considered if it's time to convert them from shell script to python script.
There was a problem hiding this comment.
Done in a390c05 — extracted shared commit/branch/PR flow into open_bump_pr LOCKFILE in pr-lib.sh; npm-bump.sh and yarn-bump.sh now source it and pass their lockfile.
There was a problem hiding this comment.
suggestion (non-blocking): Do a quick check with an LLM to see if any of this shell code could be put into the pr-lib.sh script so it can be re-used across scripts.
I find that LLMs don't seem to have seen a lot of shell script code that uses library script files, so they tend to duplicate code more often in shell scripts than other languages.
AND/OR - in this repo when I've noticed a shell script or scripts growing more advanced or sophisticated, I've also considered if it's time to convert them from shell script to python script.
| ({"package-lock.json"}, "npm"), | ||
| ({"yarn.lock"}, "yarn"), | ||
| ({"pnpm-lock.yaml"}, "pnpm"), | ||
| ({"package-lock.json", "yarn.lock"}, "npm"), # npm wins when both exist |
There was a problem hiding this comment.
question (non-blocking): Why does npm win when both exist?
There was a problem hiding this comment.
Arbitrary, but also, npm is the more popular of the two, so maybe a better fallback 🤷
| echo "yarn.lock updated for ${YARN_PACKAGE}." | ||
| out fixed true |
There was a problem hiding this comment.
change (blocking): fixed=true here just means "yarn.lock changed".
The npm path in investigate-security-alert.yml runs a follow-up npm audit command to see if there are still remaining vulnerabilities after the npm audit fix.
This script should re-check the output of something like yarn npm audit to verify there are no remaining vulns.
There was a problem hiding this comment.
Done in 9e3f667 — now re-runs yarn npm audit and requires the package to be absent before fixed=true. Validated live on fxa: yarn up -R brace-expansion cleared the moderate advisory but left a high one (patched version out of range), so this correctly reports fixed=false instead of opening a PR that didn't fully fix it. I think the out-of-range case could be fixed by the resolutions-override issue #122 .
Per review: fixed=true previously meant only 'yarn.lock changed', which could open a PR while a vuln remained. Now re-run yarn npm audit and require the package to be absent, mirroring the npm path's post-fix audit. Validated live on fxa: 'yarn up -R brace-expansion' cleared the moderate advisory but left a high one (out-of-range), so this correctly reports fixed=false. Also DRY the npm/yarn routing in post_alert_action via pkg_manager interpolation (review quibble).
npm-bump.sh and yarn-bump.sh differed only in lockfile name and PR wording. Move the shared commit/branch/PR flow into pr-lib.sh's open_bump_pr LOCKFILE; both scripts now just source pr-lib and call it. Addresses review feedback on shell duplication.
2 participants
Addresses #122.
Adds a yarn remediation path so npm-only remediation stops skipping yarn repos (e.g. fxa). Routes by lockfile (
detect_js_package_manager); a yarn repo runsyarn up -R <pkg> --mode=update-lockfileand opens a PR via the existing helpers, mirroring the npm/pip paths.Validated live on fxa (yarn@4.9.2): one run cleared all transitive
brace-expansionalerts, yarn.lock-only. See the PR comment for details.Remaining (handoff):
resolutionsfallback when a patch is out of range, workspaces conflicts, classic v1, and end-to-end test ofyarn-bump.sh.