-
Notifications
You must be signed in to change notification settings - Fork 49
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: address missed comments in #705 #707
Comments
Any thoughts on the first bullet? I'm not sure we can tests it in the regression part of the test. |
Yes, we certainly shouldn't have npm publish a known bad attestation to this effect. We also probably don't want to maintain a test key in the code, and acting conditionally. What about tampering during a test with a known attestation but mocking the signature verification for checking the bad digest alone? |
the code currently does not allow mocking / ignoring / passing a custom pubKey for signature verification. |
Right: maybe our best bet is to parameterise the public key used for verification. |
yes we can do that. It needs to be an internal API and should not be exposed to end-users. |
Created #709 for tracking the part on mocking sig verification. |
closes #707 Signed-off-by: laurentsimon <[email protected]>
I pressed the merge button by mistake in #705. There were unresolved comments:
https://github.com/slsa-framework/slsa-verifier/blob/main/verifiers/internal/gha/npm_test.go#L801 in unit tests, but it's missing from the regression tests. I think the main problem here is that to tests it, we need to be able to publish a package with the correct version and package name, but a different hash. I don't think it's actually possible. We don't explicitly verifi package name and version in the regression tests, we verify signature mismatch. Ideas?
@trishankatdatadog @ianlewis
The text was updated successfully, but these errors were encountered: