Skip to content

feat(EC-1950,EC-1982): test attestation parity and trust verification fixes#1770

Merged
joejstuart merged 12 commits into
conforma:mainfrom
joejstuart:EC-1982
Jul 9, 2026
Merged

feat(EC-1950,EC-1982): test attestation parity and trust verification fixes#1770
joejstuart merged 12 commits into
conforma:mainfrom
joejstuart:EC-1982

Conversation

@joejstuart

@joejstuart joejstuart commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • EC-1950: Bring test_attestation package to feature parity with the test package — adds rules for warned results, informative tests, erred/skipped results, subject mismatch, unsupported result values, and rule data validation.
  • EC-1982: Fix trust verification for in-toto test attestations attached as OCI referrers:
    • Fix _has_trusted_provenance to call ec.sigstore.verify_attestation directly on the referrer ref instead of looking for provenance via ec.oci.image_referrers. Tekton Chains stores SLSA provenance as cosign .att tags, not OCI referrers.
    • Fix verified_statements to use parsed_blob_from_image so the in-toto statement is extracted from the OCI artifact layer instead of returning the manifest JSON.

Jira: EC-1950, EC-1982

Test plan

  • All 998 ec-policies unit tests pass with 100% coverage
  • its-poc demo Phase 3: legitimate test attestation verified through Chains trust chain
  • its-poc demo: fake attestation correctly excluded (no Chains provenance)

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds shared intoto subject-digest helpers, changes trust verification to validate attestations directly, and expands release test-attestation policy logic to use rule-data-driven result categories, subject matching, and rule-data validation. Tests and Antora docs were updated to match the new policy behavior and naming.

Changes

Intoto trust flow

Layer / File(s) Summary
Digest helpers and reuse
policy/lib/intoto/intoto.rego, policy/lib/intoto/intoto_test.rego, policy/release/slsa_build_scripted_build/slsa_build_scripted_build.rego
Adds subject_digests and subject_digest, tests both helpers, and reuses subject_digest in scripted build subject collection.
Direct attestation verification
policy/lib/intoto/trust.rego
Parses blobs from images, verifies Sigstore attestations directly, and matches attestation subjects against the expected digest.
Trust test mocks
policy/lib/intoto/trust_test.rego
Reworks trust test fixtures around image manifests, layer digests, and statement-digest-based selection.

Estimated code review effort: 4 (Complex) | ~55 minutes

Release test attestation policy

Layer / File(s) Summary
Rule data and core policy
policy/lib/rule_data/rule_data.rego, policy/release/test_attestation/test_attestation.rego
Adds default result vocabularies and updates the policy to use configurable result sets, count-based detail, subject matching, new erred/skipped checks, and rule-data validation.
Policy tests
policy/release/test_attestation/test_attestation_test.rego
Updates mocks, manifest wiring, and assertions for the revised result model and new validation cases.
Docs and navigation
antora/docs/modules/ROOT/pages/packages/release_slsa_build_scripted_build.adoc, antora/docs/modules/ROOT/pages/packages/release_test_attestation.adoc, antora/docs/modules/ROOT/pages/release_policy.adoc, antora/docs/modules/ROOT/partials/release_policy_nav.adoc
Updates source links and documents the expanded rule set and navigation entries.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant trust.rego
  participant oci
  participant ec.sigstore
  trust.rego->>oci: parsed_blob_from_image(referrer.ref)
  trust.rego->>ec.sigstore: verify_attestation(referrer.ref, sigstore.opts)
  ec.sigstore-->>trust.rego: success, attestations
  trust.rego->>trust.rego: compare subject_digest(subject) to expected_digest
  trust.rego->>trust.rego: _all_tasks_trusted(att)
Loading
sequenceDiagram
  participant test_attestation.rego
  participant rule_data.rego
  participant intoto.rego
  participant oci
  test_attestation.rego->>rule_data.rego: supported_test_attestation_results, categorized result keys
  test_attestation.rego->>oci: parse image reference
  test_attestation.rego->>intoto.rego: subject_digests(subject), subject_digest(subject)
  test_attestation.rego->>test_attestation.rego: emit deny/warn results
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the main changes: test attestation parity and trust verification fixes.
Description check ✅ Passed The description matches the implemented changes and objectives for both EC-1950 and EC-1982.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 8, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 7:28 PM UTC · Completed 7:35 PM UTC
Commit: 8f05d87 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review — ✅ Approve

