feat(EC-1957): verify base image release signatures - #1780
Conversation
…istries Add a third path in _image_ref_permitted that verifies a base image's release signature using ec.sigstore.verify_image() with a release_public_key from rule data. This provides stronger cryptographic assurance than registry prefix matching. - Add signature verification clause to _image_ref_permitted - Relax allowed_registries_provided to pass when only release_public_key is configured - Add deprecation warn rule when prefixes are used without a release key - Add schema validation for release_public_key (must be a string) - Update example rule_data.yml with release_public_key entry - Regenerate docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 2:11 AM UTC · Completed 2:20 AM UTC |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBase image release policies now support signature verification through ChangesBase image signature policy
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RuleData
participant BaseImagePolicy
participant Sigstore
participant PolicyResult
RuleData->>BaseImagePolicy: provide release_public_key
BaseImagePolicy->>Sigstore: verify image signature
Sigstore-->>BaseImagePolicy: return verification result
BaseImagePolicy->>PolicyResult: permit or deny base image
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@antora/docs/modules/ROOT/pages/packages/release_base_image_registries.adoc`:
- Around line 12-16: Update all specified base-image documentation to describe
registry-prefix, snapshot-digest, and release-signature verification. In
antora/docs/modules/ROOT/pages/packages/release_base_image_registries.adoc lines
12-16, include release_public_key in the solution; in lines 24-28, rename the
rule/title and solution to cover all three verification paths. Update the
permitted-image labels in antora/docs/modules/ROOT/pages/release_policy.adoc
lines 35-38, 119-122, and 345-348, and the corresponding navigation label in
antora/docs/modules/ROOT/partials/release_policy_nav.adoc lines 22-26.
In `@antora/docs/modules/ROOT/pages/release_policy.adoc`:
- Around line 362-363: Update the visible label for the xref targeting
github_certificate__gh_workflow_ref to “GitHub Certificate Checks: GitHub
Workflow Ref”; leave the github_certificate__gh_workflow_repository link label
unchanged.
In `@policy/release/base_image_registries/base_image_registries.rego`:
- Around line 235-238: Update the _rule_data_errors rule to distinguish an
absent release key from a present value before type validation, so an explicitly
provided empty array is rejected as a non-string while a missing key remains
allowed. Validate every present release key value with the existing string
check, and add a regression case covering release_public_key: [].
🪄 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: 7be5a78f-860d-4e49-8d34-07fcc03095e5
📒 Files selected for processing (6)
antora/docs/modules/ROOT/pages/packages/release_base_image_registries.adocantora/docs/modules/ROOT/pages/release_policy.adocantora/docs/modules/ROOT/partials/release_policy_nav.adocexample/data/rule_data.ymlpolicy/release/base_image_registries/base_image_registries.regopolicy/release/base_image_registries/base_image_registries_test.rego
ReviewFindingsMedium
Low
Previous runReviewFindingsLow
Previous run (2)ReviewFindingsMedium
Low
Previous run (3)ReviewFindingsMedium
Low
Previous run (4)ReviewFindingsMedium
Low
Previous run (5)ReviewFindingsMedium
Low
Labels: PR adds sigstore signature verification feature with documentation updates Previous run (6)ReviewFindingsMedium
Low
Previous run (7)ReviewFindingsMedium
Low
Previous run (8)Review — approveSummaryThis PR adds signature-based base image verification to the AnalysisCorrectness — The signature verification logic is structurally sound. The three-branch Security — The Test adequacy — Eight new tests comprehensively cover: successful signature verification, failed verification, the noop behavior when no key is configured, using only a release key without prefixes, the deprecation warning (presence, absence with key, absence without prefixes), and Documentation — Antora docs, release policy pages, and nav partials are all updated. The new deprecation rule is documented with its warning message, code, and source link. The example Observations
Labels: PR adds new OPA/Rego policy feature for signature-based base image verification |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ready-for-merge
Move the registry prefix deprecation from a standalone warn rule into _rule_data_errors with warning severity, and rename the deny rule title to "Base image is permitted" since verification is no longer registry-specific. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Review · |
|
🤖 Review · Started 3:00 AM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…tries Replace _has_sig_errors helper with direct info.success check via object.get, matching the pattern used in trust.rego. Consolidate _release_public_key_provided into _release_public_key to eliminate duplicate rule_data.get calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Review · |
|
🤖 Review · |
PR Summary by QodoVerify base image release signatures via Sigstore (release_public_key)
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Captures OPA type system constraints discovered through trial and error when working with ec.sigstore.verify_image. The built-in's return type is declared in ec-cli (internal/rego/sigstore/sigstore.go) and is not visible from ec-policies — understanding it required reading the Go source in the ec-cli repo. Key finding: direct field access and count() both fail on the typed return; object.get() is the workaround. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review by Qodo
Context used✅ Compliance rules (platform):
15 rules 1.
|
![qodo-for-conforma[bot]](https://avatars.githubusercontent.com/in/3609154?s=60&v=4){kind=small}
|
🤖 Finished Review · ❌ Failure · Started 7:49 PM UTC · Completed 8:08 PM UTC |
|
🤖 Review · |
Use the shared sigstore.opts config as defaults when building verification options, with release_public_key overriding via object.union. This ensures deployments with custom default_sigstore_opts (rekor_url, ignore_rekor, etc.) have those settings respected by base image verification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| _signing_identity := identity if { | ||
| identities := rule_data.get(_signing_identities_key) | ||
| is_object(identities) | ||
| identity := object.get(identities, _signing_identity_name, {}) | ||
| is_object(identity) | ||
| count(identity) > 0 |
There was a problem hiding this comment.
Since we're already doing checks on the rule data format, this could be simpler I think:
_signing_identity := rule_data.get(_signing_identities_key)["release"]
There was a problem hiding this comment.
But we could also consider adding a rule_data rule, e.g.:
_signing_identity := rule_data.get_signing_identity("release")
|
🤖 Finished Review · ✅ Success · Started 6:37 PM UTC · Completed 6:57 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| } | ||
| image.parse(image_ref).digest in allowed_digests | ||
| } else if { | ||
| opts := object.union(sigstore.opts, _signing_identity) |
There was a problem hiding this comment.
There was some discussion in slack about whether this object.union makes sense now.
|
🤖 Review · |
|
🤖 Review · |
…irectly Rename the signing_identities key from "release" to "rh-release" and remove the object.union merge with sigstore.opts. Each signing identity in rule data is now self-contained with the complete set of verification options passed directly to ec.sigstore.verify_image. There are no useful shared defaults since every verification method requires a distinct combination of fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 8:48 PM UTC · Completed 9:08 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 5:27 PM UTC · Completed 5:48 PM UTC |
|
🤖 Review · |
…gistries Validate signing identity rule data to enforce valid sigstore verification method combinations. Adds JSON schema validation for types and allowed properties, plus semantic rules that enforce: - must specify public_key or certificate identity fields - keyless mode requires issuer and rekor_url - key-based mode requires rekor_url, rekor_public_key, or ignore_rekor The validation logic lives in the sigstore library (validate function and opts_schema) so it is reusable by any sigstore consumer. The base_image_registries policy wraps the errors with rule-data-specific context. Also adds rekor_public_key to the default sigstore opts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 6:01 PM UTC · Completed 6:17 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ready-for-merge
…tion - Always validate allowed_registry_prefixes format (type, uniqueness) even when signing_identities is configured; only skip minItems requirement when a valid signing identity object is present - Add is_array guard to deprecation warning rule - Add validation warning when signing_identities is configured but does not contain the expected rh-release key - Fix test_no_release_key_is_noop to override data.rule_data explicitly so it exercises the prefix-only code path as intended Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Response to bot review findingsFixed in 786edf4:
By design / intentional:
Minor / acknowledged:
|
|
🤖 Finished Review · ✅ Success · Started 9:28 PM UTC · Completed 9:46 PM UTC |
| } | ||
| } | ||
|
|
||
| _rule_data_errors contains error if { |
There was a problem hiding this comment.
[medium] backward-incompatible
Existing consumers providing allowed_registry_prefixes without signing_identities will now receive a new warning-severity deny result on every evaluation. Consumers that treat all deny results equally will see a behavioral change. This is an intentional deprecation nudge.
| _validate_key_rekor(identity) := [error] if { | ||
| _has_value(identity, "public_key") | ||
| object.get(identity, "ignore_rekor", false) != true | ||
| not _has_value(identity, "rekor_url") |
There was a problem hiding this comment.
[low] logic-error
_validate_keyless_rekor only checks for rekor_url in keyless verification, while _validate_key_rekor accepts rekor_url, rekor_public_key, or ignore_rekor. The asymmetry is likely intentional (keyless requires Rekor timestamps for Fulcio certificates) but is undocumented.
| @@ -105,8 +106,13 @@ _image_ref_permitted(image_ref) if { | |||
| img := image.parse(component.containerImage) | |||
| } | |||
| image.parse(image_ref).digest in allowed_digests | |||
There was a problem hiding this comment.
[low] fail-open
The _image_ref_permitted else-if chain evaluates sigstore verification last. Broad allowed_registry_prefixes bypass signature verification even when signing_identities is configured. Intentional for backward compatibility during migration.
| info := ec.sigstore.verify_image(image_ref, _signing_identity) | ||
| object.get(info, "success", false) == true | ||
| } | ||
|
|
There was a problem hiding this comment.
[low] fail-open
When signing_identities exists but lacks the rh-release key, a warning is emitted but the system falls back to prefix-based matching without a hard failure.
| _rule_data_errors contains error if { | ||
| prefixes := rule_data.get(_rule_data_key) | ||
| is_array(prefixes) | ||
| count(prefixes) > 0 |
There was a problem hiding this comment.
[low] pattern-inconsistency
The deprecation warning message omits the 'Rule data' prefix used consistently by all other _rule_data_errors entries.
Suggested fix: Prefix the message: 'Rule data allowed_registry_prefixes is configured without signing_identities...'
| @@ -170,7 +176,6 @@ _cyclonedx_image_ref(component) := image_ref if { | |||
| image_ref := sbom.image_ref_from_purl(purl) | |||
There was a problem hiding this comment.
[low] pattern-inconsistency
Descriptive comment above the first _rule_data_errors rule was removed. This commenting pattern is established across the codebase for validation blocks.
| info := ec.sigstore.verify_image(image_ref, _signing_identity) | ||
| object.get(info, "success", false) == true | ||
| } | ||
|
|
There was a problem hiding this comment.
[low] edge-case
_signing_identity relies on OPA's behavior that string-key indexing of an array is undefined (not an error). Technically correct but non-obvious.
fullsend-ai-review
Bot
added
requires-manual-review
3 participants
Summary
ec.sigstore.verify_image()withsigning_identitiesrule data (named entryrh-release)allowed_registry_prefixesis configured withoutsigning_identitiessigning_identitiesstructure, entry types, sigstore opts (viasigstore.validate), and missingrh-releasekeyallowed_registry_prefixesformat always validated (type, uniqueness);minItemsrequirement relaxed when a valid signing identity is presentResolves: EC-1957
Test plan
make cipasses (1025/1025 tests, 0 lint violations)🤖 Generated with Claude Code