Allow PNC SLSA build type for PipelineRun attestations

[vnadempa]

What:

The goal of this change is to make Conforma work with the PNC SLSA v1 attestation build type.

This is a targeted compatibility change. It allows the PNC build type to be recognized as a PipelineRun attestation and adds a regression test for this specific attestation format.

This change does not make all rules defined in this repository automatically work with the PNC attestation. Additional rules may require separate future work, either in this repository or through custom Enterprise Contract definitions, where applicable.

• Added the PNC SLSA v1 build type to the allowed provenance build types.
• Added a regression test for the PNC SLSA v1 attestation.

Why:

PNC produces an SLSA v1 attestation using the PNC-specific build type:

https://project-ncl.github.io/slsa-pnc-cli-buildtypes/workflow/v1

The change is needed so that Conforma recognizes this attestation as a supported PipelineRun attestation.

The existing attestation_type.known_attestation_type rule already recognizes the PNC attestation's _type, so this change does not modify that rule.

Testing

• PNC-specific regression test: PASS
• attestation_type.known_attestation_type: PASS
• Full policy suite: 1070/1071 tests pass.
• The remaining lib.json_test.test_validate_args failure is also present on the clean baseline and is unrelated to this change.

Tickets:

MMENG-4662

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 72aff7cb-0ad2-47b3-a046-9ee144e907d1

📥 Commits

Reviewing files that changed from the base of the PR and between 9030faa and 59955a0.

📒 Files selected for processing (1)

• policy/release/lib/attestations_test.rego

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The test suite adds a PNC SLSA v1 attestation fixture and verifies that allowed_provenance_build_types selects it unchanged.

Changes

PNC attestation coverage

Layer / File(s)	Summary
PNC fixture and selection test policy/release/lib/attestations_test.rego	Adds a non-Tekton PNC SLSA v1 fixture and tests selection through allowed_provenance_build_types.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 59955

This change enables recognition of the PNC SLSA build type with a targeted regression test; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely states that the PNC SLSA build type is allowed for PipelineRun attestations.
Description check	✅ Passed	The description includes the required What, Why, and Tickets sections. It explains the compatibility change, its context, the regression test, and the reported full-suite result.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:11 AM UTC · Completed 3:24 AM UTC

Commit: 87c4a29 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [intent-vs-implementation-mismatch] policy/release/lib/attestations_test.rego — The PR title ("Allow PNC SLSA build type for PipelineRun attestations") and description ("Added the PNC SLSA v1 build type to the allowed provenance build types") imply a production behavior change, but the diff only adds a test. The PNC build type is not added to the default allowed_provenance_build_types list in policy/lib/rule_data/rule_data.rego. The test injects the PNC type via with data.rule_data__configuration__, confirming only that the existing configuration-based mechanism works for this specific attestation shape. The PR title/description may mislead future readers into believing PNC support is built in when it must still be configured externally via rule_data.
  Remediation: Either (1) update the PR title and description to accurately reflect that this is a regression test for PNC attestation compatibility when configured externally, or (2) if the intent is to allow PNC by default, add the PNC build type URL to the defaults list in policy/lib/rule_data/rule_data.rego.

Low

• [test-adequacy] policy/release/lib/attestations_test.rego:481 — The new test has more value than a pure duplicate of test_custom_v1_build_type. It calls lib.pipelinerun_attestations (the high-level entry point including timestamp-based latest-attestation selection), while the existing test_custom_v1_build_type calls lib.pipelinerun_slsa_provenance_v1 directly. The new test also includes a realistic PNC fixture with finishedOn, resolvedDependencies, and PNC-specific externalParameters fields, exercising the _build_finished_on timestamp extraction and _is_pipelinerun_v1 guard with a structurally distinct attestation. However, the overlap in what is being proven (a non-Tekton build type works when configured) is still significant.

Previous run

Review

Findings

Low

• [test adequacy] policy/release/lib/attestations_test.rego:484 — The new test test_pipelinerun_attestations_pnc_build_type only tests the positive case (PNC build type is in the allowed list and the attestation is returned). It does not test the negative case where the PNC build type is NOT in the allowed list, verifying the attestation is rejected. The analogous existing tests test_custom_v02_build_type and test_custom_v1_build_type both test the negative case first before the positive case. Adding the negative assertion would make this regression test more robust.
  Remediation: Add an assertion before the positive test that verifies lib.pipelinerun_attestations returns an empty list when data.rule_data__configuration__ does not include the PNC build type, following the pattern in test_custom_v1_build_type.

Previous run (2)

Review

Findings

Low

• [pattern-inconsistency] policy/lib/rule_data/rule_data.rego:181 — The newly added build type URL uses spaces for indentation instead of tabs, which is inconsistent with every other entry in the allowed_provenance_build_types list and the file's convention. Run make fmt to auto-fix the indentation.

---

Labels: PR adds support for a new SLSA provenance build type (PNC)

[st3penta]

Hi @vnadempa, i get your intentions here, but adding this build type to the default will enable it for everyone, and it is not ideal imho.
Do you know you can provide custom rule_data to your conforma setup? See here: https://conforma.dev/docs/cli/configuration.html#_data_sources.
This way you can enable the build_type without having to change these defaults

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:54 PM UTC · Completed 3:05 PM UTC

Commit: 87c4a29 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.03

[simonbaird]

I think it's fine, but do you want to squash it into a single commit?

[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/release/lib/attestations_test.rego	100.00% <100.00%> (ø)

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:18 AM UTC · Completed 8:31 AM UTC

Commit: 87c4a29 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.62

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:21 AM UTC · Completed 3:33 AM UTC

Commit: 87c4a29 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.68

[vnadempa]

I think it's fine, but do you want to squash it into a single commit?

Done — I squashed the two commits into a single commit and pushed the update. The checks are running again.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 3:28 AM UTC · Completed 3:43 AM UTC

Commit: 87c4a29 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.86

[vnadempa]

Hi @vnadempa, i get your intentions here, but adding this build type to the default will enable it for everyone, and it is not ideal imho. Do you know you can provide custom rule_data to your conforma setup? See here: https://conforma.dev/docs/cli/configuration.html#_data_sources. This way you can enable the build_type without having to change these defaults

Thanks for pointing this out. You're right — adding the PNC build type to the global default would enable it for everyone, which isn't what we want.

I updated the change to remove the PNC build type from the default provenance allowlist and instead explicitly provide it through rule_data for the relevant Conforma setup.

I also added/updated the test to verify that the PNC build type is recognized when supplied through rule_data, while keeping it excluded from the defaults. The full test suite passes (1071/1071).

Thanks for the guidance!

[fullsend-ai-review]

Review skipped — this PR is already closed.

The /fs-review command only reviews open PRs/MRs.

_{Posted by fullsend post-review check}

[fullsend-ai-retro]

Retro: PR #1816 — Allow PNC SLSA build type for PipelineRun attestations

Timeline

1. Aug 19 03:10 — vnadempa opens PR adding PNC SLSA v1 build type to global defaults in rule_data.rego + regression test.
2. Aug 19 03:24 — Review agent run 1 approves, applies ready-for-merge. Finds only a [low] formatting inconsistency (spaces vs tabs).
3. Aug 19 12:53 — Human reviewer st3penta catches the key design issue: adding a vendor-specific build type to global defaults enables it for all Conforma users. Recommends using custom rule_data per-deployment config instead.
4. ~Aug 25 — Author removes the production code change per st3penta's feedback, leaving only the test file.
5. Aug 25 15:04 — Review agent run 2 approves test-only diff, flags [low] missing negative test case.
6. Aug 25 18:29 — simonbaird (member) responds to the test-adequacy finding: "I don't think it's necessary." Suggests squashing commits.
7. Aug 26 08:31 — Author force-pushes squash. Review agent run 3 re-reviews same code, finds [medium] intent-vs-implementation-mismatch (PR title still implies production change but diff is test-only). Removes ready-for-merge, applies requires-manual-review. Cost: $3.62.
8. Aug 27 03:27 — PR closed without merge. Review agent run 4 was mid-execution; post-review script correctly skipped actions on the closed PR.

Total estimated review cost: ~$12–13 across 4 runs. Agents repo: fullsend-ai/agents@v0.37.0.

Key Findings

The review agent missed the most important issue on this PR. The critical feedback — that a vendor-specific (PNC) build type should not be added to the universal defaults list — came from a human reviewer 10 hours after the agent approved and labeled the PR ready-for-merge. The agent had the contextual signals to detect the pattern break: all 5 existing entries in allowed_provenance_build_types are Tekton-specific URLs, and the new PNC entry uses a completely different domain. However, AGENTS.md has zero guidance about the rule_data defaults mechanism, the 4-tier priority hierarchy (rule_data__configuration__ > rule_data_custom > rule_data > defaults), or inclusion criteria for global defaults.

The agent's intent-vs-implementation-mismatch finding (run 3) was valid but late. It correctly flagged that the PR metadata was stale after the scope change, but this observation came 7 days after st3penta had already caused the scope change. It did appropriately trigger the requires-manual-review label.

The test-adequacy finding was low-value. simonbaird correctly dismissed it — the negative case for custom build types is already covered by test_custom_v1_build_type. The agent was pattern-matching against test structure without assessing existing coverage.

Existing Issue Coverage

• Premature ready-for-merge labeling is partially addressed by Review post-script should not apply ready-for-merge label while a human reviewer has outstanding CHANGES_REQUESTED fullsend-ai/fullsend#5065 (don't apply when human has outstanding changes requested) and #5143 (extend draft-PR label gate). These focus on different triggers than the domain-knowledge gap here.
• Redundant dispatch on closed PRs is covered by Skip review dispatch when PR is already closed or merged fullsend-ai/fullsend#3010, #1870, feat(scripts): add ec- prefix to enterprise-contract repository names #1439. Run 4's case (PR closed during execution) was handled correctly by the post-review script.
• Rego/OPA paradigm guidance in Add Rego/OPA paradigm guidance to AGENTS.md to reduce false-positive review findings #1757 addresses a different class of false positives (imperative pattern suggestions) and does not cover the rule_data defaults mechanism.

Proposals filed

• Document rule_data defaults inclusion criteria and priority hierarchy in AGENTS.md (in conforma/policy)