npm_and_yarn: add --no-save to pnpm deep-update fallback#15105
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates PNPM deep-update behavior to avoid unintended manifest/specifier mutations by adding --no-save, and aligns the corresponding spec expectations.
Changes:
- Append
--no-savetopnpm update ... --lockfile-onlyin the deep update helper. - Update the lockfile updater spec to expect the new flag and fingerprint.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| npm_and_yarn/lib/dependabot/npm_and_yarn/native_helpers.rb | Adds --no-save to the pnpm deep update command and documents why it’s required. |
| npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/pnpm_lockfile_updater_spec.rb | Updates test expectations to match the new pnpm command line and fingerprint. |
| "#{flags}update #{dependency_name} --depth Infinity --lockfile-only --no-save", | ||
| fingerprint: "#{flags}update <dependency_name> --depth Infinity --lockfile-only --no-save" |
There was a problem hiding this comment.
--no-save is already used by dependabot-core on the primary pnpm update path in three places:
file_updater/pnpm_lockfile_updater.rb:192(run_pnpm_update_packages)update_checker/subdependency_version_resolver.rb:256(pnpm_update_commandwhen a target version is known)
All ship the same --no-save flag. The flag is supported on pnpm 7-10 (the versions the npm_and_yarn ecosystem documents support for); this PR brings the deep-update fallback in line with what the primary path already does, no new minimum-version requirement is introduced.
a2b7318 to
b2c7e41
Compare
pnpm update <dep> --depth Infinity --lockfile-only rewrites caret ranges in package.json and the matching specifier: lines in pnpm-lock.yaml to ^<currently-resolved-version> for every direct dep whose resolved version is newer than its declared range floor. Dependabot returns only the lockfile from this flow, so the package.json mutations are discarded while the lockfile keeps specifier: entries that no longer match the manifest, and a downstream frozen-lockfile install rejects the PR. Adding --no-save tells pnpm to leave package.json ranges alone while still resolving and writing the lockfile graph for the target transitive dependency. Verified locally to: still bump the target transitive in the lockfile, produce zero specifier: rewrites, produce zero package.json mutations. Fixes dependabot#15104
b2c7e41 to
222a7c9
Compare
What are you trying to accomplish?
Fix a manifest/lockfile mismatch produced by the deep-update fallback during pnpm security updates.
pnpm update <dep> --depth Infinity --lockfile-onlyrewrites caret ranges inpackage.jsonand the matchingspecifier:lines inpnpm-lock.yamlto^<currently-resolved-version>for every direct dep whose resolved version is newer than its declared floor. Dependabot only returns the lockfile from this flow, so the package.json mutations are dropped while the lockfile keepsspecifier:entries that no longer match the manifest. CI's frozen-lockfile install rejects the PR.The comment above
run_pnpm_deep_update_commandclaims this command does not modifypackage.json. That has not been true on the pnpm versions Dependabot ships against (9.0.5via corepack,10.16.0global).Fix: add
--no-saveto the deep-update command.--no-savetells pnpm to leavepackage.jsonranges alone while still resolving and writing the lockfile graph for the target transitive.Fixes: #15104
Anything you want to highlight for special attention from reviewers?
--no-savedoes not block the legitimate transitive bump: against a manifest wherews@8.18.3was vulnerable andws@8.20.1was the patched version,pnpm update ws --depth Infinity --lockfile-only --no-savestill movedwsto8.20.1everywhere the parent ranges allowed.pnpm_lockfile_updater_spec.rb:775-776) asserts the exact command string, updated to match the new flag.--no-saveis already in use on three of dependabot-core's other pnpm command paths (file_updater/pnpm_lockfile_updater.rb:192inrun_pnpm_update_packages, andupdate_checker/subdependency_version_resolver.rb:256inpnpm_update_command). This PR brings the deep-update fallback in line with the primary path, no new minimum-version requirement is introduced.pnpm audit --fixwriting overrides into version-update PRs) and Gate npm_and_yarn audit-fix fallbacks to security updates only #15051 (gates audit-fix fallback to security updates only). Gate npm_and_yarn audit-fix fallbacks to security updates only #15051 does not fix this bug because affected PRs are themselves security updates.How will you know you've accomplished your goal?
Reproducer: https://github.com/ivnnv/dependabot-pnpm-deep-update-bug
Checklist
Run via this PR's CI — the call-site spec at
pnpm_lockfile_updater_spec.rb:775-776already asserts the exactpnpm update --depth Infinitycommand string and is updated in this PR to match the new flag. Lint is green; e2e/integration/updater suites are still in progress at the time of writing and will be monitored.Verified against the public reproducer at https://github.com/ivnnv/dependabot-pnpm-deep-update-bug (before/after
--no-save: 10 → 0specifier:rewrites, 18 → 0 lines ofpackage.jsonmutations). Also verified against a real workspace with a vulnerable transitivews@8.18.3that--no-savestill bumpswsto8.20.1wherever parent ranges allow.The previous comment above
run_pnpm_deep_update_commandclaimed the command did not modifypackage.json; that claim was wrong on the pnpm versions Dependabot ships against. The replacement comment spells out exactly when and why--no-saveis required, so the next reader does not have to rediscover this.