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

[feature][nodejs] SLSA v1.0 support #2499

Open
2 tasks
ianlewis opened this issue Jul 31, 2023 · 10 comments
Open
2 tasks

[feature][nodejs] SLSA v1.0 support #2499

ianlewis opened this issue Jul 31, 2023 · 10 comments
Assignees
Labels
area:nodejs Issue related to the Node.js builder type:feature New feature or request

Comments

@ianlewis
Copy link
Member

ianlewis commented Jul 31, 2023

npm now supports SLSA v1.0 so we should support generating v1.0 by default in the builder.

  • Support for SLSA v1.0 in Node.js builder by default
  • e2e tests for Node.js builder with SLSA v1.0
@ianlewis ianlewis added type:feature New feature or request area:nodejs Issue related to the Node.js builder labels Jul 31, 2023
@ianlewis ianlewis added this to the Node.js builder GA milestone Jul 31, 2023
@ianlewis
Copy link
Member Author

ianlewis commented Aug 10, 2023

Looks like npm does some validation on the server side that isn't compatible with SLSA v1.0 provenance generated BYOB

npm ERR! code E422
npm ERR! 422 Unprocessable Entity - PUT https://registry.npmjs.org/@ianlewis%2factions-test - Error verifying sigstore provenance bundle: Invalid GitHub Actions SLSA v1 provenance predicate:
npm ERR! buildDefinition.buildType: https://github.com/slsa-framework/slsa-github-generator/delegator-generic@v0 does not match expected value for buildType: https://slsa-framework.github.io/github-actions-buildtypes/workflow/v1,
npm ERR! buildDefinition.externalParameters.workflow.ref: <empty> does not match expected value for SourceRepositoryRef: refs/tags/v0.1.160,
npm ERR! buildDefinition.externalParameters.workflow.repository: <empty> does not match expected value for SourceRepositoryURI: https://github.com/ianlewis/actions-test,
npm ERR! buildDefinition.externalParameters.workflow.path: <empty> does not match expected value for BuildConfigURI: .github/workflows/nodejs.yml,
npm ERR! buildDefinition.internalParameters.github.event_name: <empty> does not match expected value for BuildTrigger: push,
npm ERR! buildDefinition.internalParameters.github.repository_id: <empty> does not match expected value for SourceRepositoryIdentifier: 474793590,
npm ERR! buildDefinition.internalParameters.github.repository_owner_id: <empty> does not match expected value for SourceRepositoryOwnerIdentifier: 49289,
npm ERR! runDetails.builder.id: https://github.com/ianlewis/slsa-github-generator/.github/workflows/builder_nodejs_slsa3.yml@refs/heads/nodejs-slsa-v10 does not match expected value for builderId: https://github.com/actions/runner/github-hosted

https://github.com/ianlewis/actions-test/actions/runs/5817446938/job/15772122301

This might be the kind of thing that could be solved by slsa-framework/slsa#940

/cc @steiza @MarkLodato @joshuagl

@MarkLodato
Copy link
Member

There are two issues:

  1. GitHub only accepts the one buildType. Standardize externalParameters for CI/CD builds slsa#940 could help here.
  2. GitHub only accepts the one builder ID (and maybe public key identity). I'm not sure how to resolve this one.

@ianlewis
Copy link
Member Author

Indeed.

I don't see anything about the key identity so far (it's actually different from the builder ID for BYOB) so fingers crossed there.

But yes, runDetails would be out of scope of slsa-framework/slsa#940

@laurentsimon
Copy link
Collaborator

laurentsimon commented Aug 10, 2023