PR: feat(EC-1950,EC-1982): test attestation parity and trust verification fixes
Reviewed: 12 files, +701 / −248

Summary

This PR delivers two Jira items cleanly in a single branch:

EC-1950test_attestation feature parity. Adds six new rules (no_erred_test_attestations, no_skipped_test_attestations, no_failed_informative_test_attestations, no_test_warnings [configurable], subject_mismatch, rule_data_provided) and makes result types configurable via rule data, matching the existing test package's capabilities. Rule data defaults and schema validation are correctly wired through rule_data.rego.

EC-1982 — Trust verification fix. Corrects two issues in policy/lib/intoto/trust.rego:

  1. _has_trusted_provenance now calls ec.sigstore.verify_attestation directly on the statement referrer instead of searching for a provenance referrer via a second ec.oci.image_referrers call — matching how Tekton Chains actually attaches SLSA provenance (cosign .att tags, not OCI referrers).
  2. verified_statements now uses oci.parsed_blob_from_image to extract the in-toto statement from the OCI artifact's first layer instead of treating the referrer digest as a plain blob reference.

Both changes remove the now-unnecessary two-hop verification path and the associated OPA v1.12.1 type-checker workaround, resulting in a net simplification.

Shared refactor. subject_digest / subject_digests are extracted from slsa_build_scripted_build.rego and trust.rego into policy/lib/intoto/intoto.rego for reuse by the new subject_mismatch rule and the trust layer.

Architecture & correctness

  • Dual result detection via _has_result: The two-clause function (result string match OR count > 0) is a sound defensive pattern — a buggy producer reporting result: "PASSED" alongside failures: 5 will still trigger a deny. Test test_count_triggers_deny exercises this exactly.
  • Sentinel count key "n/a" for erred/skipped: Since the predicate spec defines no count field for ERROR/SKIPPED results, passing "n/a" as the count key causes object.get(predicate, "n/a", 0) to return 0, disabling the count-based clause. The comments explain this clearly.
  • subject_mismatch uses image.parse(input.image.ref).digest which correctly produces the algorithm:value format (e.g., sha256:abc123) matching intoto.subject_digests output.
  • Informative tests are correctly partitioned: no_failed_tests (deny) uses not _test_name(statement) in rule_data.get("informative_test_attestations"), while no_failed_informative_test_attestations (warn) uses the complementary in check — these are mutually exclusive per statement.
  • informative_test_attestations defaults to [] via the rule_data.get fallback, meaning no tests are informative by default — correct fail-strict behavior.
  • Rule data validation schema constrains result values to the known enum ["PASSED", "FAILED", "WARNED", "ERROR", "SKIPPED"] and requires unique items. informative_test_attestations validates as a unique string array without enum restriction, allowing arbitrary test names.

Trust model change

The trust model simplification (single-hop verification) is architecturally significant but well-justified. The previous two-hop model (image → statement referrer → provenance referrer) assumed provenance was attached as an OCI referrer to the statement artifact. Tekton Chains instead signs the statement artifact itself via cosign, so direct ec.sigstore.verify_attestation on the statement referrer is the correct verification path. The _attests_to_subject check against referrer.digest provides the binding that ensures the provenance attestation actually covers the statement being verified.

Test adequacy

Tests comprehensively cover:

  • All six new rules (erred, skipped, informative, warnings, subject mismatch, rule data validation)
  • Edge cases (count-based trigger without result match, missing subject, custom rule data overrides)
  • Multi-attestation scenarios with the updated mock infrastructure
  • All trust.rego mocks updated to remove provenance referrer indirection and add ec.oci.image_manifest mock for parsed_blob_from_image

Documentation

Auto-generated Antora docs are updated with correct line numbers and new rule entries. Navigation and release policy docs include all new rules.

Findings

No blocking findings. The change is well-structured, properly tested, and the security improvements (subject mismatch validation, dual result detection) strengthen the policy framework.

