feat(EC-1950): bring test_attestation to feature parity with test package

[robnester-rh]

Summary

Brings the test_attestation package to feature parity with the test package so teams adopting OCI-referrer test result attestations get the same policy rigor as those using embedded TEST_OUTPUT task results.

• AC-1: Configurable result values via rule data (predicate spec vocabulary: PASSED/FAILED/WARNED/ERROR/SKIPPED)
• AC-2: no_erred_test_attestations deny rule for ERROR results
• AC-3: no_skipped_test_attestations deny rule for SKIPPED results
• AC-4: Informative test support — failed informative tests warn instead of deny
• AC-5: Subject digest validation — deny if attestation subject doesn't match the image being evaluated (ANY-match semantics)
• AC-6: Rule data schema validation for all 6 new keys
• AC-7: 9 new tests covering all ACs

Predicate alignment

Updated to match the actual attest-test-result step action output which uses integer count fields (failures, warnings, successes) rather than string arrays (failedTests, warnedTests). The _has_result helper checks both predicate.result (string match) and count fields (predicate[key] > 0), following test.rego's _did_result pattern.

Test plan

• 996/996 tests pass (make quiet-test)
• Regal lint clean
• make conventions-check (needs CI)
• Review predicate format against step action with @joejstuart

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 753e4a65-199f-4659-beba-bea8fac6bf45

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[codecov]

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/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]

🤖 Review · ⚠️ Cancelled · Started 2:03 PM UTC · Ended 2:09 PM UTC
Commit: 47d3320 · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 2:12 PM UTC · Ended 2:20 PM UTC
Commit: 47d3320 · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 2:23 PM UTC · Ended 2:30 PM UTC
Commit: 47d3320 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:32 PM UTC · Completed 2:51 PM UTC
Commit: 47d3320 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [logic-error] policy/lib/intoto/intoto.rego:53 — The subject_digest function uses := (complete rule assignment) with some algorithm, value in subject.digest. If a subject contains multiple digest algorithms (e.g., both sha256 and sha512), OPA will produce a runtime conflict error because a complete rule can only yield a single value per input. This is pre-existing behavior (identical to the former _subject_digest in trust.rego and slsa_build_scripted_build.rego), but the new subject_mismatch deny rule adds another consumer, making this a good time to address it.
  Remediation: Consider changing subject_digest to a partial rule that returns a set (subject_digests(subject) contains digest if { ... }), or document the single-algorithm-per-subject invariant and add a guard.

Low

• [edge-case] policy/release/test_attestation/test_attestation.rego — The _has_result function has two overloads: one matching on the result string field, the other on a numeric count key > 0. For no_erred_test_attestations and no_skipped_test_attestations, the count_key is "n/a" (disabling count matching), while for no_failed_tests and no_test_warnings, count matching is active. This means an attestation with result: "PASSED" but failures: 5 triggers deny. This is tested (test_count_triggers_deny) and appears intentional (fail-closed), but the rule metadata description could be more precise about the dual-trigger semantics.

• [missing-test] policy/release/test_attestation/test_attestation_test.rego — No tests verify the no_skipped_test_attestations or no_erred_test_attestations rules when a test is in the informative list. Unlike no_failed_tests which explicitly excludes informative tests via the not _test_name(statement) in {...} guard, these deny rules do not exclude informative tests. An informative test that errors or is skipped will still produce a deny violation. Tests should verify this asymmetry is intentional.

• [edge-case] policy/release/test_attestation/test_attestation.rego — The subject_mismatch rule calls image.parse(input.image.ref) and accesses .digest. If input.image.ref has no digest (e.g., registry.io/repo/image:tag), img_digest will be an empty string, causing a subject_mismatch violation for every test attestation since no subject digest can equal "". While conforma typically operates with digest-pinned refs, adding a guard img_digest != "" would be defensive.

• [fail-open] policy/release/test_attestation/test_attestation.rego — All result classification values (failed_test_attestation_results, etc.) are configurable via rule_data. Setting any to [] disables the corresponding deny rule. The schema validation checks valid enum members and uniqueItems but does not enforce minItems. This mirrors the existing pattern in test.rego and requires privileged access to rule_data to exploit, but adding minItems: 1 would prevent accidental policy disabling.

Previous run

Review

Findings

Medium

• [breaking-api] policy/release/test_attestation/test_attestation.rego — The failure_msg for no_failed_tests and no_test_warnings changed format: "failed tests CVE-2024-1234, CVE-2024-5678" → "failures: 2" and "warned tests deprecated-api-v1" → "warnings: 1". Downstream systems that parse these human-readable violation messages will see different output. Consumers matching only on the violation code field are unaffected.
  Remediation: Document the message format change in release notes.

