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

[BUG] Attestations incorrectly reported as invalid for npm < 10.5.0 #7279

Closed
2 tasks done
davidlj95 opened this issue Mar 11, 2024 · 16 comments
Closed
2 tasks done

[BUG] Attestations incorrectly reported as invalid for npm < 10.5.0 #7279

davidlj95 opened this issue Mar 11, 2024 · 16 comments
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x

Comments

@davidlj95
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Recently npm audit signatures started failing in a project of mine. Error is about packages having invalid attestations. Specifically

  • @semantic-release/npm@11.0.3
  • ts-api-utils@1.3.0

Full log: https://gist.github.com/davidlj95/1c2752b4f59dda2527cfc27862350af0

As you can see, was using npm version 10.2.4.

When using 10.5.0, error was gone. Versions 10.3.0 and 10.4.0 are affected too after a quick test.

Expected Behavior

Attestations keep working for recent versions of npm

For instance, GitHub Hosted Runner Ubuntu 220.4 (latest) uses version 10.2.4

Steps To Reproduce

  1. Using npm with version from 10.2.4 (or maybe older) < 10.5.0
  2. Install one of the packages listed above
  3. Run npm audit signatures
  4. Command fails with error
audited 499 packages in 2s

497 packages have verified registry signatures

56 packages have verified attestations

2 packages have invalid attestations:

@semantic-release/npm@11.0.3 (https://registry.npmjs.org/)
ts-api-utils@1.3.0 (https://registry.npmjs.org/)

Someone might have tampered with these packages since they were published on the registry!

Environment

  • npm: 10.2.4 (reproduces til 10.4.0)
  • Node.js: v20.11.1
  • OS Name: macOs Sonoma 14.4
  • System Model Name: Apple MacBook Pro
  • npm config:
; "user" config from /Users/davidlj95/.npmrc

//registry.npmjs.org/:_authToken = (protected)

; node bin location = /Users/davidlj95/.n/bin/node
; node version = v20.11.1
; npm local prefix = /Users/davidlj95/Code/tmp/invalid-attestations
; npm version = 10.2.4
; cwd = /Users/davidlj95/Code/tmp/invalid-attestations
; HOME = /Users/davidlj95
; Run `npm config ls -l` to show all defaults.
@davidlj95
Copy link
Contributor Author

Whoops, marked as closed by mistake :P

@TaylorHo
Copy link

TaylorHo commented Mar 13, 2024

Same error here, using npm v10.2.4 and Node.js v20.11.1.

@wraithgar
Copy link
Member

This is due to sigstore/rekor#1888. The timestamp was removed from the checkpoint. npm was patched to account for this but older versions will not be able to properly verify attestations. Please remember to update npm even beyond the version that ships with node.js, as it has often gotten updates since whenever that version of node.js was built.

@yoadfe
Copy link

yoadfe commented Mar 16, 2024

@wraithgar Which was the first npm version that got patched for this? Couldn't find anything in the changelog.

@mitar
Copy link

mitar commented Mar 17, 2024

More reports of the issue: microsoft/playwright#29798

@mitar
Copy link

mitar commented Mar 17, 2024

Please remember to update npm even beyond the version that ships with node.js, as it has often gotten updates since whenever that version of node.js was built.

Sorry, but this is failing with LTS node.js. I am using node LTS Docker image and I would expect that things keep working, not that at some point before LTS end of life, existing Docker builds starts failing.

@ljharb
Copy link
Contributor

ljharb commented Mar 18, 2024

That expectation holds for node, but is incorrect for a remote service node/npm connects to.

@mitar
Copy link

mitar commented Mar 18, 2024

That expectation holds for node, but is incorrect for a remote service node/npm connects to.

Suuureee, but it could be backported to an older version of npm, so that things continue to work, if there is an external dependency like this?

@ljharb
Copy link
Contributor

ljharb commented Mar 18, 2024

The usual path is that node updates the npm version it includes, but you can do that yourself so you don’t have to wait for it.

@mitar
Copy link

mitar commented Mar 18, 2024

So the current LTS will eventually get npm upgraded to >= 10.5.0? The latest npm with node 20 is 10.2.4.

@wraithgar
Copy link
Member

That's up to the Node.js folks. We're working on updating the docs for this feature to remind folks to update their own version of npm manually. This is a pretty new feature and using it is going to require folks to change how they think about "staying current". It may mean updating beyond whatever version of npm ships with a given version of Node.js.

@mitar
Copy link

mitar commented Mar 18, 2024

@wraithgar Thanks! It would be great if then also Docker image rules would be changed to include npm updates. Currently they state that they bundle only the npm bundled with node. This is a problem because it means that a) everyone has to figure out that npm updating is necessary in the first place b) it increases CI time for everyone instead of image having the latest reasonable version bundled in.

@ljharb
Copy link
Contributor

ljharb commented Mar 18, 2024

I'm not sure why the docker images would need to be any different than node itself - and yes, everyone has to learn that npm updating is necessary.

You can always make your own base docker image that upgrades npm itself, and avoid CI time increases.

@mitar
Copy link

mitar commented Mar 18, 2024

You can always make your own base docker image that upgrades npm itself, and avoid CI time increases.

Of course. But then everyone has to do it. Not the best for the ecosystem.

@davidlj95
Copy link
Contributor Author

fwiw docs were updated to clarify this:

#7304
npm/documentation#1010

thx peeps!

mitar added a commit to peer/db that referenced this issue Mar 21, 2024
Tbhesswebber added a commit to Tbhesswebber/eslint-config that referenced this issue Mar 26, 2024
As explained in [this issue](npm/cli#7279),
it's important to keep NPM set to the latest if you're verifying
signatures to ensure that you're using the latest signature logic.
domwebber added a commit to RelucentDev/prettier-config that referenced this issue Mar 27, 2024
Based on npm/cli#7279 bug rather than a project-related bug
hangy added a commit to TempMaiSe/TempMaiSe that referenced this issue Apr 3, 2024
Fixes attestation issue, see npm/cli#7279
@davidlj95
Copy link
Contributor Author

For whoever may be interested, with latest releases of GitHub Actions GitHub-hosted runners today (like ubuntu-latest one), npm version bundled is 10.5.0. So no need anymore to force an npm update in order to properly audit signatures

mitar added a commit to charon/charon that referenced this issue May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x
Projects
None yet
Development

No branches or pull requests

6 participants