Low-severity observations (non-blocking)
  • Message wording when count is absent: When a test has result: "FAILED" but no failures key in the predicate, the message reads "failures: 0". This is technically accurate (the count defaults to 0) but could be slightly confusing to operators who see "0 failures" alongside a "FAILED" result. Consider a future refinement to distinguish "not reported" from "0" — not blocking for this PR.
  • Sentinel "n/a" pattern: Using a literal string "n/a" as a nonexistent predicate key is functional but unconventional. An alternative would be a separate _has_result_by_string(predicate, results) function, avoiding the unused parameter. This is a style preference and does not affect correctness.

Labels: PR adds new policy rules and modifies trust verification logic

Previous run

Review — approve

PR: feat(EC-1950,EC-1982): test attestation parity and trust verification fixes

Summary

This PR contains two related changes:

  1. EC-1982 — Trust verification fix: Changes the trust model for in-toto test attestations from a two-hop lookup (image → referrer → provenance referrer → Sigstore verify) to a direct model (image → referrer → Sigstore verify). This aligns with how Tekton Chains actually stores SLSA provenance (cosign .att tags, not OCI referrers). Also switches verified_statements from parsed_blob to parsed_blob_from_image so the in-toto statement is correctly extracted from the OCI artifact's first layer rather than parsing the manifest JSON.

  2. EC-1950 — Test attestation feature parity: Brings test_attestation to parity with the test package by adding rules for warned results, informative tests, erred/skipped results, subject mismatch validation, unsupported result values, and rule data validation. All result types are now configurable via rule_data instead of hardcoded.

Shared subject_digest/subject_digests helpers are extracted from duplicated code in trust.rego and slsa_build_scripted_build.rego into the intoto library.

Correctness

  • Trust model change is well-motivated and correctly implemented. The new model directly verifies the in-toto referrer via Sigstore rather than searching for a separate provenance referrer. The _has_trusted_provenance still validates: (a) Sigstore verification succeeds, (b) an attestation subject matches the referrer digest, and (c) all pipeline tasks are trusted. Fail-closed behavior is preserved via object.get defaults.
  • parsed_blobparsed_blob_from_image is correct. OCI artifacts stored as images have content in the first layer, not the manifest. parsed_blob_from_image correctly fetches the manifest, extracts the first layer digest, and fetches that blob. Tests properly mock ec.oci.image_manifest.
  • _has_result dual-trigger is intentional and security-positive. A test with result: "PASSED" but failures: 5 will still trigger a deny — preventing result-string spoofing when the underlying data contradicts it. Tested in test_count_triggers_deny.
  • subject_digest non-determinism with multiple algorithms is safe. In all usage contexts (_attests_to_subject, collect_subjects, _subject_matches), Rego explores all possibilities through unification. Documented with "Safe when subjects have one digest algorithm."
  • Subject mismatch validation is a good security addition. Prevents attestation substitution — a test attestation produced for image A cannot satisfy requirements for image B.
  • Rule data validation is thorough. JSON Schema constraints ensure configured values are in the valid enum set, preventing silent policy bypass via invalid configuration.

Test Coverage

Comprehensive coverage including:

  • All six new rules tested individually
  • Edge cases: zero counts, missing subjects, count-only triggers (no result string match), empty strings
  • Informative test demotion verified (warns instead of denies)
  • Rule data validation — both valid and invalid inputs tested
  • Custom rule data overrides tested
  • Multi-attestation and mixed-result scenarios updated for new model

Findings

# Severity File Description
1 low policy/lib/rule_data/rule_data.rego Missing explicit default for informative_test_attestations in the defaults map. While rule_data.get safely falls back to [] (line 190–192), adding an explicit "informative_test_attestations": [] entry with a comment alongside the other test attestation keys would improve discoverability and make the empty-by-default semantics explicit.
2 low policy/release/test_attestation/test_attestation.rego Message format change from listing specific test names ("failed tests CVE-2024-1234, CVE-2024-5678") to counts ("failures: 2") is a breaking change for any downstream consumers that parse the msg field. The code field is the stable programmatic identifier, so this is unlikely to cause issues in well-designed integrations, but worth noting in release notes.
Previous run (2)

Review

Verdict: Approve

Summary

This PR fixes trust verification to discover Tekton Chains provenance via cosign .att tags instead of OCI referrers, and substantially enhances the test_attestation policy with configurable result types, new result categories (ERROR/SKIPPED), informative test support, subject-mismatch validation, and rule data validation. Changes are well-structured and comprehensively tested.

Changes reviewed

