Audit fix fallback#14589
Conversation
robaiken
force-pushed
the
robaiken/audit-fix-fallback
branch
from
b473d72 to
31c19b0
Compare
There was a problem hiding this comment.
Pull request overview
Adds fallback behavior for npm/yarn/pnpm subdependency updates where the “normal” update command is a no-op (especially in workspace repos), by running the relevant * audit * --fix command and improving diagnostic context when no files change.
Changes:
- Add
npm audit fix/yarn npm audit --fix/pnpm audit --fixfallback paths when subdependency update commands appear to make no lockfile changes. - Add new workspace fixtures (npm8, Yarn Berry, pnpm) and specs covering the fallback behavior.
- Improve
NoChangeErrormessages and Sentry context with a detected package manager value.
Reviewed changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb | Adds audit-fix fallbacks and “no-op” detection for yarn berry, pnpm, and npm8 subdependency resolution. |
| npm_and_yarn/lib/dependabot/npm_and_yarn/native_helpers.rb | Adds helper wrappers to run npm audit fix, pnpm audit --fix, and yarn npm audit --fix. |
| npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/yarn_lockfile_updater.rb | Adds Yarn Berry subdependency fallback to yarn npm audit --fix when the update appears to be a no-op. |
| npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater.rb | Adds npm8 subdependency fallback to npm audit fix when the update appears to be a no-op. |
| npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater.rb | Improves NoChangeError message/context with detected package manager; adjusts pnpm no-change handling. |
| npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver_spec.rb | Adds specs for Yarn Berry and pnpm workspace subdependency fallback behavior. |
| npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/yarn_lockfile_updater_spec.rb | Adds spec asserting Yarn Berry audit-fix fallback when update is a no-op. |
| npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater_spec.rb | Adds spec asserting npm audit-fix fallback for workspace subdependency no-op updates. |
| npm_and_yarn/spec/dependabot/npm_and_yarn/file_updater_spec.rb | Updates NoChangeError expectation and adds tests for package manager detection. |
| npm_and_yarn/spec/fixtures/projects/yarn_berry/workspace_subdependency_update/** | New Yarn Berry workspace fixture (package.json + yarn.lock). |
| npm_and_yarn/spec/fixtures/projects/pnpm/workspace_subdependency_update/** | New pnpm workspace fixture (package.json + pnpm-workspace.yaml + pnpm-lock.yaml). |
| npm_and_yarn/spec/fixtures/projects/npm8/workspace_subdependency_update/** | New npm8 workspace fixture (package.json + package-lock.json). |
Files not reviewed (2)
- npm_and_yarn/spec/fixtures/projects/npm8/workspace_subdependency_update/package-lock.json: Language not supported
- npm_and_yarn/spec/fixtures/projects/pnpm/workspace_subdependency_update/pnpm-lock.yaml: Language not supported
|
I ran the dependabot cli with this changes against the workflow where And I got below error: And summary says: |
@robaiken provided the fix and I have tested the same and it's working as expected |
|
The standard package manager update commands ( Core mechanism
New helper methods
PR naming Pending: |
thavaahariharangit
left a comment
thavaahariharangit
left a comment
There was a problem hiding this comment.
Nice approach! The audit-fix fallback is a solid way to handle those stubborn transitive deps in workspace repos. Left a few suggestions below — mostly around reducing the duplication and a couple of edge cases worth thinking about.
Will this allow Dependabot to properly utilise |
|
@robaiken should this be marked as "Closes #13177" ?
I think you need the |
robaiken
force-pushed
the
robaiken/audit-fix-fallback
branch
from
5ff89e3 to
450210a
Compare
robaiken
force-pushed
the
robaiken/audit-fix-fallback
branch
4 times, most recently
from
b02485b to
5314633
Compare
| NativeHelpers.run_npm_audit_fix_command | ||
| sub_dependencies.each { |dep| dep.metadata[:audit_fix_used] = true } | ||
| rescue SharedHelpers::HelperSubprocessFailed | ||
| Dependabot.logger.info("npm audit fix failed or partially fixed — continuing with any changes made") |
There was a problem hiding this comment.
Does audit fix raise error even when doing some updates. In that case is it make sense to have partial fix (if it raises errror)? I am just a little confused on this.
There was a problem hiding this comment.
The goal is to trigger security fixes no matter what, sometimes they are only partially fixed but this is still better than just failing
| rescue SharedHelpers::HelperSubprocessFailed | ||
| Dependabot.logger.info( | ||
| "pnpm audit --fix failed or partially fixed — continuing with any changes made" | ||
| ) |
There was a problem hiding this comment.
Same question similar to the npm audit fix one asked above.
There was a problem hiding this comment.
this is running pnpm audit fix, pnpm have their own version
| rescue SharedHelpers::HelperSubprocessFailed | ||
| Dependabot.logger.info( | ||
| "yarn npm audit --fix failed or partially fixed — continuing with any changes made" | ||
| ) |
There was a problem hiding this comment.
Same question similar to the npm audit fix one asked above.
|
@karlhorky correct — those
Without |
|
Would be great to see that added, thank you! Assuming this lands while pnpm 10.x is around, I imagine the |
…ckfile_updater_spec.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…nt for npm and yarn
…ed_content Agent-Logs-Url: https://github.com/dependabot/dependabot-core/sessions/988c3f95-a729-413c-98ac-994924de2c00 Co-authored-by: robaiken <6567647+robaiken@users.noreply.github.com>
pnpm audit --fix adds overrides to package.json. Since run_pnpm_update and run_pnpm_updater only return lockfile content, a manifest change would produce inconsistent output. Snapshot package.json files before the fallback and revert both manifest(s) and lockfile if any change is detected.
Adds a first-tier fallback that runs pnpm update --depth Infinity <dep> (with -r --include-workspace-root for workspaces) when the regular update is a no-op. This updates transitive dependencies in the lockfile without modifying any package.json (unlike pnpm audit --fix). If --depth Infinity is also a no-op we fall through to the existing audit --fix path.
robaiken
force-pushed
the
robaiken/audit-fix-fallback
branch
from
4b777e5 to
2f5ac87
Compare
|
@robaiken @markhallen amazing that this got merged! What is the release / rollout procedure for Dependabot changes? I guess first of all it will show up as a new release on Releases? And then after some amount of time be available in regular Dependabot usage on https://github.com repos? Looking forward to giving this a shot in a lot of different repos! And then after verification, closing my issue: |
|
@karlhorky we are having some delays on rolling out this feature, I will keep you posted when we roll this out |
|
@karlhorky The feature flag as been enabled |
|
@robaiken Great! I'm not sure whether that means that this should be available for all repos on github.com now, but after using the comment
I didn't dig into the logs on the workflow run yet. |
|
👀 I just gave one of our open Dependabot vulns a re-run to see if things were better, but trying to update Running |
|
The feature flag has been rolled out to everyone now! |
|
https://github.com/alveusgg/alveusgg/actions/runs/25863233146/job/75998346176 https://github.com/alveusgg/extension/actions/runs/25863257306/job/75998430099 It works 🎉 I am incredibly grateful for you getting this out, Dependabot finally works with pnpm properly!! |
|
Hm, then again, https://github.com/nodejs/nodejs.org/pull/8894/changes + https://github.com/nodejs/nodejs.org/pull/8893/changes + https://github.com/nodejs/nodejs.org/pull/8892/changes + https://github.com/nodejs/nodejs.org/pull/8891/changes were just opened and don't have a legitimate diff on them 🤔 |
|
@robaiken thanks! I tried it out by using
See the renaming of the PR here: 
Maybe new |
|
On the other hand, this other Dependabot PR seems to do the right thing 🎉
|
What are you trying to accomplish?
What: Add an audit fix fallback for npm, yarn (berry), and pnpm when the standard subdependency update command is a no-op. When the normal update command (
npm update,yarn up -R,pnpm update) doesn't change the lockfile — common for transitive dependencies not listed in any package.json in workspace repos — Dependabot now falls back tonpm audit fix,yarn npm audit --fix, orpnpm audit --fixrespectively.Why: Dependabot currently fails to update vulnerable transitive dependencies in npm workspace repositories because the standard update commands only target packages listed in
package.json. This leaves workspace repos unable to receive Dependabot PRs for subdependency security updates (e.g.,picomatch). The audit fix commands can update these transitive dependencies directly in the lockfile. I believe that is more preferable to attempt to deliver these prs rather than erroring out like we currently do.Anything you want to highlight for special attention from reviewers?
ToolFeatureNotSupportederror previously raised for pnpm transitive dependency updates, unblocking that path entirely.enable_audit_fix_fallbackfeature flag for safe rollout.How will you know you've accomplished your goal?
With the feature flag enabled, workspace repos with vulnerable transitive dependencies will receive Dependabot PRs that update the lockfile via audit fix.
Checklist