sharing verification code (slsa-verifier?) is also a direction we could follow. It would consolidate logic verification in one codebase to help scaling across registries. (Example: Verification of GCB provenance is supported in slsa-verifier, but it's more work for each registry to implement this themselves).

@ianlewis
Copy link
Member Author

It looks like they went with the structure we specified in the github-actions-buildtypes repo. If we update BYOB to follow that structure then the issues with internalParameters and externalParameters mostly go away.

npm ERR! buildDefinition.externalParameters.workflow.ref: <empty> does not match expected value for SourceRepositoryRef: refs/tags/v0.1.160,
npm ERR! buildDefinition.externalParameters.workflow.repository: <empty> does not match expected value for SourceRepositoryURI: https://github.com/ianlewis/actions-test,
npm ERR! buildDefinition.externalParameters.workflow.path: <empty> does not match expected value for BuildConfigURI: .github/workflows/nodejs.yml,
npm ERR! buildDefinition.internalParameters.github.event_name: <empty> does not match expected value for BuildTrigger: push,
npm ERR! buildDefinition.internalParameters.github.repository_id: <empty> does not match expected value for SourceRepositoryIdentifier: 474793590,
npm ERR! buildDefinition.internalParameters.github.repository_owner_id: <empty> does not match expected value for SourceRepositoryOwnerIdentifier: 49289,

@ianlewis
Copy link
Member Author

@steiza @feelepxyz @kommendorkapten Is there a way that we can maybe loosen the SLSA v1.0 buildDefinition.buildType and runDetails.builder.id to allow our Node.js builder to publish provenance to the official npm registry? Right now it seems to validate against the npm CLI's values.

npm ERR! buildDefinition.buildType: https://github.com/slsa-framework/slsa-github-generator/delegator-generic@v0 does not match expected value for buildType: https://slsa-framework.github.io/github-actions-buildtypes/workflow/v1,
npm ERR! runDetails.builder.id: https://github.com/ianlewis/slsa-github-generator/.github/workflows/builder_nodejs_slsa3.yml@refs/heads/nodejs-slsa-v10 does not match expected value for builderId: https://github.com/actions/runner/github-hosted

@feelepxyz
Copy link

Is there a way that we can maybe loosen the SLSA v1.0 buildDefinition.buildType and runDetails.builder.id to allow our Node.js builder to publish provenance to the official npm registry?

Yes will take a look at this 👍

@feelepxyz
Copy link

Is there a way that we can maybe loosen the SLSA v1.0 buildDefinition.buildType and runDetails.builder.id to allow our Node.js builder to publish provenance to the official npm registry?

@ianlewis this should now be fixed! Let me know if this is working as expected.

@ianlewis
Copy link
Member Author

I finally took another look at this today. Indeed the builder id error message isn't there anymore but we may still need to update our format for externalParameters and internalParameters to match what npm expects.

npm ERR! 422 Unprocessable Entity - PUT https://registry.npmjs.org/@ianlewis%2factions-test - Error verifying sigstore provenance bundle: Invalid GitHub Actions SLSA v1 provenance predicate:
npm ERR! buildDefinition.externalParameters.workflow.ref: <empty> does not match expected value for SourceRepositoryRef: refs/tags/v0.1.162,
npm ERR! buildDefinition.externalParameters.workflow.repository: <empty> does not match expected value for SourceRepositoryURI: https://github.com/ianlewis/actions-test,
npm ERR! buildDefinition.externalParameters.workflow.path: <empty> does not match expected value for BuildConfigURI: .github/workflows/nodejs.yml,
npm ERR! buildDefinition.internalParameters.github.event_name: <empty> does not match expected value for BuildTrigger: push,
npm ERR! buildDefinition.internalParameters.github.repository_id: <empty> does not match expected value for SourceRepositoryIdentifier: 474793590,
npm ERR! buildDefinition.internalParameters.github.repository_owner_id: <empty> does not match expected value for SourceRepositoryOwnerIdentifier: 49289

@ianlewis ianlewis self-assigned this Feb 12, 2024
@ianlewis
Copy link
Member Author

Probably I will need to refactor the generate-attestations action by moving some of the code into tscommon. This will allow me to create another generate-nodejs-attestations action which generates the SLSA in the way that npm expects without altering format for the rest of the BYOB workflows.

ianlewis pushed a commit that referenced this issue Jun 6, 2024
# Summary

Remove the expected GA for Node.js builder since it's stalled on SLSA
v1.0 support (#2499)

## Testing Process

N/A

## Checklist

- [x] Review the contributing
[guidelines](https://github.com/slsa-framework/slsa-github-generator/blob/main/CONTRIBUTING.md)
- [x] Add a reference to related issues in the PR description.
- [x] Update documentation if applicable.
- [x] Add unit tests if applicable.
- [x] Add changes to the
[CHANGELOG](https://github.com/slsa-framework/slsa-github-generator/blob/main/CHANGELOG.md)
if applicable.

Signed-off-by: Ian Lewis <ianlewis@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:nodejs Issue related to the Node.js builder type:feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants