Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support npmv7 (lock file v2) for transitiveRemediation #10371

Closed
rarkins opened this issue Jun 9, 2021 · 14 comments
Closed

Support npmv7 (lock file v2) for transitiveRemediation #10371

rarkins opened this issue Jun 9, 2021 · 14 comments
Labels
manager:npm package.json files (npm/yarn/pnpm) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:feature Feature (new functionality)

Comments

@rarkins
Copy link
Collaborator

rarkins commented Jun 9, 2021

What would you like Renovate to be able to do?

Support transitiveRemediation option for npm v7.

Did you already have any implementation ideas?

Unfortunately it needs some bugs in npm identified and either fixed by the npm team or a workaround found.

@rarkins rarkins added type:feature Feature (new functionality) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others manager:npm package.json files (npm/yarn/pnpm) status:blocked Issue is blocked by another issue or external requirement labels Jun 9, 2021
@HonkingGoose
Copy link
Collaborator

@rarkins Can you maybe add links/references to those bugs you need to get fixed before we can start on this feature? It's not clear to me what upstream fixes you need before this can get unblocked. 😉

@rarkins
Copy link
Collaborator Author

rarkins commented Jun 15, 2021

One of them is npm/cli#3171

I didn't have time to document any other problems I found, unfortunately.

@HonkingGoose
Copy link
Collaborator

One of them is npm/cli#3171

The author of the bug report says:

Closing this issue as it's now resolved (tested with npm 7.20.3, but I think it was already fixed since a few minor versions).

But I think you're still waiting on bugfixes for other things as well? So this is still status:blocked I think?

@rarkins
Copy link
Collaborator Author

rarkins commented Sep 24, 2021

Todo:

  • Test whether npm@7 "heals" a package-lock.json when it is incomplete

Example:

  • Find an example repo where the lock file is at least a little out of date (could benefit from lock file maintenance)
  • Find a range which can be bumped
  • Replace the existing version with a valid newer version
  • Delete the resolved file entry + hash
  • Run npm install
  • Verify if the lock file has been updated (not just the file + hash but also its transitive dependencies if relevant)

npm@6 does this, npm@7 did not when I last tried it. In other words npm@7 failed to identify invalid lock files, while npm@6 and all versions of yarn do.

@rarkins rarkins added status:requirements Full requirements are not yet known, so implementation should not be started and removed status:blocked Issue is blocked by another issue or external requirement labels Sep 24, 2021
@wrslatz
Copy link
Contributor

wrslatz commented Jan 14, 2022

👋🏻 Is there any update for this issue? With Node.js 12 approaching EOL on 2022-04-30, many projects will be updating to Node.js 14 / 16. Newer npm versions come packaged with Node.js 14 / 16, meaning more projects will be using the v2 version for lock files and lack of Renovate support for transitive remediation with lock file v2 version will impact more projects.

Edited: clarity

@rarkins
Copy link
Collaborator Author

rarkins commented Jan 14, 2022

Are you sure you understand what this issue is about? Because it's certainly no blocker for updating your npm version.

@wrslatz
Copy link
Contributor

wrslatz commented Jan 14, 2022

Are you sure you understand what this issue is about? Because it's certainly no blocker for updating your npm version.

If we update to npm v7 / v8, our lock files will update to the v2 format. Won't that mean we would lose out on transitive remediation until that support is added?

@rarkins
Copy link
Collaborator Author

rarkins commented Jan 15, 2022

Thanks for the clarification. Transitive remediation is quite an edge case, so that's why I don't consider it a blocker. Unfortunately some changes to npm>6 make it much harder to achieve.

@codepunkt
Copy link

That's interesting to hear.

Transitive remediation and de-duping of transitive dependencies are really important to us - just wanted to let you know.

@wrslatz
Copy link
Contributor

wrslatz commented Jan 15, 2022

Transitive remediation and de-duping of transitive dependencies are really important to us - just wanted to let you know.

Same here. Transitive dependencies make up the vast majority of our findings and automated fixes from Renovate.

@wrslatz
Copy link
Contributor

wrslatz commented Mar 11, 2022

Linking related issue for awareness: #3080

@rarkins
Copy link
Collaborator Author

rarkins commented Mar 12, 2022

This is now partially supported. If a transitive remediation bubbles all the way up to to cause a package.json change, then the PR will be created. But any transitive remediation which affects the package-lock.json only will not create a PR. This is still due to npm 7+ limitations we have not been able to overcome. The good news it that any such vulnerabilities would be remediated by a simple "lock file update" PR so you should enable those e.g. on a weekly basis and you should achieve essentially the same level of remediation.

@mizdra
Copy link

mizdra commented Mar 14, 2022

Workaround: Use dependabot for security updates

Official support is available now, but I'd like to introduce the workaround I use. This allows normal package updates to be opened in renovate and security updates in dependabot. This may be useful for those who want to automate the review of security updates.

# .github.dependabot.yml

# NOTE: Yaml aliases are not allowed in dependabot.yml. Therefore, there are many duplicates.

version: 2
updates:
  - package-ecosystem: "npm"
    directory: "/"
    schedule:
      # PR of security update ignores interval and opens PR at arbitrary timing,
      # but this is a required option, so I have no choice to specify it.
      interval: "daily"
    # Prevent PRs from opening except for security updates
    open-pull-requests-limit: 0
    # Add reviewers automatically
    reviewers:
      - "my-org/my-team"
      - "octocat"

  - package-ecosystem: "npm"
    directory: "/packages/pakcage-a"
    schedule:
      interval: "daily"
    open-pull-requests-limit: 0
    reviewers:
      - "my-org/my-team"
      - "octocat"

  - package-ecosystem: "npm"
    directory: "/packages/pakcage-b"
    schedule:
      interval: "daily"
    open-pull-requests-limit: 0
    reviewers:
      - "my-org/my-team"
      - "octocat"

@tvsbrent
Copy link

tvsbrent commented Jun 14, 2022

I admit that I am way out of my depth in this discussion, but I did see in that starting in version 8.6.0, npm began to "complain" about invalid lock files, ones where --force or --legacy-peer-deps had been used to create them. This thread provides more details. Is that fix in any way related to the issues previously seen in NPM 7 and that were preventing transitive dependencies from working in v2 lock files?

@renovatebot renovatebot locked and limited conversation to collaborators Oct 1, 2023
@rarkins rarkins converted this issue into discussion #24816 Oct 1, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
manager:npm package.json files (npm/yarn/pnpm) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:feature Feature (new functionality)
Projects
None yet
Development

No branches or pull requests

6 participants