Add --skip-att-sig-check flag

[simonbaird]

Implement --skip-att-sig-check flag to skip attestation signature validation checks, mirroring the existing --skip-image-sig-check flag. When enabled, attestation signature verification is bypassed during image validation.

Why? Often I'm debugging/troubleshooting something and I get given an image ref to look at. We can use cosign download attestation to inspect the attestation, which is very useful, but if we want to try running Conforma against it, we must either guess, find, or ask to be provided with the public key. Sometimes that's not so difficult, but other times it may be very difficult or even impossible. (Consider for example if the image was built on an ephemeral cluster and the signing secret used is gone forever.)

Now we can use --skip-image-sig-check and --skip-att-sig-check and carry on with the debugging. Note that we added the --skip-image-sig-check recently for other reasons, see https://redhat.atlassian.net/browse/EC-1647. The
--skip-att-sig-check is perhaps a little more complicated because we need to add a function that can download the attestation without verifying it.

The argument against this is that it may encourage less secure practices, but I would say it's acceptable because we're not changing the default behavior, which is always to require signature verification.

Ref: https://redhat.atlassian.net/browse/EC-1815

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds support for independently skipping attestation signature verification during image validation via a new --skip-att-sig-check CLI flag. When enabled, validation fetches and parses attestations without verifying signatures. This is wired through the policy layer and conditionally applied in the validate function.

Changes

Attestation Signature Check Skip Feature

Layer / File(s)	Summary
Policy flag and CLI wiring internal/policy/policy.go, cmd/validate/image.go	Policy interface adds SkipAttSigCheck() bool method; policy struct stores the flag; Options wires it via NewPolicy. The CLI adds --skip-att-sig-check flag and populates data.skipAttSigCheck for policy construction.
Attestation fetching without verification internal/evaluation_target/application_snapshot_image/application_snapshot_image.go	New exported method FetchAttestationsWithoutVerification() fetches Sigstore attestations via cosign OCI remote APIs without signature verification, parses bundle-based and non-bundle attestations by predicate type (SLSA v0.2, v1, SPDX), and returns wrapped errors on failure.
Conditional validation logic internal/image/validate.go	ValidateImage now conditionally skips attestation signature checks when p.SkipAttSigCheck() is true, fetching attestations without verification and continuing even on fetch failures; otherwise performs existing attestation validation.
Documentation docs/modules/ROOT/pages/ec_validate_image.adoc	Documents new --skip-att-sig-check CLI option with its default value (false).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding a --skip-att-sig-check flag for skipping attestation signature verification, which is the primary objective of the changeset.
Description check	✅ Passed	The description is well-related to the changeset, explaining what the flag does, why it was added (debugging workflows), and how it mirrors existing functionality.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[simonbaird]

Making it a draft because it has no Jira (yet anyway).

[codecov]

Codecov Report

❌ Patch coverage is 19.64286% with 45 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...ation_snapshot_image/application_snapshot_image.go	0.00%	42 Missing ⚠️
internal/image/validate.go	62.50%	3 Missing ⚠️

Flag	Coverage Δ
acceptance	55.39% <19.64%> (-0.20%)	⬇️
generative	17.74% <0.00%> (-0.09%)	⬇️
integration	26.47% <5.35%> (-0.10%)	⬇️
unit	68.78% <16.07%> (-0.26%)	⬇️

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

