search

sigstore / cosign

Code signing and transparency for containers and binaries
Apache License 2.0
4.23k stars 505 forks source link

Set `bundleVerified` to true after Rekor verification (Resolves #3740) #3745

Closed maxlambrecht closed 1 day ago

maxlambrecht commented 1 week ago

Summary

This PR addresses an issue where the bundleVerified flag was not set to true after a successful online Rekor verification. The change ensures that bundleVerified accurately reflects the verification status when Rekor lookup is used.

Resolves #3740

Release Note

Documentation

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 36.83%. Comparing base (2ef6022) to head (b62e437). Report is 140 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3745 +/- ## ========================================== - Coverage 40.10% 36.83% -3.27% ========================================== Files 155 200 +45 Lines 10044 12233 +2189 ========================================== + Hits 4028 4506 +478 - Misses 5530 7177 +1647 - Partials 486 550 +64 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

haydentherapper commented 1 week ago

Thanks! Can you add a test for this? I think you just need to check what's returned by a code block like https://github.com/sigstore/cosign/blob/65969ae877dd2e64520378905c5dda974c98a576/pkg/cosign/verify_test.go#L553-L556.

If adding this test isn't straightforward, that's fine, this is a simple enough change.

maxlambrecht commented 3 days ago

Thanks! Can you add a test for this? I think you just need to check what's returned by a code block like

https://github.com/sigstore/cosign/blob/65969ae877dd2e64520378905c5dda974c98a576/pkg/cosign/verify_test.go#L553-L556 .

If adding this test isn't straightforward, that's fine, this is a simple enough change.

It wasn't straightforward, but I managed to add a comprehensive test covering several scenarios. I refactored the existing test, which only covered a failing validation, and combined it with other cases to ensure thorough coverage.

Let me know if you need any further adjustments or additional scenarios covered.