Area Files Assessment
Trust verification policy/lib/intoto/trust.rego ✅ Correct: removes OCI referrer indirection, calls ec.sigstore.verify_attestation directly on referrer ref, switches to parsed_blob_from_image for cosign tag OCI layout
Shared helpers policy/lib/intoto/intoto.rego ✅ New public subject_digests/subject_digest helpers consolidate duplicated logic
DRY refactor policy/release/slsa_build_scripted_build/slsa_build_scripted_build.rego ✅ Replaces local subject_digest with shared intoto.subject_digest
Test attestation policy policy/release/test_attestation/test_attestation.rego ✅ Major feature expansion with proper patterns
Rule data defaults policy/lib/rule_data/rule_data.rego ✅ New defaults follow existing vocabulary conventions
Tests *_test.rego files ✅ Comprehensive coverage for all new rules and edge cases
Docs antora/docs/ ✅ Documentation updated to match new rules

Key observations

  1. Trust model change is correct: Chains stores SLSA provenance as cosign .att tags (not OCI referrers). The fix to call ec.sigstore.verify_attestation directly on the statement referrer ref, and to use parsed_blob_from_image (which reads the first layer from the image manifest), correctly handles this storage model.

  2. _has_result dual-path design: The helper fires when either the result string matches OR a positive count field exists. Test test_count_triggers_deny confirms this is intentional — a PASSED result with failures: 5 still triggers a denial. This is a reasonable defense-in-depth approach.

  3. Informative tests exclusion: The no_failed_tests deny rule correctly excludes tests listed in informative_test_attestations using not _test_name(statement) in {t | some t in rule_data.get("informative_test_attestations")}, while the no_failed_informative_test_attestations warn rule captures them.

  4. Subject mismatch validation: The new subject_mismatch rule validates that attestation subjects contain the evaluated image's digest. The guard img_digest != "" prevents false positives when the image ref has no digest.

  5. Rule data validation: The JSON schema in _rule_data_errors restricts status values to the known enum ["PASSED", "FAILED", "WARNED", "ERROR", "SKIPPED"], while informative_test_attestations accepts any string array. This is appropriate since test names are user-defined.

Minor notes (non-blocking)

  • The removal of public slsa_build_scripted_build.subject_digest is technically a breaking change if any external consumer imports it. Given it's internal policy code and the function is now available as lib.intoto.subject_digest, this is acceptable.
  • The .github/workflows/release.yaml change adding GEMINI_CLI_TRUST_WORKSPACE is unrelated to the main fix but harmless.
  • The erred/skipped rules pass "n/a" as the count key to _has_result since those result types have no corresponding count field in the predicate spec. This is clearly commented.

All new rules are added to the redhat collection and rule_data_provided additionally to policy_data. Test coverage appears comprehensive across happy paths, edge cases, and the new feature matrix.


Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

  • .github/workflows/release.yaml
Previous run (3)

Review — request-changes

Summary

The PR correctly addresses EC-1982 by switching trust verification from OCI referrer-based provenance discovery to cosign-tag-based discovery, and swapping parsed_blob for parsed_blob_from_image so the in-toto statement is extracted from the OCI artifact's layer rather than treating the manifest as the blob. The semantic change aligns with how Tekton Chains actually stores SLSA provenance.

However, the tests in trust_test.rego are not updated to reflect either change. The parsed_blob_from_image function calls ec.oci.image_manifest internally, and no test provides a mock for that built-in. This will break every test that expects verified_statements to return results, violating the project's 100% coverage requirement.


Findings

1 · Tests broken: missing ec.oci.image_manifest mock — high / correctness

File: policy/lib/intoto/trust_test.rego

oci.parsed_blob_from_image(ref) (in oci.rego:11-17) internally calls ec.oci.image_manifest(ref) to fetch the OCI manifest, then extracts the first layer digest to build a blob reference. None of the tests in trust_test.rego mock ec.oci.image_manifest. In OPA, an unmocked custom built-in causes the rule body to fail (undefined), so parsed_blob_from_image silently returns no value.

Affected tests (expect count ≥ 1, will get 0):

  • test_verified_statement_happy_path
  • test_multiple_statements_mixed
  • test_verified_statements_by_predicate
  • test_existential_attestation_matching

Every test that exercises the positive path through verified_statements is broken. The project enforces 100% coverage (make test / CI), so this PR cannot merge as-is.

