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

feat: add verifyAttestations to registry.manifest #259

Merged
merged 2 commits into from
Feb 13, 2023
Merged

feat: add verifyAttestations to registry.manifest #259

merged 2 commits into from
Feb 13, 2023

Conversation

feelepxyz
Copy link
Contributor

Add support for verifying sigstore attestations when fetching the registry.manifest. This will be ased in CLI as part of audit signatures.

Note: not the prettiest codes here yet as I've focused on just trying to cover all the corner cases and have tests to match.

RFC: npm/rfcs#626

Signed-off-by: Philip Harrison [email protected]

README.md Show resolved Hide resolved
@feelepxyz feelepxyz marked this pull request as ready for review February 8, 2023 19:45
@feelepxyz feelepxyz requested a review from a team as a code owner February 8, 2023 19:45
@feelepxyz feelepxyz requested review from wraithgar and removed request for a team February 8, 2023 19:45
@feelepxyz feelepxyz changed the title Add: verifyAttestations to registry.manifest add: verifyAttestations to registry.manifest Feb 8, 2023
@feelepxyz feelepxyz changed the title add: verifyAttestations to registry.manifest feat: add verifyAttestations to registry.manifest Feb 8, 2023
test/registry.js Outdated Show resolved Hide resolved
Copy link
Contributor

@bdehamer bdehamer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor style things and a suggestion for how to hit 100% coverage -- otherwise, looks good to me. Nice work!

test/registry.js Outdated Show resolved Hide resolved
lib/registry.js Outdated Show resolved Hide resolved
lib/registry.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Adding sigstore-js for `audit signatures`.
Related: #259

Signed-off-by: Philip Harrison <[email protected]>
@feelepxyz feelepxyz requested a review from wraithgar February 10, 2023 10:44
Add support for verifying sigstore attestations when fetching the registry.manifest.
This will be ased in CLI as part of `audit signatures`.

RFC: npm/rfcs#626

Signed-off-by: Philip Harrison <[email protected]>
Co-authored-by: Brian DeHamer <[email protected]>
Signed-off-by: Philip Harrison <[email protected]>
@wraithgar wraithgar merged commit 2916b72 into npm:main Feb 13, 2023
wraithgar pushed a commit that referenced this pull request Feb 13, 2023
Adding sigstore-js for `audit signatures`.
Related: #259

Signed-off-by: Philip Harrison <[email protected]>
@github-actions github-actions bot mentioned this pull request Feb 13, 2023
wraithgar pushed a commit to npm/cli that referenced this pull request Feb 14, 2023
Update `audit signatures` to also verify Sigstore attestations.

Additional changes:
- Adding error message to json error output as there are a lot of different failure cases with signature verification that would be hard to debug without this
- Adding predicateType to json error output for attestations to diffentiate between provenance and publish attestations

References:
- Pacote changes: npm/pacote#259
- RFC: npm/rfcs#626

Signed-off-by: Philip Harrison <[email protected]>
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.

3 participants