Skip to content

fix: use TrustedRoot from TUF cache for key-based verification#3218

Merged
simonbaird merged 2 commits intoconforma:mainfrom
SequeI:rekorFix
Apr 3, 2026
Merged

fix: use TrustedRoot from TUF cache for key-based verification#3218
simonbaird merged 2 commits intoconforma:mainfrom
SequeI:rekorFix

Conversation

@SequeI
Copy link
Copy Markdown
Contributor

@SequeI SequeI commented Apr 2, 2026

The key-based verification path (--public-key) bypassed the modern TUF cache and always fetched Rekor public keys via the legacy TUF client, which fails with expired root.json after cosign v3's initialize stopped populating the legacy cache.

Move cosign.TrustedRoot() out of the keyless-only branch so both workflows use the modern cache first, falling back to legacy fetches when unavailable.

https://redhat.atlassian.net/browse/EC-1755?atlOrigin=eyJpIjoiNDA4YmEzNTk0YzZjNGUyNTg1ZGQ3YjNmMzE5Y2IxNDUiLCJwIjoiaiJ9 for more info

The key-based verification path (--public-key) bypassed the modern
TUF cache and always fetched Rekor public keys via the legacy TUF
client, which fails with expired root.json after cosign v3's
initialize stopped populating the legacy cache.

Move cosign.TrustedRoot() out of the keyless-only branch so both
workflows use the modern cache first, falling back to legacy fetches
when unavailable.

Signed-off-by: SequeI <asiek@redhat.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f9badad4-f971-4f5b-87ed-4d8ccf7dbf9e

📥 Commits

Reviewing files that changed from the base of the PR and between ea10de5 and f5e7a9c.

⛔ Files ignored due to path filters (3)
  • acceptance/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
  • tools/go.sum is excluded by !**/*.sum
📒 Files selected for processing (3)
  • acceptance/go.mod
  • go.mod
  • tools/go.mod
✅ Files skipped from review due to trivial changes (3)
  • acceptance/go.mod
  • tools/go.mod
  • go.mod

📝 Walkthrough

Walkthrough

Adjusted control flow in checkOpts so trusted-root loading is executed based on environment overrides independently of the keyless workflow check; Fulcio/CT-log material and related population remain conditional on p.PublicKey == "" and opts.TrustedMaterial == nil.

Changes

Cohort / File(s) Summary
Trusted-Root Loading Refactor
internal/policy/policy.go
Reorganized conditional nesting in checkOpts: trusted-root loading now runs when environment overrides allow irrespective of keyless status. Fulcio/CT-log roots, intermediates, and CT public keys are only populated when p.PublicKey == "" and opts.TrustedMaterial == nil, changing when remote material is fetched.
Dependency version bumps
go.mod, acceptance/go.mod, tools/go.mod
Updated indirect github.com/docker/cli module versions (various files) to v29.3.1+incompatible; no other module directives or API declarations changed.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly summarizes the main change: refactoring to use TrustedRoot from TUF cache for key-based verification, which aligns with the core fix in internal/policy/policy.go.
Description check ✅ Passed The description is clearly related to the changeset, explaining the rationale for moving cosign.TrustedRoot() out of the keyless-only branch and detailing the problem with the legacy TUF client.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix: Use TrustedRoot from TUF cache for key-based verification

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Move TrustedRoot TUF cache lookup outside keyless-only branch
• Both key-based and keyless workflows now use modern TUF cache first
• Fulcio/CT log fallback restricted to keyless path only (p.PublicKey == "")
Diagram
flowchart LR
  A["checkOpts()"] --> B{"PublicKey set?"}
  B -- "yes" --> C["Set SigVerifier"]
  B -- "no" --> D["Set Identities (keyless)"]
  C --> E{"hasSigstoreEnvOverrides?"}
  D --> E
  E -- "no" --> F["cosign.TrustedRoot()"]
  F -- "success" --> G["opts.TrustedMaterial = trustedRoot"]
  F -- "fail" --> H{"PublicKey == '' AND TrustedMaterial == nil?"}
  E -- "yes" --> H
  G --> I["Return opts"]
  H -- "yes" --> J["Fetch Fulcio roots, intermediates, CT log keys"]
  H -- "no" --> I
  J --> I
Loading

Grey Divider

File Changes

1. internal/policy/policy.go 🐞 Bug fix +21/-21

Extend TUF trusted root usage to key-based verification path

• Moved cosign.TrustedRoot() TUF cache lookup outside the keyless-only branch so key-based
 (--public-key) verification also benefits from the modern TUF cache.
• Fulcio root/intermediate certs and CT log public key fetches are now guarded by `p.PublicKey == ""
 && opts.TrustedMaterial == nil`, restricting them to the keyless fallback path only.
• Sigstore env override check now applies to both key-based and keyless workflows.

internal/policy/policy.go


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

Code Review by Qodo

Grey Divider

Looking for bugs?

Check back in a few minutes. An AI review agent is analyzing this pull request.

Grey Divider

Qodo Logo

@joejstuart
Copy link
Copy Markdown
Contributor

/ok-to-test

simonbaird
simonbaird previously approved these changes Apr 3, 2026
Copy link
Copy Markdown
Member

@simonbaird simonbaird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, thanks very much @SequeI .

@joejstuart
Copy link
Copy Markdown
Contributor

/retest

Should fix a particular CVE being detected by clair
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 3, 2026

Codecov Report

❌ Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/policy/policy.go 82.35% 3 Missing ⚠️
Flag Coverage Δ
acceptance 55.20% <70.58%> (+0.01%) ⬆️
generative 17.87% <0.00%> (-0.03%) ⬇️
integration 26.61% <0.00%> (-0.02%) ⬇️
unit 69.01% <76.47%> (-0.02%) ⬇️

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

Files with missing lines Coverage Δ
internal/policy/policy.go 91.97% <82.35%> (ø)

... and 8 files with indirect coverage changes

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

@simonbaird
Copy link
Copy Markdown
Member

/ok-to-test

@simonbaird
Copy link
Copy Markdown
Member

Looks like a Konflux flake... Will try again.

@simonbaird
Copy link
Copy Markdown
Member

/retest

@simonbaird simonbaird merged commit d16e3a0 into conforma:main Apr 3, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants