Skip to content

Default to using the new protobuf format #4318

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

steiza
Copy link
Member

@steiza steiza commented Jul 31, 2025

Summary

This is a first step towards #4221, where we default --new-bundle-format to true instead of false.

We never did add protobuf support to cosign sign (see #3927 and #3139) - maybe we want to wait until that is done? Otherwise we could just have --new-bundle-format default to true when we add it to cosign sign.

Release Note

  • Modified attest, attest-blob, sign, sign-blob, dockerfile-verify, manifest-verify, verify, verify-attestation, verify-blob, verify-blob-attestation so that --new-bundle-format defaults to true instead of false.

Documentation

Ran make docgen as part of this PR.

Copy link

codecov bot commented Jul 31, 2025

Codecov Report

❌ Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 34.32%. Comparing base (2ef6022) to head (70e45b0).
⚠️ Report is 466 commits behind head on main.

Files with missing lines Patch % Lines
cmd/cosign/cli/options/attest.go 0.00% 1 Missing ⚠️
cmd/cosign/cli/options/attest_blob.go 0.00% 1 Missing ⚠️
cmd/cosign/cli/options/sign.go 0.00% 1 Missing ⚠️
cmd/cosign/cli/options/signblob.go 0.00% 1 Missing ⚠️
cmd/cosign/cli/options/verify.go 0.00% 1 Missing ⚠️
cmd/cosign/cli/verify.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4318      +/-   ##
==========================================
- Coverage   40.10%   34.32%   -5.78%     
==========================================
  Files         155      216      +61     
  Lines       10044    15007    +4963     
==========================================
+ Hits         4028     5151    +1123     
- Misses       5530     9192    +3662     
- Partials      486      664     +178     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@steiza steiza marked this pull request as ready for review July 31, 2025 17:05
@steiza steiza requested review from a team as code owners July 31, 2025 17:05
@haydentherapper
Copy link
Contributor

Overall LGTM. Let's hold off on this PR until we land #4316 and #4319, do a pass over the other open PRs, and review the Cosign v3 brainstorm to see if there's anything else we want to get in. Then let's cut a new release of Cosign, hopefully the last v2 release, and then start merging these breaking changes.

steiza added 7 commits August 7, 2025 14:42
…ml; until cosign sign supports new bundle format

Signed-off-by: Zach Steindler <[email protected]>
Signed-off-by: Zach Steindler <[email protected]>
Have `cosign sign` default to true

Signed-off-by: Zach Steindler <[email protected]>
@steiza steiza force-pushed the default-protobuf branch from 1326825 to 307f358 Compare August 7, 2025 19:52
Signed-off-by: Zach Steindler <[email protected]>
@steiza
Copy link
Member Author

steiza commented Aug 7, 2025

So, in some senses this is not a breaking change, in that we've made it so that verify-blob and verify-blob-attestation will "fall back" to trying the old bundle format if they can't parse the provided bundle using the protobuf:

$ cat test.sigstore.json 
{"base64Signature":"MEUCIEjqaXVxJgbngDBIXX9DnZ9yuQBK2Y2dU2C1O74T0RvlAiEA9WI9il4+bHMVtzGl8lxrX3SxCfdg5IMUT90Zk5roOu8="}

Verifies if you explicitly say it's the old bundle format:

$ go run cmd/cosign/main.go verify-blob --bundle=test.sigstore.json --new-bundle-format=false --key=cosign.pub --use-signed-timestamps=false --insecure-ignore-tlog=true ../sigstore-go/examples/sigstore-go-signing/hello_world.txt 
...
Verified OK

... but also verifies if you omit the bundle format (defaults to true):

$ go run cmd/cosign/main.go verify-blob --bundle=test.sigstore.json --key=cosign.pub --use-signed-timestamps=false --insecure-ignore-tlog=true ../sigstore-go/examples/sigstore-go-signing/hello_world.txt 
...
Verified OK

I spent some time looking into providing this fallback behavior for OCI verify commands (verify, verify-attestation) as well. It's technical possible of course, but it would either add quite a few requests to your registry for each verify (essentially doubling the requests) or require a somewhat substantial refactor in order to query once and then pass the results along to pkg/cosign/verify.go functions - what do you think?

@steiza
Copy link
Member Author

steiza commented Aug 7, 2025

There's some issues with how the trusted root file is working in verify - I'll have to take a look later

@steiza
Copy link
Member Author

steiza commented Aug 8, 2025

I couldn't get the scaffolding working with the new bundle format - I think there was some issue with how the trusted root was constructed - but we can update it when we overhaul the scaffolding test.

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.

2 participants