search

dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.63k stars 993 forks source link

Lockfile not updated for NPM workspaces using v3 lockfiles #6270

Closed Mr0grog closed 1 year ago

Mr0grog commented 1 year ago

Is there an existing issue for this?

Package ecosystem

npm

Package manager version

9.x

Language version

Node.js 18.x

Manifest location and content before the Dependabot update

/package.json, /package-lock.json, /package-a/package.json, /package-b/package.json

dependabot.yml content

version: 2
updates:
  - package-ecosystem: "npm"
    directory: "/"
    schedule:
      interval: "weekly"
    versioning-strategy: increase
    open-pull-requests-limit: 10

Updated dependency

No response

What you expected to see, versus what you actually saw

I recently updated a repo from having multiple separate packages to having multiple package joined under an NPM workspace (that is, there’s a package.json in the root directory with a workspaces property that points to the subdirectories with actual packages that have code and dependencies). I also happened to update the package-lock.json file from lockfile version 2 (NPM 8 and earlier) to version 3 (NPM 9).

When dependabot creates PRs against the new repository, it no longer updates the package-lock.json file in the root directory. However, it does so just fine when the lockfile uses the version 2 format.

For a live example, see these two repos, which are identical except for what lockfile format they use:

This all seems to work fine with version 3 lockfiles in non-workspaces repositories. For example: https://github.com/Mr0grog/test-dependabot-npm-lockfile-v3

Native package manager behavior

No response

Images of the diff or a link to the PR, issue, or logs

For a live example, see these two repos, which are identical except for what lockfile format they use:

This all seems to work fine with version 3 lockfiles in non-workspaces repositories. For example: https://github.com/Mr0grog/test-dependabot-npm-lockfile-v3

Smallest manifest that reproduces the issue

No response

jeffwidman commented 1 year ago

Thanks for taking the time to put together a very detailed issue including the test repos. I'm less familiar with npm ecosystem myself, so will try to get a colleague to take a look.

colinrotherham commented 1 year ago

We've seen this issue recently, appears to have been since:

It's our npm workspace package.json files that receive Dependabot updates (but not package-lock.json)

package.json

"workspaces": [
  "app",
  "docs/examples/*",
  "package"
],

Our GitHub Actions workflow then fails on npm ci

npm ERR! code EUSAGE
npm ERR! 
npm ERR! `npm ci` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.
npm ERR! 
npm ERR! Invalid: lock file's sass@1.57.0 does not satisfy sass@1.57.1
npm ERR! 
npm ERR! Clean install a project
Mr0grog commented 1 year ago

@colinrotherham yeah, that is basically how I first experienced this as well.