Files with missing lines	Coverage Δ
cmd/validate/image.go	91.42% <100.00%> (+0.06%)	⬆️
internal/policy/policy.go	91.74% <100.00%> (+0.07%)	⬆️
internal/image/validate.go	69.71% <62.50%> (-1.09%)	⬇️
...ation_snapshot_image/application_snapshot_image.go	71.69% <0.00%> (-13.10%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[simonbaird]

This isn't in the sprint currently, so maybe review it only after you reviewed everything else.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/evaluation_target/application_snapshot_image/application_snapshot_image.go (1)

280-314: ⚡ Quick win

Deduplicate attestation parsing to avoid behavior drift

Line 280 through Line 314 duplicates the non-bundle parsing path from ValidateAttestationSignature. A future change in one path can silently desync the other and produce inconsistent attestation handling.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@internal/evaluation_target/application_snapshot_image/application_snapshot_image.go`
around lines 280 - 314, This block duplicates the non-bundle attestation parsing
logic found in ValidateAttestationSignature; extract the parsing/appending logic
into a single helper (or call the existing ValidateAttestationSignature parsing
routine) so both paths use the same code path: for each signature from
layers.Get() call the shared function that runs ProvenanceFromSignature,
switches on PredicateType and converts to SLSA v0.2/v1 or generic attestation
via SLSAProvenanceFromSignature, SLSAProvenanceFromSignatureV1 and appends the
result to a.attestations; remove the duplicated switch in the current loop and
replace it with a call to that helper to avoid divergence.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/image/validate.go`:
- Around line 87-90: The attestation fetch error is being suppressed in the
validation path; change the behavior in the block that calls
a.FetchAttestationsWithoutVerification(ctx) so that instead of only logging and
continuing (the log.Debugf("Failed to fetch attestations without verification:
%v", err) path) you return or propagate a wrapped error to abort this
component's validation; locate the call to
a.FetchAttestationsWithoutVerification and replace the silent-continue with an
immediate return of the error (or fmt.Errorf/Wrap with context) so callers see
the registry fetch failure.

---

Nitpick comments:
In
`@internal/evaluation_target/application_snapshot_image/application_snapshot_image.go`:
- Around line 280-314: This block duplicates the non-bundle attestation parsing
logic found in ValidateAttestationSignature; extract the parsing/appending logic
into a single helper (or call the existing ValidateAttestationSignature parsing
routine) so both paths use the same code path: for each signature from
layers.Get() call the shared function that runs ProvenanceFromSignature,
switches on PredicateType and converts to SLSA v0.2/v1 or generic attestation
via SLSAProvenanceFromSignature, SLSAProvenanceFromSignatureV1 and appends the
result to a.attestations; remove the duplicated switch in the current loop and
replace it with a call to that helper to avoid divergence.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 1da8673f-6dfe-44f1-a84d-a6396b58f8cb

📥 Commits

Reviewing files that changed from the base of the PR and between 3743934 and 767ffe0.

📒 Files selected for processing (5)

• cmd/validate/image.go
• docs/modules/ROOT/pages/ec_validate_image.adoc
• internal/evaluation_target/application_snapshot_image/application_snapshot_image.go
• internal/image/validate.go
• internal/policy/policy.go

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t suppress attestation fetch failures in skip mode

Line 87 currently swallows registry fetch errors and continues, which can surface misleading downstream results instead of the real retrieval failure. Surface this failure in output and stop this component’s validation path.

Proposed change

 	if p.SkipAttSigCheck() {
 		log.Debug("Attestation signature check skipped, fetching attestations without verification")
 		if err := a.FetchAttestationsWithoutVerification(ctx); err != nil {
-			log.Debugf("Failed to fetch attestations without verification: %v", err)
-			// Continue with validation even if attestation fetch fails
+			out.SetAttestationSyntaxCheckFromError(fmt.Errorf("unable to fetch attestations without verification: %w", err))
+			return out, nil
 		}
 	} else {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/image/validate.go` around lines 87 - 90, The attestation fetch error
is being suppressed in the validation path; change the behavior in the block
that calls a.FetchAttestationsWithoutVerification(ctx) so that instead of only
logging and continuing (the log.Debugf("Failed to fetch attestations without
verification: %v", err) path) you return or propagate a wrapped error to abort
this component's validation; locate the call to
a.FetchAttestationsWithoutVerification and replace the silent-continue with an
immediate return of the error (or fmt.Errorf/Wrap with context) so callers see
the registry fetch failure.