Remediation: Add a manifest mock (e.g. _mock_image_manifest(_) := {"layers": [{"digest": "sha256:layer001..."}]}) and pass it via with ec.oci.image_manifest as _mock_image_manifest in every test that calls verified_statements. The ec.oci.blob mock (_mock_blob) must also handle the newly-constructed blob ref (<repo>@<layer_digest>) — the existing catch-all _mock_blob(_) signature handles this, but tests like _mock_blob_multi that match on ref substrings will need updating.

2 · Zero-count tests pass for wrong reasons — medium / correctness

File: policy/lib/intoto/trust_test.rego

Several tests that assert count(result) == 0 still pass, but not for the reason they intend:

  • test_no_provenance_referrers — does not mock ec.sigstore.verify_attestation, so _has_trusted_provenance fails because the verify built-in is undefined — not because "no provenance referrers exist" (the old reason). The old code path ec.oci.image_referrers(referrer.ref) is no longer called.
  • test_unrecognized_statement_type and test_malformed_blob_has_trusted_provenance succeeds, but parsed_blob_from_image fails (no manifest mock) before the _type / JSON-parse check is reached. These tests no longer validate what their names claim.

This is not a runtime bug, but it means the test suite has silent coverage gaps. When the manifest mock is added (per finding 1), these tests will need re-examination to ensure each one fails for the intended reason.

Remediation: After adding the manifest mock, audit each zero-count test: verify that removing the specific condition under test (e.g., removing the verify mock, using a recognized type) flips the result. Remove or update the _mock_referrers_no_provenance helper, which mocks the second-level referrer lookup that no longer occurs.

3 · Stale trust-model comment in file header — medium / documentation

File: policy/lib/intoto/trust.rego (lines 28–31)

The header comment says:

"A statement is verified if ANY provenance referrer attached to the statement referrer provides valid Sigstore-attested SLSA provenance..."

This describes the old two-level OCI-referrer chain. The new code verifies provenance via cosign tags on the statement referrer itself — there is no longer a "provenance referrer attached to the statement referrer." The comment should reflect the cosign-tag discovery model.

Remediation: Update the comment to describe the current model, e.g.: "A statement is verified if the statement referrer has valid Sigstore-attested SLSA provenance (discovered via cosign tags) with all tasks trusted."

@github-actions github-actions Bot added size: L and removed size: XS labels Jul 8, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 8, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 8:27 PM UTC · Ended 8:33 PM UTC
Commit: 8f05d87 · View workflow run →

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
unit-tests 100.00% <100.00%> (ø)

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

Files with missing lines Coverage Δ
policy/lib/intoto/intoto.rego 100.00% <100.00%> (ø)
policy/lib/intoto/intoto_test.rego 100.00% <100.00%> (ø)
policy/lib/intoto/trust.rego 100.00% <100.00%> (ø)
policy/lib/intoto/trust_test.rego 100.00% <100.00%> (ø)
policy/lib/rule_data/rule_data.rego 100.00% <ø> (ø)
...uild_scripted_build/slsa_build_scripted_build.rego 100.00% <100.00%> (ø)
...icy/release/test_attestation/test_attestation.rego 100.00% <100.00%> (ø)
...elease/test_attestation/test_attestation_test.rego 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 8, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:33 PM UTC · Completed 8:38 PM UTC
Commit: 8f05d87 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 8, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
policy/release/test_attestation/test_attestation.rego (1)

46-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Count-based trigger isn't documented in rule metadata.

_has_result's second clause fires purely from a count field (failures/warnings > 0), independent of predicate.result and independent of the configured results list (confirmed by test_count_triggers_deny, which denies a PASSED-result statement due to failures: 5). None of the three rule descriptions here (no_failed_informative_test_attestations, no_test_warnings, no_failed_tests) mention this fallback — they only describe the configurable result-string match. Worth a one-line addition to each description so users/auditors understand why a rule can fire even when result looks passing.

Also applies to: 63-93, 94-121, 122-151

🤖 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 `@policy/release/test_attestation/test_attestation.rego` around lines 46 - 61,
The rule metadata descriptions for the test attestation policies do not mention
that `_has_result` can also match via count fields like failures or warnings,
even when predicate.result is passing. Update the descriptions for
no_failed_informative_test_attestations, no_test_warnings, and no_failed_tests
in test_attestation.rego to explicitly document this fallback behavior so the
count-based trigger is discoverable.
policy/lib/intoto/intoto_test.rego (1)