FWIW, you can work around the issue by configuring NPM to always use v2 lock files with an .npmrc file in your project (that way you don't need to manually correct every PR): https://github.com/usdigitalresponse/univaf/commit/99e21cece6673ed8c97ced820ba62b3d7ee1371b

colinrotherham commented 1 year ago

@Mr0grog @jeffwidman Soon as we merged the downgrade to ~"lockfileVersion": 3~ "lockfileVersion": 2 we've had multiple Dependabot security update PRs come through our repositories:

Which may indicate that Dependabot alerts don't work with "lockfileVersion": 3 either?

deivid-rodriguez commented 1 year ago

The core update logic should be quite similar for both security and version updates, so it makes sense that the issue is present for both. Just to clarify, I assume you do get Dependabot alerts while using v3 lockfiles, it's just security updates to fix those that don't work, right?

colinrotherham commented 1 year ago

@deivid-rodriguez Yeah Dependabot alerts continued but for our time on "lockfileVersion": 3 they show:

The currently installed version can't be determined.

To resolve the issue add a supported lockfile (package-lock.json or yarn.lock).

Dependabot version update PRs also continued for both top-level and npm workspace packages but noticed that PRs for workspace-only version updates missed package-lock.json changes when on "lockfileVersion": 3

deivid-rodriguez commented 1 year ago

🤔 Can you show a screen capture of that error? Did it happen when trying to create security updates manually, or was it a fixed banner on the alert? Just trying to figure out if this is specific to dependabot-core or some other part of GitHub also has a similar issue.

colinrotherham commented 1 year ago

@deivid-rodriguez Not a problem. It's an alert for jquery which we use for tests and isn't shipped.

The Dependabot alert was opened automatically as soon as we upgraded to "lockfileVersion": 3 in https://github.com/alphagov/govuk-frontend/compare/6d41719109...97ef82d8e9 where jquery is installed in an npm workspace app/package.json

Screenshot shows an alert box with "Try again" and "View logs" buttons. Presume this a fixed banner?

Cross-Site Scripting (XSS) in jquery

View logs

  proxy | time="2023-01-18T09:34:50Z" level=info msg="proxy starting" commit=0bb2c3212d8d39c57e071ea37f15752221cc1316
  proxy | 2023/01/18 09:34:50 Listening (:1080)
updater | 2023-01-18T09:34:50.337752321 [581190403:main:WARN:src/devices/src/legacy/serial.rs:222] Detached the serial input due to peer close/error.
updater | time="2023-01-18T09:34:52Z" level=info msg="guest starting" commit=59daee434b65804d3668d6363c6ec47eb09ecbfc
updater | time="2023-01-18T09:34:52Z" level=info msg="starting job..." fetcher_timeout=10m0s job_id=581190403 updater_timeout=45m0s updater_version=f26817c7eb79601996da80a3fd0d4ab6a936f6da
updater | I, [2023-01-18T09:34:54.564418 #7]  INFO -- sentry: ** [Raven] Raven 3.1.2 ready to catch errors
updater | INFO <job_581190403> Starting job processing
  proxy | 2023/01/18 09:34:58 [002] GET https://github.com:443/alphagov/govuk-frontend/info/refs?service=git-upload-pack
  proxy | 2023/01/18 09:34:58 [002] * authenticating git server request (host: github.com)
  proxy | 2023/01/18 09:34:58 [002] 200 https://github.com:443/alphagov/govuk-frontend/info/refs?service=git-upload-pack
  proxy | 2023/01/18 09:34:58 [004] POST https://github.com:443/alphagov/govuk-frontend/git-upload-pack
  proxy | 2023/01/18 09:34:58 [004] * authenticating git server request (host: github.com)
  proxy | 2023/01/18 09:34:58 [004] 200 https://github.com:443/alphagov/govuk-frontend/git-upload-pack
  proxy | 2023/01/18 09:34:58 [006] POST https://github.com:443/alphagov/govuk-frontend/git-upload-pack
  proxy | 2023/01/18 09:34:58 [006] * authenticating git server request (host: github.com)
  proxy | 2023/01/18 09:34:58 [006] 200 https://github.com:443/alphagov/govuk-frontend/git-upload-pack
updater | INFO <job_581190403> Finished job processing
updater | time="2023-01-18T09:34:59Z" level=info msg="task complete" container_id=job-581190403-file-fetcher exit_code=0 job_id=581190403 step=fetcher
updater | I, [2023-01-18T09:35:01.381403 #7]  INFO -- sentry: ** [Raven] Raven 3.1.2 ready to catch errors
updater | INFO <job_581190403> Starting job processing
updater | INFO <job_581190403> Starting update job for alphagov/govuk-frontend
updater | INFO <job_581190403> Checking if jquery  needs updating
  proxy | 2023/01/18 09:35:04 [012] GET https://registry.npmjs.org:443/jquery
  proxy | 2023/01/18 09:35:04 [012] 200 https://registry.npmjs.org:443/jquery
  proxy | 2023/01/18 09:35:04 [014] GET https://registry.npmjs.org:443/jquery/3.6.3
  proxy | 2023/01/18 09:35:07 [014] 200 https://registry.npmjs.org:443/jquery/3.6.3
updater | INFO <job_581190403> Latest version is 3.6.3
updater | INFO <job_581190403> Dependabot can't update vulnerable dependencies for projects without a lockfile or pinned version requirement as the currently installed version of jquery isn't known.
updater | INFO <job_581190403> Finished job processing
updater | INFO Results:
updater | Dependabot encountered '1' error(s) during execution, please check the logs for more details.
updater | time="2023-01-18T09:35:08Z" level=info msg="task complete" container_id=job-581190403-updater exit_code=0 job_id=581190403 step=updater

We'd previously had the same alert on "lockfileVersion": 2 but on package-lock.json not app/package.json

deivid-rodriguez commented 1 year ago

Yeah, that's what I wanted to know, that's the banner that shows up when dependabot fails to create a PR 👍. I will have a look at this.

daniel-marmelshtein-healthy commented 1 year ago

Hey @Mr0grog, Did the problem been fixed? I am having the same issue as @colinrotherham When I am looking in the main branch of this repo I see that the deployment been failed.

Screen Shot 2023-02-28 at 17 34 34

Mr0grog commented 1 year ago

@daniel-marmelshtein-healthy Yep, it's working fine for me in that repo. The failure you're seeing is expected and isn't related to the lockfile (the deploy step in that workflow errors and stops if there's another future commit in the pipeline waiting to build & deploy, that way we aren't rapidly and pointlessly restarting servers).

avivek commented 1 year ago

Hi @deivid-rodriguez @Mr0grog ,

We are using github security alerts and github dependency graph and whenever the lock file version is 3 we see that there are no dependency detected from the package-lock.json in the dependency graphs and hence the security alerts do not show for any packages in the package-lock.json.

My understanding is that this issue is supposed to address the parsing of lock file version 3 of npm. So is this fix deployed to Github cloud org's??

jeffwidman commented 1 year ago

@avivek This repo is about the code that creates PR's to bump versions--whether from cron or from security alerts--but not about the security alerts themselves. Versus your problem is with parsing the manifests to generate security alerts. For various reasons, that's handled by a separate team/codepath.

I suggest starting a discussion over in https://github.com/community/community/discussions/categories/code-security as that's probably the best way to reach the team that handles Dependency Graphs / alert generation. Feel free to drop a breadcrumb link on this issue to the discussion.

avivek commented 1 year ago

@jeffwidman here is the link to the discussion: https://github.com/community/community/discussions/49759

ctsstc commented 1 year ago

I'm still getting the following error which I believe is due to our submodules:

npm ERR! `npm ci` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.

The lockfile that dependabot generates is missing the modules from the submodules it seems.

I verified that npm ci does not have issues on main. If I pull down the branch that Dependabot creates and run npm install it results in changes to the lock file with the missing dependencies of the submodules it seems.