• [breaking-schema] policy/release/test_attestation/test_attestation.rego — The policy no longer reads failedTests/warnedTests string arrays from the attestation predicate. It now reads numeric failures/warnings count fields. The PR description acknowledges this aligns with actual attest-test-result step action output. Attestation producers still using the old schema will see degraded detail in messages ("failures: 0") but the result-string matching path still catches FAILED/WARNED results, so enforcement is preserved. See also: [scope-creep] finding below.
  Remediation: Coordinate with attestation producers to confirm migration to numeric count fields.

• [breaking-api] policy/release/test_attestation/test_attestation.rego — ERROR and SKIPPED result values previously triggered test_result_known (unsupported result). Now they are accepted as valid and caught by dedicated deny rules (no_erred_test_attestations, no_skipped_test_attestations) with different violation codes. The net enforcement effect is the same (deny), but consumers filtering on specific violation code strings will need updating.
  Remediation: Document the violation code changes in release notes.

• [breaking-api] policy/release/test_attestation/test_attestation.rego — The new subject_mismatch deny rule is net-new enforcement that will reject test attestations whose subject digest does not match the image being evaluated. This could cause previously-passing evaluations to fail if attestation subjects were not precisely matching.

• [breaking-api] policy/release/slsa_build_scripted_build/slsa_build_scripted_build.rego — Public function subject_digest removed from slsa_build_scripted_build and replaced by intoto.subject_digest. Any external consumer importing data.slsa_build_scripted_build.subject_digest will break.
  Remediation: Check if any external consumers reference the old path. If so, add a compatibility alias or coordinate the migration.

• [scope-creep] policy/release/test_attestation/test_attestation.rego — The predicate data model change from string arrays (failedTests, warnedTests) to numeric counts (failures, warnings) alters existing rule semantics. The PR description explicitly acknowledges this alignment with actual step action output. See also: [breaking-schema] finding above.

Low

• [logic-error] policy/release/test_attestation/test_attestation.rego:208 — no_erred_test_attestations and no_skipped_test_attestations pass "n/a" as count_key to _has_result. The count-based clause (object.get(predicate, "n/a", 0)) is dead code since no realistic predicate has a key named "n/a". The rules work correctly via the result-string clause, but a clearer approach would be a dedicated single-clause helper or an empty-string sentinel.

• [missing-test] policy/release/test_attestation/test_attestation_test.rego — No test for a statement with no subject field (malformed attestation), which would trigger subject_mismatch deny via the empty-list fallback in _subject_matches. A direct test would document this edge case behavior.

• [missing-effective-on] policy/release/test_attestation/test_attestation.rego — test.rego uses effective_on dates on some rules to phase them in. The new deny rules (no_erred_test_attestations, no_skipped_test_attestations, subject_mismatch) enforce immediately. Consider whether phased rollout is appropriate.

• [documentation-comment] policy/lib/intoto/intoto.rego — New exported subject_digest function has no documentation comment. Other exported functions in this file have at least a one-line comment.

---

Labels: PR adds new policy rules and configurable features (enhancement) with corresponding Antora documentation updates (documentation).

[robnester-rh]

Fullsend review findings — responses

Medium

• [breaking-api] Message format change — Intentional. Aligned with the actual attest-test-result step action output which produces integer counts (failures, warnings), not string arrays (failedTests, warnedTests). Will document in release notes.

• [breaking-schema] Predicate data model change — Intentional. The previous schema (failedTests/warnedTests arrays) came from a PoC spec example, not from the actual step action. Enforcement is preserved via dual-path _has_result (checks both predicate.result string and count fields).

• [breaking-api] ERROR/SKIPPED violation codes — Intentional per AC-2/AC-3. Operators now get specific short_names to waive (no_erred_test_attestations, no_skipped_test_attestations) instead of the generic test_result_known.

• [breaking-api] subject_mismatch is net-new enforcement — Intentional per AC-5. Closes a security gap where a valid-but-mismatched attestation (produced for image A but attached to image B) would pass. Will discuss phased rollout via effective_on with @joejstuart.

• [breaking-api] subject_digest moved — Moved from slsa_build_scripted_build to intoto.rego to avoid reimplementing digest format conversion. The slsa_build_scripted_build package was updated to use the new location in the same commit. No known external consumers of the old path.

• [scope-creep] — Acknowledged. The predicate model change is part of aligning with the real step action output, not incidental scope expansion.

Low — addressed in d414a97

• [documentation-comment] — Added doc comment on intoto.subject_digest. Fixed.

• [missing-test] — Added test_missing_subject_triggers_mismatch for the no-subject edge case. Fixed.

Low — acknowledged, no change

• [logic-error] "n/a" as count_key — Follows test.rego's _did_result pattern where ERROR and SKIPPED have no corresponding count field. The "n/a" sentinel makes object.get(predicate, "n/a", 0) return 0, so the count clause is a no-op. This is intentional — it keeps the helper signature uniform across all callers.

• [missing-effective-on] — Good point. Will discuss with @joejstuart whether phased rollout via effective_on is appropriate for the new deny rules.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:35 PM UTC · Completed 4:46 PM UTC
Commit: 47d3320 · View workflow run →