Skip to content
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

Add npm dependencies with lockfile #351

Merged
merged 1 commit into from
Apr 8, 2024
Merged

Add npm dependencies with lockfile #351

merged 1 commit into from
Apr 8, 2024

Conversation

ericcornelissen
Copy link
Contributor

@ericcornelissen ericcornelissen commented Apr 5, 2024

Add pacote (at v11.1.11) as a proper npm dependency because it was being installed in CI dynamically. It is added as a devDependency because users of the package do not need it so shouldn't be included for them. Accordingly, update CI pipelines to use npm clean-install, which will install exactly what is in the lockfile, for more details see: https://docs.npmjs.com/cli/v10/commands/npm-ci.

This change hardens the supply chain. Before, even if you trust npm to be immutable and always give you the same code for pacote, it would resolve different versions of transitive dependencies (evidenced by the presence of version ranges in the dependency list for pacote in the newly added lockfile, line 1294 to 1314). Now, both direct and transitive dependencies are pinned, and their checksum can be checked at install time to ensure you're getting the same bits as you did last time.

Additionally, you could consider adding an .npmrc file with ignore-scripts=true to avoid install time attacks (it looks like neither this project nor its dependencies use any scripts).

Add pacote (at v11.1.11) as a proper npm dependency because it was being
installed in CI dynamically. It is added as a devDependency because
users of the package do not need it so shouldn't be included for them.
Accordingly, update CI pipelines to use `npm clean-install`, which will
install exactly what is in the lockfile, for more details see:
<https://docs.npmjs.com/cli/v10/commands/npm-ci>.

This change hardens the supply chain. Before, even if you trust npm to
be immutable and always give you the same code for pacote, it would
resolve different versions of transitive dependencies (evidenced by the
presence of version ranges in the dependency list for pacote in the
newly added lockfile, line 1294 to 1314).
@monperrus monperrus merged commit c7b27c5 into monperrus:master Apr 8, 2024
2 checks passed
@monperrus
Copy link
Owner

Thanks @ericcornelissen

Now we start to need renovate here, WDYT?

@ericcornelissen ericcornelissen deleted the npm-dev-deps branch April 8, 2024 08:18
@ericcornelissen
Copy link
Contributor Author

Now we start to need renovate here, WDYT?

That would make some sense @monperrus (or Dependabot). If you do decide to use a dependency maintenance bot you could consider also enabling it for your GitHub Actions (e.g. with renovate).

@monperrus
Copy link
Owner

monperrus commented Apr 9, 2024 via email

@ericcornelissen
Copy link
Contributor Author

Given the absence of a dependabot config file, I think #352 was a dependabot security update (however, that's only visible to users with the right authorization). Security updates are indeed enabled by default, but "regular" dependency maintenance is an opt-in feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants