fix: add cosignv3 signature support, Fix LogID encoding

[JasonPowr]

Cherry-pick: sigstore#1968

[qodo-for-securesign]

PR Summary by Qodo

Fix LogID hex encoding and add cosign v3 bundle signature verification

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Decode transparency LogID hex strings into raw bytes everywhere they are marshalled/used.
• Fix cosign TLog/CTLog key map indexing by using hex-encoded LogID bytes.
• Add verification path for cosign v3 (new bundle format) policy signatures and update fixtures.

Diagram

graph TD
  A["TrustRoot reconciler"] --> B["SigstoreKeys (LogID bytes)"] --> C["Webhook validator"] --> D{{"cosign verify"}}
  B --> E["TrustRoot proto conversion"] --> C
  C --> F["TLog/CTLog key maps"]
  C --> G["Signature/attestation verification"] --> D

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize LogID parsing/formatting helpers

• ➕ Avoids repeated hex.DecodeString/EncodeToString logic across packages
• ➕ Reduces risk of future representation mismatches (bytes vs hex vs base64)
• ➖ Requires a small API surface addition (utility package or config helper)
• ➖ Might be slightly more invasive than a cherry-pick fix

2. Change LogID source to produce bytes directly

• ➕ Eliminates decode steps and associated error paths
• ➕ Single canonical representation from generation through verification
• ➖ May require changing upstream helper functions/interfaces
• ➖ Potentially larger diff and higher regression risk

Recommendation: The PR’s approach is appropriate for a targeted cherry-pick: decode hex LogIDs at the boundaries where they are constructed/converted, and ensure map keys are derived consistently via hex.EncodeToString on the underlying bytes. If this logic continues to grow, consider centralizing LogID encode/decode helpers to prevent future byte/string mismatches.

Files changed (9) +92 / -28

Enhancement (1) +6 / -0

validation.goAdd verification helper for cosign new bundle format signatures +6/-0

Add verification helper for cosign new bundle format signatures

• Introduces validBundleSignatures, which configures the claim verifier and verifies using the attestation verification path needed for the new bundle format. This enables signature validation when cosign's NewBundleFormat is in use.

pkg/webhook/validation.go

Bug fix (4) +50 / -10

gentestdata.goDecode generated LogIDs from hex before storing into SigstoreKeys +12/-2

Decode generated LogIDs from hex before storing into SigstoreKeys

• Updates testdata generation to store LogId.KeyId as raw bytes by hex-decoding the generated LogID string. Adds clearer decode error messages for TLog and CTLog LogID population.

hack/gentestdata/gentestdata.go

sigstore_keys.goFix TransparencyLogInstance conversion to emit LogID bytes +6/-1

Fix TransparencyLogInstance conversion to emit LogID bytes

• Decodes the computed LogID (hex string) into bytes before populating the protobuf LogId.KeyId. Prevents downstream consumers from treating ASCII hex as the key ID bytes.

pkg/apis/config/sigstore_keys.go

trustroot.goDecode LogIDs in reconciler and transparency log instance generation +16/-3

Decode LogIDs in reconciler and transparency log instance generation

• Ensures Rekor and CTLog LogIDs are hex-decoded to bytes when populating SigstoreKeys in reconciliation and when generating TransparencyLogInstance protobufs. Improves error messages for invalid LogID encodings.

pkg/reconciler/trustroot/trustroot.go

validator.goBranch signature verification for NewBundleFormat and fix LogID map keys +16/-4

Branch signature verification for NewBundleFormat and fix LogID map keys

• Updates keyless and RFC3161 timestamp authority validation to call the new bundle verification path when checkOpts.NewBundleFormat is set. Fixes CTLog/Rekor key maps to use hex.EncodeToString(LogId.KeyId) rather than string-casting raw bytes.

pkg/webhook/validator.go

Tests (4) +36 / -18

marshalledEntry.jsonUpdate marshalled trustroot fixture LogID encodings +4/-4

Update marshalled trustroot fixture LogID encodings

• Adjusts JSON test fixture values for logId.keyId to match the corrected byte representation (base64 of raw bytes) rather than base64 of ASCII hex.

pkg/reconciler/trustroot/testdata/marshalledEntry.json

marshalledEntryFromMirrorFS.jsonUpdate mirror FS marshalled fixture LogID encodings +4/-4

Update mirror FS marshalled fixture LogID encodings

• Keeps mirror-based trustroot fixtures consistent with the corrected LogID byte encoding used in marshalled outputs.

pkg/reconciler/trustroot/testdata/marshalledEntryFromMirrorFS.json

trustroot_test.goFix trustroot reconciler tests to use decoded LogID bytes +11/-2

Fix trustroot reconciler tests to use decoded LogID bytes

• Updates test config map construction to hex-decode known LogID strings once and reuse the resulting bytes for TLog/CTLog entries. Aligns tests with the corrected LogID representation.

pkg/reconciler/trustroot/trustroot_test.go

validator_test.goUpdate validator tests for LogID bytes and hex-keyed maps +17/-8

Update validator tests for LogID bytes and hex-keyed maps

• Adds a mustHexDecode helper and updates multiple tests to populate LogId.KeyId with decoded bytes. Adjusts expected transparency log key map keys to be the hex string forms of the LogID bytes.

pkg/webhook/validator_test.go

[qodo-for-securesign]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. LogID double-encoding on upgrade ✓ Resolved 🐞 Bug ☼ Reliability

Description

The webhook now uses hex.EncodeToString(LogId.KeyId) when building Rekor/CTLog key maps, but older
ConfigMaps produced by the previous controller stored KeyId as ASCII bytes of the hex string (via
[]byte(logID)). If the webhook rolls out before the reconciler rewrites the ConfigMap, the webhook
will hex-encode those ASCII bytes (double-encoding) and fail to match transparency log IDs, breaking
verification.

Code

pkg/webhook/validator.go[1657]

+		ctlogKeys.Keys[hex.EncodeToString(ctlog.LogId.KeyId)] = cosign.TransparencyLogPubKey{

Evidence

The PR changes LogID producers from storing ASCII bytes ([]byte(logID)) to storing decoded bytes
(hex.DecodeString(logID)), and simultaneously changes the webhook consumer to hex-encode whatever
bytes it reads when building the Rekor/CT log key maps. Because SigstoreKeys are
protojson-unmarshaled from the ConfigMap without any migration step, stale pre-upgrade ConfigMaps
will retain the legacy ASCII-byte representation and be double-encoded by the new webhook until the
reconciler updates them.

pkg/webhook/validator.go[1649-1661]
pkg/webhook/validator.go[1733-1755]
pkg/reconciler/trustroot/trustroot.go[88-115]
pkg/apis/config/sigstore_keys.go[161-182]
pkg/apis/config/sigstore_keys.go[91-97]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The webhook expects `LogId.KeyId` to be raw bytes and converts it to the hex LogID string via `hex.EncodeToString`. But older serialized SigstoreKeys in ConfigMaps used `KeyId: []byte(logID)` where `logID` is already a hex string, so `KeyId` is ASCII bytes. After this PR, a rolling upgrade can put the new webhook in front of an old ConfigMap, causing `hex.EncodeToString([]byte("d12d..."))` to produce a different (double-encoded) key and break Rekor/CT log key lookup.

### Issue Context
Producers now decode the LogID hex string before storing in protobuf (`hex.DecodeString(logID)`), but consumers may see stale ConfigMap entries until the reconciler updates them.

### Fix Focus Areas
- pkg/webhook/validator.go[1652-1760]
- pkg/reconciler/trustroot/trustroot.go[88-115]

### Suggested fix
1. Add a small helper in the webhook to normalize both representations when constructing map keys:
  - If `len(keyID)==32`, treat as raw bytes and `hex.EncodeToString(keyID)`.
  - Else if `len(keyID)==64` and it is valid hex (or `hex.DecodeString(string(keyID))` succeeds to 32 bytes), treat as legacy ASCII-hex and use `string(keyID)`.
  - Otherwise, fall back to `hex.EncodeToString(keyID)`.
2. Use this helper for both CTLog and Rekor key maps (`fulcioCertsFromAuthority` and `rekorKeysFromTrustRef`).
3. Add/extend a unit test that loads a SigstoreKeys entry containing legacy ASCII `KeyId` bytes and asserts the produced `TrustedTransparencyLogPubKeys.Keys` map is keyed by the correct hex LogID string.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[codecov-commenter]

Codecov Report

❌ Patch coverage is 34.88372% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.18%. Comparing base (c84a0f5) to head (d5ef2b1).

Files with missing lines	Patch %	Lines
hack/gentestdata/gentestdata.go	0.00%	8 Missing ⚠️
pkg/webhook/validator.go	46.66%	5 Missing and 3 partials ⚠️
pkg/reconciler/trustroot/trustroot.go	50.00%	3 Missing and 3 partials ⚠️
pkg/webhook/validation.go	0.00%	4 Missing ⚠️
pkg/apis/config/sigstore_keys.go	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #248      +/-   ##
==========================================
+ Coverage   51.10%   51.18%   +0.08%     
==========================================
  Files         122      122              
  Lines        7409     7443      +34     
==========================================
+ Hits         3786     3810      +24     
- Misses       3252     3255       +3     
- Partials      371      378       +7     

Flag	Coverage Δ
e2e	56.22% <37.14%> (+0.20%)	⬆️
unit	30.68% <30.23%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.