tests: add unit tests for sigstore verifier and signer logic#1299
tests: add unit tests for sigstore verifier and signer logic#1299pushkarscripts wants to merge 4 commits into
Conversation
Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
patzielinski
left a comment
There was a problem hiding this comment.
I'm not sure if this meaningfully tests our Sigstore workflow. The main concern I have is that we don't test any kind of signing or verification, which is done for other methods, e.g. GPG: https://github.com/gittuf/gittuf/blob/main/internal/signerverifier/gpg/gpg_test.go.
This is what I was referring to in the issue about using Sigstore's package to possibly mock the API or just spin up a Rekor server here.
005ae0f to
8f99c8f
Compare
|
I’ve updated the patch to move a bit closer to the signing and verification workflow by exercising I’ll spend some time next looking into mocking Sigstore interactions or using a local Rekor instance so the full signing and verification flow can be tested more realistically. |
Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
8f99c8f to
5503413
Compare
|
@patzielinski @adityasaky just checking in on this PR. Would appreciate any feedback. Thanks! |
|
|
||
| // 1. Test Verification with externally signed artifact | ||
| t.Run("Verify externally signed artifact", func(t *testing.T) { | ||
| entity, err := virtualSigstore.Sign(identity, issuer, data) |
There was a problem hiding this comment.
Instead of calling virtualSigstore.Sign, call gittuf's Signer.Sign. This should also fix the extension issue and let you actually be able to verify.
There was a problem hiding this comment.
Instead of calling virtualSigstore.Sign, call gittuf's Signer.Sign. This should also fix the extension issue and let you actually be able to verify.
I've been looking into this but got a bit stuck.
VirtualSigstore implements root.TrustedMaterial, but I don't see a way to use it directly with Signer.Sign() since it doesn't implement sign.CertificateProvider or sign.Transparency.
Am I missing something, or should I add test-only wrappers for those interfaces?
9be5977 to
25c1124
Compare
Remove the unused IDTokenGetter dependency injection plumbing, document that injected Fulcio and Rekor implementations take precedence over configured URLs, and drop the redundant mock-based signer workflow test. Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
25c1124 to
e8f468e
Compare
|
Thanks @wlynch for the review! I've addressed the comments about the unused ID token injection support and added documentation clarifying that the injected Fulcio/Rekor implementations take precedence over the configured URLs. I also removed the redundant signer mock test. I'm still working through the remaining feedback about using gittuf's |
Description
Related to #1259
Adds unit tests to improve coverage for Sigstore verifier and signer logic in
internal/signerverifier/sigstore.The tests cover:
KeyID,ExpectedExtensionKind,SetExtension)NewVerifierFromIdentityAndIssuer(default, with options, and edge cases)KeyIDMetadataKey)SignandVerifyThe tests now attempt to move closer to the actual signing and verification workflow. Since Sigstore depends on external services (e.g., Rekor/Fulcio), these tests currently validate expected failure behavior in this setup rather than full end-to-end verification.
AI Usage
request.
pull request. I have described my use of AI below.
I used generative AI for general guidance while structuring tests and thinking about coverage. All code and tests were written, reviewed, and understood by me before submission.
Contributor Checklist
request.
applicable.
Signoff.
Conduct.