109-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a multi-algorithm regression test for subject_digest (singular).

Current tests cover subject_digest only with single-algorithm maps and subject_digests with a multi-algorithm map, but never subject_digest with a multi-algorithm map — the exact input that triggers the eval-conflict issue flagged in policy/lib/intoto/intoto.rego (Lines 57-60).

🤖 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 `@policy/lib/intoto/intoto_test.rego` around lines 109 - 121, Add a regression
test for intoto.subject_digest using a digest map with multiple algorithms,
since only single-algorithm cases are currently covered and that is the path
that exposes the eval-conflict behavior. Extend the existing intoto_test.rego
tests near test_subject_digest and test_subject_digests_multi_algorithm so
subject_digest is asserted against a multi-algorithm {"digest": {...}} input,
and verify the expected result or conflict behavior matches the intended
contract.
🤖 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_test_attestation.adoc`:
- Around line 98-100: Update the `test_data_found` solution text in the release
test attestation docs so it only states that the predicate must include a
`result` field. Remove the mention of recognized values like
PASSED/WARNED/FAILED, since that guidance belongs to `test_result_known`; keep
the wording aligned with the existing `test_data_found` rule and its surrounding
documentation.

In `@policy/lib/intoto/intoto.rego`:
- Around line 51-60: `subject_digest` in intoto.rego currently uses `some digest
in subject_digests(subject)`, which can become ambiguous for subjects with
multiple digest algorithms and cause evaluation conflicts. Update
`subject_digest` to either require exactly one entry from
`subject_digests(subject)` or select a digest deterministically, and make sure
the caller in `slsa_build_scripted_build.rego` only relies on the safe
single-digest behavior.

---

Nitpick comments:
In `@policy/lib/intoto/intoto_test.rego`:
- Around line 109-121: Add a regression test for intoto.subject_digest using a
digest map with multiple algorithms, since only single-algorithm cases are
currently covered and that is the path that exposes the eval-conflict behavior.
Extend the existing intoto_test.rego tests near test_subject_digest and
test_subject_digests_multi_algorithm so subject_digest is asserted against a
multi-algorithm {"digest": {...}} input, and verify the expected result or
conflict behavior matches the intended contract.

In `@policy/release/test_attestation/test_attestation.rego`:
- Around line 46-61: The rule metadata descriptions for the test attestation
policies do not mention that `_has_result` can also match via count fields like
failures or warnings, even when predicate.result is passing. Update the
descriptions for no_failed_informative_test_attestations, no_test_warnings, and
no_failed_tests in test_attestation.rego to explicitly document this fallback
behavior so the count-based trigger is discoverable.
🪄 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: 66c4af4b-1580-4a1b-989d-05ed07d46865

📥 Commits

Reviewing files that changed from the base of the PR and between a4348f6 and 6e81e63.

📒 Files selected for processing (13)
  • .github/workflows/release.yaml
  • antora/docs/modules/ROOT/pages/packages/release_slsa_build_scripted_build.adoc
  • antora/docs/modules/ROOT/pages/packages/release_test_attestation.adoc
  • antora/docs/modules/ROOT/pages/release_policy.adoc
  • antora/docs/modules/ROOT/partials/release_policy_nav.adoc
  • policy/lib/intoto/intoto.rego
  • policy/lib/intoto/intoto_test.rego
  • policy/lib/intoto/trust.rego
  • policy/lib/intoto/trust_test.rego
  • policy/lib/rule_data/rule_data.rego
  • policy/release/slsa_build_scripted_build/slsa_build_scripted_build.rego
  • policy/release/test_attestation/test_attestation.rego
  • policy/release/test_attestation/test_attestation_test.rego
✅ Files skipped from review due to trivial changes (1)
  • antora/docs/modules/ROOT/pages/packages/release_slsa_build_scripted_build.adoc
🚧 Files skipped from review as they are similar to previous changes (1)
  • policy/lib/intoto/trust_test.rego

Comment thread policy/lib/intoto/intoto.rego
robnester-rh and others added 3 commits July 8, 2026 16:01
…kage

Add configurable result values, ERROR/SKIPPED handling, informative
test support, subject digest validation, and rule data schema
validation to the test_attestation package, matching the capabilities
of the test package for OCI-referrer-based test result attestations.

- AC-1: Replace hardcoded result checks with rule_data.get lookups
  using predicate spec vocabulary (PASSED/FAILED/WARNED/ERROR/SKIPPED)
- AC-2: Add no_erred_test_attestations deny rule
- AC-3: Add no_skipped_test_attestations deny rule
- AC-4: Add informative_test_attestations support (warn not deny)
- AC-5: Add subject_mismatch deny rule with ANY-match semantics
- AC-6: Add rule_data_provided schema validation for all 6 keys
- AC-7: 9 new tests covering all acceptance criteria

Moves subject_digest from private in trust.rego to public in
intoto.rego to avoid reimplementing digest format conversion.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The attest-test-result step action produces integer count fields
(failures, warnings, successes) not string arrays (failedTests,
warnedTests, passedTests). Update the policy rules to match:

- Replace _test_list (string array extraction) with _count_detail
  (integer count formatting) and _has_result (dual check: result
  string match OR count > 0, following test.rego's _did_result)
- Update all mock test data to use the step action's predicate
  structure
- Quote failure_msg values containing colons for YAML compatibility

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
policy/lib/intoto/trust.rego (1)

67-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Simplify to satisfy Regal's superfluous object.get lint.

The default false is equivalent to a plain reference here (a missing success key makes the rule fail either way, so it stays fail-closed). Regal flags this at error level, which may break the lint stage.

♻️ Proposed change
-	object.get(verification, "success", false) == true
+	verification.success == true
🤖 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 `@policy/lib/intoto/trust.rego` at line 67, The trust.rego rule is using a
redundant object.get check for verification success, which triggers Regal’s
superfluous object.get lint. Update the expression in the policy rule to
reference the success field directly from verification instead of calling
object.get, keeping the same fail-closed behavior. Use the existing verification
predicate in the trust policy to locate and simplify the condition.

Source: Linters/SAST tools

🤖 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.

Nitpick comments:
In `@policy/lib/intoto/trust.rego`:
- Line 67: The trust.rego rule is using a redundant object.get check for
verification success, which triggers Regal’s superfluous object.get lint. Update
the expression in the policy rule to reference the success field directly from
verification instead of calling object.get, keeping the same fail-closed
behavior. Use the existing verification predicate in the trust policy to locate
and simplify the condition.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: c419969c-9ce5-472a-ba3e-4ddb6228726c

📥 Commits

Reviewing files that changed from the base of the PR and between 6e81e63 and 24753a3.

📒 Files selected for processing (12)
  • antora/docs/modules/ROOT/pages/packages/release_slsa_build_scripted_build.adoc
  • antora/docs/modules/ROOT/pages/packages/release_test_attestation.adoc
  • antora/docs/modules/ROOT/pages/release_policy.adoc
  • antora/docs/modules/ROOT/partials/release_policy_nav.adoc
  • policy/lib/intoto/intoto.rego
  • policy/lib/intoto/intoto_test.rego
  • policy/lib/intoto/trust.rego
  • policy/lib/intoto/trust_test.rego
  • policy/lib/rule_data/rule_data.rego
  • policy/release/slsa_build_scripted_build/slsa_build_scripted_build.rego
  • policy/release/test_attestation/test_attestation.rego
  • policy/release/test_attestation/test_attestation_test.rego
✅ Files skipped from review due to trivial changes (3)
  • antora/docs/modules/ROOT/pages/packages/release_slsa_build_scripted_build.adoc
  • policy/lib/intoto/intoto_test.rego
  • antora/docs/modules/ROOT/pages/packages/release_test_attestation.adoc
🚧 Files skipped from review as they are similar to previous changes (8)
  • antora/docs/modules/ROOT/pages/release_policy.adoc
  • policy/release/slsa_build_scripted_build/slsa_build_scripted_build.rego
  • policy/lib/intoto/intoto.rego
  • policy/lib/rule_data/rule_data.rego
  • antora/docs/modules/ROOT/partials/release_policy_nav.adoc
  • policy/release/test_attestation/test_attestation.rego
  • policy/lib/intoto/trust_test.rego
  • policy/release/test_attestation/test_attestation_test.rego

@joejstuart joejstuart changed the title fix(EC-1982): discover Chains provenance via cosign tags in trust verification feat(EC-1950,EC-1982): test attestation parity and trust verification fixes Jul 8, 2026
robnester-rh
robnester-rh previously approved these changes Jul 9, 2026
Comment thread policy/release/test_attestation/test_attestation.rego Outdated
Comment thread policy/release/test_attestation/test_attestation.rego Outdated
Comment thread policy/release/test_attestation/test_attestation_test.rego Outdated
Comment thread policy/release/test_attestation/test_attestation_test.rego Outdated
Comment thread policy/release/test_attestation/test_attestation_test.rego
Comment thread antora/docs/modules/ROOT/pages/packages/release_test_attestation.adoc Outdated
Comment thread policy/release/test_attestation/test_attestation.rego
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:02 PM UTC · Completed 6:07 PM UTC
Commit: 87c4a29 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Jul 9, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 6:15 PM UTC · Ended 6:17 PM UTC
Commit: 87c4a29 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 6:18 PM UTC · Ended 6:20 PM UTC
Commit: 87c4a29 · View workflow run →

robnester-rh and others added 9 commits July 9, 2026 13:19
OPA strict mode requires unused arguments to use _ (wildcard).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add doc comment on intoto.subject_digest
- Add test for missing subject field triggering subject_mismatch

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Change subject_digest to set-based subject_digests to handle
  multi-algorithm subjects without runtime conflicts
- Add empty-digest guard to subject_mismatch rule for tag-only refs
- Use subject_digests in _subject_matches for correct set membership
- Add test for multi-algorithm subject digests

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ification

Chains stores SLSA provenance as cosign-tagged artifacts (.att suffix),
not as OCI referrers. _has_trusted_provenance was using
ec.oci.image_referrers to find provenance, which never found it. Now
calls ec.sigstore.verify_attestation directly on the referrer ref, which
uses cosign's tag-based discovery.

Also fixes parsed_blob -> parsed_blob_from_image so the in-toto statement
content is extracted from the OCI artifact layer rather than returning the
manifest JSON.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-based trust

Tests now mock ec.oci.image_manifest (used by parsed_blob_from_image to
traverse manifest → layer → blob) and match verify_attestation calls on
statement refs instead of provenance refs (since _has_trusted_provenance
now calls verify_attestation directly rather than looking up provenance
via image_referrers). Removed dead provenance referrer mock code that
was no longer exercised.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address PR review feedback: remove unnecessary array-to-set conversions
in `in` expressions since Rego handles both identically, and add comments
explaining the zero-count and empty-string edge case tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The zero-count tests exercised the same result-string clause of
_has_result already covered by the basic warned/failed tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:21 PM UTC · Completed 6:28 PM UTC
Commit: 87c4a29 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge feature enhancement New feature or request and removed ready-for-merge All reviewers approved — ready to merge labels Jul 9, 2026
@joejstuart joejstuart requested a review from simonbaird July 9, 2026 19:35
@joejstuart joejstuart merged commit da5071a into conforma:main Jul 9, 2026
15 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 7:53 PM UTC · Completed 7:58 PM UTC
Commit: 87c4a29 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro Analysis: PR #1770 — test attestation parity and trust verification fixes

Originating PR: #1770

Context limitations

GitHub API access was unavailable during this analysis (403), so the review conversation, CI results, iteration count, and agent/human review comments could not be examined. Analysis is based solely on the merged code. The repo clone contains only the merge commit, making diff-based analysis of the PR branch impossible.

Observations

This PR (feat(EC-1950,EC-1982)) adds a new test_attestation package with feature parity to the existing test package, plus a trust verification layer (lib/intoto/trust.rego) for in-toto attestation chain-of-trust. The code is well-structured with comprehensive test coverage (enforced at 100%) and good inline documentation.

The merged codebase shows two architectural patterns that would benefit from better documentation for future agent work:

  1. Dual test result pathstest.rego (pipeline task results via TEST_OUTPUT) vs test_attestation.rego (in-toto attestations verified via Sigstore/SLSA provenance). These must remain in sync for feature parity.
  2. Trust verification chaintrust.rego couples lib.intoto, lib.sigstore, lib.tekton, and OCI builtins. Changes anywhere in this chain require understanding the full trust model.

Proposals

1 proposal filed — see below.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request feature ready-for-merge All reviewers approved — ready to merge size: XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants