feat(EC-1725): Validate proxy URLs against approved patterns in SBOMs#1708
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughAdds enforcement that hermetic build tasks must set the Changes
Sequence Diagram(s)sequenceDiagram
participant Task as Build Task Invocation
participant Att as Attestation/SBOM Producer
participant PE as Policy Engine (Rego)
participant RD as Rule Data (proxy settings, patterns)
Task->>Att: emit invocation params (including enable-hermeto-proxy)
Att->>PE: submit attestation and SBOMs
PE->>RD: read rule_data (proxy_enabled_purl_types, allowed_proxy_url_patterns)
PE->>PE: evaluate hermetic_task rules (is hermetic → check enable-hermeto-proxy)
PE->>PE: evaluate sbom_cyclonedx / sbom_spdx rules (parse purl → check URLs vs patterns)
alt violation
PE->>Att: emit deny result (code, message, term)
else no violation
PE->>Att: no deny / pass
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
policy/release/hermetic_task/hermetic_task_test.rego (1)
436-468: Minor duplication in test fixtures.
_good_attestation_with_proxy(lines 441-455) and_good_attestation(lines 457-471) are now identical since both includeenable-hermeto-proxy: "true". Consider removing_good_attestation_with_proxyand using_good_attestationconsistently, or keeping_good_attestation_with_proxyexplicitly named for test clarity intest_hermetic_task_with_proxy_enabled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@policy/release/hermetic_task/hermetic_task_test.rego` around lines 436 - 468, The two fixtures _good_attestation_with_proxy and _good_attestation are identical (both set enable-hermeto-proxy:"true"); remove the duplicate _good_attestation_with_proxy and update tests (e.g., test_hermetic_task_with_proxy_enabled) to reference _good_attestation instead, or if you prefer explicit naming keep _good_attestation_with_proxy and remove/adjust _good_attestation so each fixture serves a unique purpose; update any references accordingly (search for _good_attestation_with_proxy, _good_attestation, and test_hermetic_task_with_proxy_enabled to ensure consistency).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@acceptance/samples/policy-input-golden-container.json`:
- Line 479: The build-container-arm64 task is marked HERMETIC but is missing the
enable-hermeto-proxy parameter required by the hermeto_proxy_enabled rule;
update the task definition for build-container-arm64 to include
"enable-hermeto-proxy": "true" so it matches other hermetic build tasks and
satisfies the hermeto_proxy_enabled rule.
In `@antora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adoc`:
- Around line 51-52: Update the prose to explicitly document that the rule skips
entries where downloadLocation == "NOASSERTION": state that for packages with a
PURL type in proxy_enabled_purl_types the downloadLocation must match
allowed_proxy_url_patterns, except when downloadLocation is the literal string
"NOASSERTION" (which the rule treats as a skip/no-check); reference the same
configuration names (proxy_enabled_purl_types and allowed_proxy_url_patterns) so
readers can map the behavior to the implementation.
In `@policy/release/sbom_cyclonedx/sbom_cyclonedx.rego`:
- Around line 282-283: The rule currently only allows proxy URLs for the
`redhat` release collection; update the `allowed_proxy_urls` metadata in
sbom_cyclonedx.rego so it also covers other release collections (e.g., add
`redhat_rpms` or broaden the pattern to match non-`redhat` release collections)
so that the CycloneDX proxy URL rule enforces equivalent URLs for both `redhat`
and `redhat_rpms`; modify the `allowed_proxy_urls` entry accordingly and ensure
any references in the policy logic that check collection membership (the same
key used by the proxy URL rule) are consistent with the new entries.
In `@policy/release/sbom_spdx/sbom_spdx.rego`:
- Around line 267-268: The rule metadata for allowed_proxy_urls is currently
scoped only to the "redhat" collection which prevents it from running for
"redhat_rpms" (and thus creates inconsistent coverage); update the rule's
collections metadata (where the commented lines show "collections: - redhat") to
include "redhat_rpms" or broaden/remove the collection restriction so
allowed_proxy_urls applies to the same collections as the adjacent SBOM rules
(ensure you edit the allowed_proxy_urls rule metadata entry).
---
Nitpick comments:
In `@policy/release/hermetic_task/hermetic_task_test.rego`:
- Around line 436-468: The two fixtures _good_attestation_with_proxy and
_good_attestation are identical (both set enable-hermeto-proxy:"true"); remove
the duplicate _good_attestation_with_proxy and update tests (e.g.,
test_hermetic_task_with_proxy_enabled) to reference _good_attestation instead,
or if you prefer explicit naming keep _good_attestation_with_proxy and
remove/adjust _good_attestation so each fixture serves a unique purpose; update
any references accordingly (search for _good_attestation_with_proxy,
_good_attestation, and test_hermetic_task_with_proxy_enabled to ensure
consistency).
🪄 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: Pro
Run ID: 883a1823-31b5-4bc9-9863-fac6b5ad4b1c
📒 Files selected for processing (12)
acceptance/samples/policy-input-golden-container.jsonantora/docs/modules/ROOT/pages/packages/release_hermetic_task.adocantora/docs/modules/ROOT/pages/packages/release_sbom_cyclonedx.adocantora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adocantora/docs/modules/ROOT/pages/release_policy.adocantora/docs/modules/ROOT/partials/release_policy_nav.adocpolicy/release/hermetic_task/hermetic_task.regopolicy/release/hermetic_task/hermetic_task_test.regopolicy/release/sbom_cyclonedx/sbom_cyclonedx.regopolicy/release/sbom_cyclonedx/sbom_cyclonedx_test.regopolicy/release/sbom_spdx/sbom_spdx.regopolicy/release/sbom_spdx/sbom_spdx_test.rego
Member
|
I think the commit titled |
simonbaird
reviewed
Apr 7, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
policy/release/hermetic_task/hermetic_task_test.rego (1)
18-21: Extract shared hermetic+proxy params to reduce fixture drift.The same
HERMETIC=true+enable-hermeto-proxy=trueparameter block is duplicated across many tests. Centralizing it into a shared helper/value will make future policy changes safer and faster.Refactor sketch
+_hermetic_proxy_params := [ + {"name": "HERMETIC", "value": "true"}, + {"name": "enable-hermeto-proxy", "value": "true"}, +] slsav1_task = tekton_test.with_params( _task_base, - [ - {"name": "HERMETIC", "value": "true"}, - {"name": "enable-hermeto-proxy", "value": "true"}, - ], + _hermetic_proxy_params, )As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
Also applies to: 101-104, 110-113, 169-172, 277-280, 327-329, 452-452
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@policy/release/hermetic_task/hermetic_task_test.rego` around lines 18 - 21, Duplicate parameter block [{"name": "HERMETIC", "value": "true"}, {"name": "enable-hermeto-proxy", "value": "true"}] appears in many tests; extract it into a shared variable/helper (e.g., hermeticProxyParams) and reference that helper everywhere instead of inlining the array literal. Update tests that currently include the literal (the occurrences you noted and the block in hermetic_task_test.rego) to use the new shared symbol so future policy changes only need one edit.policy/release/sbom_spdx/sbom_spdx.rego (1)
287-291: Consider handling missing or emptydownloadLocationexplicitly for clarity.The
downloadLocationfield is required in the SPDX schema, so it should always be present in valid SBOMs. However, the current code usesobject.get(pkg, "downloadLocation", "")to default to an empty string, which then proceeds to pattern matching when empty (only NOASSERTION is explicitly skipped). This behavior isn't documented or tested. While likely intentional (strict enforcement for invalid metadata), adding a brief comment or an explicit check would clarify the intent:download_location := object.get(pkg, "downloadLocation", "") -download_location != "NOASSERTION" +download_location != "NOASSERTION" +download_location != ""Or add a comment explaining why empty values should cause a deny.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@policy/release/sbom_spdx/sbom_spdx.rego` around lines 287 - 291, The rule currently defaults download_location := object.get(pkg, "downloadLocation", "") and then runs sbom.url_matches_any_pattern even when empty (except for "NOASSERTION"); update the logic to explicitly handle missing/empty values for clarity: add an explicit check using the download_location symbol (e.g., download_location == "" or download_location == null) and treat it the same as an invalid value (deny) or add a clear comment explaining the intended behavior; adjust the condition that uses sbom.url_matches_any_pattern(download_location, patterns) and NOASSERTION so it only runs when download_location is non-empty, and reference allowed_patterns and parsed_purl.type when locating the pattern lookup to ensure the change is applied in the same block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@antora/docs/modules/ROOT/pages/packages/release_hermetic_task.adoc`:
- Line 21: Update the link target so it points at the actual rule body for
hermetic_task.hermeto_proxy_enabled (the deny implementation) instead of the
current anchor; locate the reference in the docs page that links to the source
and change the fragment/line anchor to the section that contains the deny rule
for hermetic_task.hermeto_proxy_enabled so readers jump directly to the rule
implementation.
In `@policy/release/sbom_cyclonedx/sbom_cyclonedx.rego`:
- Around line 301-303: The CycloneDX check currently evaluates distribution_url
and runs pattern matching even when the value is the SPDX placeholder
"NOASSERTION"; update the conditional around distribution_url (the variable
derived from object.get(reference, "url", "")) to skip pattern checks when
distribution_url == "NOASSERTION" (matching the SPDX rule that checks
download_location != "NOASSERTION") — i.e., require distribution_url !=
"NOASSERTION" before invoking sbom.url_matches_any_pattern(distribution_url,
patterns) for the parsed_purl.type/allowed_patterns logic so NOASSERTION entries
are bypassed.
---
Nitpick comments:
In `@policy/release/hermetic_task/hermetic_task_test.rego`:
- Around line 18-21: Duplicate parameter block [{"name": "HERMETIC", "value":
"true"}, {"name": "enable-hermeto-proxy", "value": "true"}] appears in many
tests; extract it into a shared variable/helper (e.g., hermeticProxyParams) and
reference that helper everywhere instead of inlining the array literal. Update
tests that currently include the literal (the occurrences you noted and the
block in hermetic_task_test.rego) to use the new shared symbol so future policy
changes only need one edit.
In `@policy/release/sbom_spdx/sbom_spdx.rego`:
- Around line 287-291: The rule currently defaults download_location :=
object.get(pkg, "downloadLocation", "") and then runs
sbom.url_matches_any_pattern even when empty (except for "NOASSERTION"); update
the logic to explicitly handle missing/empty values for clarity: add an explicit
check using the download_location symbol (e.g., download_location == "" or
download_location == null) and treat it the same as an invalid value (deny) or
add a clear comment explaining the intended behavior; adjust the condition that
uses sbom.url_matches_any_pattern(download_location, patterns) and NOASSERTION
so it only runs when download_location is non-empty, and reference
allowed_patterns and parsed_purl.type when locating the pattern lookup to ensure
the change is applied in the same block.
🪄 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: Pro
Run ID: a78d9360-e660-43f0-abca-a8224d886780
📒 Files selected for processing (7)
acceptance/samples/policy-input-golden-container.jsonantora/docs/modules/ROOT/pages/packages/release_hermetic_task.adocantora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adocantora/docs/modules/ROOT/pages/release_policy.adocpolicy/release/hermetic_task/hermetic_task_test.regopolicy/release/sbom_cyclonedx/sbom_cyclonedx.regopolicy/release/sbom_spdx/sbom_spdx.rego
✅ Files skipped from review due to trivial changes (1)
- antora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adoc
🚧 Files skipped from review as they are similar to previous changes (2)
- acceptance/samples/policy-input-golden-container.json
- antora/docs/modules/ROOT/pages/release_policy.adoc
simonbaird
reviewed
Apr 7, 2026
simonbaird
reviewed
Apr 7, 2026
simonbaird
left a comment
simonbaird
left a comment
Member
There was a problem hiding this comment.
See my comment about not having (essentially) the same logic defined twice, once for each format of sbom.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego (1)
744-750: Add an npm-targeted assertion to prevent config drift
_proxy_rule_dataincludes bothmavenandnpmpatterns, but onlymavenis exercised. Add one npm allow/deny test so type-specific pattern routing is covered and future regressions are caught earlier.Proposed test addition
+test_proxy_url_cyclonedx_npm_allowed if { + att := json.patch(_sbom_1_5_attestation, [{ + "op": "add", + "path": "/statement/predicate/components/-", + "value": _cdx_proxy_component( + "pkg:npm/example-lib@1.0.0", + "https://proxy.example.com/npm/example-lib/-/example-lib-1.0.0.tgz", + ), + }]) + + assertions.assert_empty(sbom_cyclonedx.deny) with input.attestations as [att] + with input.image.ref as "registry.local/spam@sha256:1230000000000000000000000000000000000000000000000000000000000123" + with ec.oci.image_referrers as [] + with ec.oci.image_tag_refs as [] + with data.rule_data as _proxy_rule_data +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego` around lines 744 - 750, Add an npm-specific test case that exercises the _proxy_rule_data npm patterns: create one assertion that passes an npm URL matching "^https://proxy\\.example\\.com/npm/.*" and expects it to be allowed, and one assertion that passes a non-matching npm URL (e.g., "https://other.example.com/npm/pkg") and expects it to be denied. Place these assertions in sbom_cyclonedx_test.rego near the existing _proxy_rule_data block and use the same rule/predicate used for the maven tests (reference the same test helper or rule name used for maven assertions) so npm-specific routing is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego`:
- Around line 744-750: Add an npm-specific test case that exercises the
_proxy_rule_data npm patterns: create one assertion that passes an npm URL
matching "^https://proxy\\.example\\.com/npm/.*" and expects it to be allowed,
and one assertion that passes a non-matching npm URL (e.g.,
"https://other.example.com/npm/pkg") and expects it to be denied. Place these
assertions in sbom_cyclonedx_test.rego near the existing _proxy_rule_data block
and use the same rule/predicate used for the maven tests (reference the same
test helper or rule name used for maven assertions) so npm-specific routing is
covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c6cda2da-04ac-4db9-a7dd-ed90cef1421a
📒 Files selected for processing (2)
policy/release/sbom_cyclonedx/sbom_cyclonedx.regopolicy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego
🚧 Files skipped from review as they are similar to previous changes (1)
- policy/release/sbom_cyclonedx/sbom_cyclonedx.rego
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
policy/release/sbom_spdx/sbom_spdx_test.rego (1)
496-502: Add explicit regression test for non-enabled PURL types being ignored.Current coverage proves enabled-type allow/deny and
NOASSERTION, but not the requirement that non-enabled types are skipped even with non-matching URLs.Suggested test addition
+test_proxy_url_spdx_non_enabled_type_skipped if { + att := json.patch(_sbom_attestation, [{ + "op": "add", + "path": "/statement/predicate/packages/-", + "value": _spdx_proxy_package( + "pkg:oci/example-lib@1.0", + "https://evil.com/example-lib-1.0.tgz", + ), + }]) + + assertions.assert_empty(sbom_spdx.deny) with input.attestations as [att] + with input.image.ref as "registry.local/spam@sha256:1230000000000000000000000000000000000000000000000000000000000123" + with ec.oci.image_referrers as [] + with ec.oci.image_tag_refs as [] + with data.rule_data as _spdx_proxy_rule_data +}As per coding guidelines,
**: Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@policy/release/sbom_spdx/sbom_spdx_test.rego` around lines 496 - 502, The current test fixture _spdx_proxy_rule_data defines proxy_enabled_purl_types and allowed_proxy_url_patterns but lacks a regression test that ensures PURL types not listed in proxy_enabled_purl_types are ignored even if their URLs would otherwise match allowed_proxy_url_patterns; add a new test case in sbom_spdx_test.rego that uses _spdx_proxy_rule_data with proxy_enabled_purl_types ["maven","npm"] and asserts that a non-enabled type (e.g., "pypi" or another not listed) is treated as ignored/NOASSERTION or not proxied despite its URL matching an allowed pattern, referencing the symbols _spdx_proxy_rule_data, proxy_enabled_purl_types and allowed_proxy_url_patterns in the test input and expected outcome.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@antora/docs/modules/ROOT/pages/packages/release_hermetic_task.adoc`:
- Line 21: Update the source link on the cited line to point to the actual deny
rule implementation by changing the fragment from `#L42` to `#L60` so it targets the
`deny contains result if` implementation for the hermeto_proxy_enabled rule;
locate the link text
`https://github.com/conforma/policy/blob/{page-origin-refhash}/policy/release/hermetic_task/hermetic_task.rego#L42`
and edit it to use `#L60` instead.
- Line 33: The source link currently points to the METADATA comment; update the
URL fragment in the AsciiDoc link to target the actual rule implementation by
pointing to the start of the deny rule in hermetic_task.rego (the `deny` rule)
instead of the metadata comment so the link opens the rule code rather than the
comment block.
---
Nitpick comments:
In `@policy/release/sbom_spdx/sbom_spdx_test.rego`:
- Around line 496-502: The current test fixture _spdx_proxy_rule_data defines
proxy_enabled_purl_types and allowed_proxy_url_patterns but lacks a regression
test that ensures PURL types not listed in proxy_enabled_purl_types are ignored
even if their URLs would otherwise match allowed_proxy_url_patterns; add a new
test case in sbom_spdx_test.rego that uses _spdx_proxy_rule_data with
proxy_enabled_purl_types ["maven","npm"] and asserts that a non-enabled type
(e.g., "pypi" or another not listed) is treated as ignored/NOASSERTION or not
proxied despite its URL matching an allowed pattern, referencing the symbols
_spdx_proxy_rule_data, proxy_enabled_purl_types and allowed_proxy_url_patterns
in the test input and expected outcome.
🪄 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: Pro
Run ID: fbae0a59-6a90-40cb-8bea-79d83241e5f3
📒 Files selected for processing (12)
acceptance/samples/policy-input-golden-container.jsonantora/docs/modules/ROOT/pages/packages/release_hermetic_task.adocantora/docs/modules/ROOT/pages/packages/release_sbom_cyclonedx.adocantora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adocantora/docs/modules/ROOT/pages/release_policy.adocantora/docs/modules/ROOT/partials/release_policy_nav.adocpolicy/release/hermetic_task/hermetic_task.regopolicy/release/hermetic_task/hermetic_task_test.regopolicy/release/sbom_cyclonedx/sbom_cyclonedx.regopolicy/release/sbom_cyclonedx/sbom_cyclonedx_test.regopolicy/release/sbom_spdx/sbom_spdx.regopolicy/release/sbom_spdx/sbom_spdx_test.rego
✅ Files skipped from review due to trivial changes (6)
- antora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adoc
- antora/docs/modules/ROOT/partials/release_policy_nav.adoc
- antora/docs/modules/ROOT/pages/packages/release_sbom_cyclonedx.adoc
- policy/release/sbom_spdx/sbom_spdx.rego
- policy/release/hermetic_task/hermetic_task_test.rego
- acceptance/samples/policy-input-golden-container.json
🚧 Files skipped from review as they are similar to previous changes (3)
- policy/release/hermetic_task/hermetic_task.rego
- policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego
- antora/docs/modules/ROOT/pages/release_policy.adoc
st3penta
requested changes
Apr 9, 2026
st3penta
left a comment
st3penta
left a comment
Contributor
There was a problem hiding this comment.
Summary
This PR adds three new Rego policy rules to enforce Sonatype proxy usage in hermetic builds, covering both the task-level (hermeto_proxy_enabled) and SBOM-level checks (CycloneDX and SPDX allowed_proxy_urls). The architecture is sound: the safe-default behavior (rules are no-op when proxy_enabled_purl_types is empty) enables incremental rollout, and the decision to check all components with proxy-enabled PURL types — not just Hermeto-fetched ones — provides stronger supply chain guarantees.
The refactoring of _not_hermetic_tasks into _required_hermetic_build_tasks plus derived sets is clean and well-structured. Test coverage for the new rules is solid, and the Antora documentation was properly updated.
Verdict
Request Changes — The main concern is the missing effective_on grace period on the hermeto_proxy_enabled rule, which could cause immediate failures for existing hermetic builds that predate the Sonatype proxy feature. This is a deviation from the established convention used by comparable rules like allowed_package_sources. The SBOM-level rules are partially mitigated by proxy_enabled_purl_types defaulting to empty, but adding effective_on to those as well would be consistent with precedent.
Additionally, please ensure these three new enforcement rules are called out in the next release notes — operators need to know about new rules that will affect their builds when enabled.
Review Breakdown
- 1 issue (blocking): missing
effective_ononhermeto_proxy_enabled - 8 suggestions: cross-validation dependency, METADATA improvements, validation gaps, edge case hardening, data-source documentation, test coverage gaps, Antora docs
- 2 nitpicks: case-variant test, acceptance test assertions
- 1 praise: defense-in-depth scope on SBOM proxy rules
joejstuart
force-pushed
the
EC-1725
branch
2 times, most recently
from
58a1167 to
0c26dc6
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
antora/docs/modules/ROOT/pages/packages/release_hermetic_task.adoc (1)
11-34:⚠️ Potential issue | 🟡 MinorDuplicate rule sections with identical anchors.
The documentation contains two sections for
hermetic_task__hermeto_proxy_enabledwith identical anchors (lines 11 and 23). This creates invalid HTML (duplicate IDs) and reflects the duplicate deny rules inhermetic_task.rego. Fixing the source duplication issue will resolve this on the nextmake generate-docsrun.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@antora/docs/modules/ROOT/pages/packages/release_hermetic_task.adoc` around lines 11 - 34, There are duplicate documentation sections using the same anchor hermetic_task__hermeto_proxy_enabled (and corresponding duplicate deny rules) causing duplicate HTML IDs; remove the duplicated section from the AsciiDoc source so only one section remains and then deduplicate the corresponding policy rule(s) in hermetic_task.rego (look for rule name/code hermetic_task.hermeto_proxy_enabled or any duplicated deny entries) so the docs generator only emits a single entry; regenerate docs with make generate-docs to confirm the duplicate anchor is gone.
♻️ Duplicate comments (1)
policy/release/sbom_spdx/sbom_spdx.rego (1)
274-276:⚠️ Potential issue | 🟠 MajorAdd
redhat_rpmsandpolicy_datacollections to the SPDXallowed_proxy_urlsrule.The SPDX rule currently includes only the
redhatcollection, while the equivalent CycloneDX rule (lines 289-292 insbom_cyclonedx.rego) includesredhat,redhat_rpms, andpolicy_data. This inconsistency means proxy URL validation won't apply toredhat_rpmsorpolicy_databuilds when using SPDX SBOMs, creating unequal policy enforcement across SBOM formats.Proposed fix
# collections: # - redhat +# - redhat_rpms +# - policy_data🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@policy/release/sbom_spdx/sbom_spdx.rego` around lines 274 - 276, The SPDX allowed_proxy_urls rule currently restricts proxy URL validation to the `redhat` collection only; update the rule `allowed_proxy_urls` in sbom_spdx.rego to include the `redhat_rpms` and `policy_data` collections alongside `redhat` so it mirrors the CycloneDX behavior and ensures proxy URL checks run for `redhat_rpms` and `policy_data` SPDX builds as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@antora/docs/modules/ROOT/pages/release_policy.adoc`:
- Around line 125-126: The release_policy.adoc contains a duplicated xref entry
for hermetic_task__hermeto_proxy_enabled; remove the duplicate xref so the rule
is listed only once, and also fix the underlying duplicate rule in
hermetic_task.rego by consolidating or removing the redundant rule definition
named hermetic_task__hermeto_proxy_enabled (ensure only a single authoritative
rule with that name remains). Ensure references and any documentation that
points to hermetic_task__hermeto_proxy_enabled still resolve correctly after
removing the duplicate.
In `@antora/docs/modules/ROOT/partials/release_policy_nav.adoc`:
- Around line 47-48: The navigation contains a duplicated xref entry for the
hermeto_proxy_enabled anchor; remove the duplicate line in
release_policy_nav.adoc so only a single
xref:packages/release_hermetic_task.adoc#hermeto_proxy_enabled[Hermetic build
task has Sonatype proxy enabled] remains (and if this duplication is generated
from hermetic_task.rego, deduplicate the rule there instead so the nav generator
emits a single entry).
In `@policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego`:
- Around line 703-718: The test test_proxy_url_cyclonedx_noassertion_skipped is
creating a CycloneDX component via _cdx_proxy_component with distribution.url ==
"NOASSERTION" and then asserting sbom_cyclonedx.deny is empty, which allows
bypassing allowed_proxy_urls for CycloneDX; update this test to not treat
"NOASSERTION" as a safe skip for CycloneDX—either remove the special-case or
change the assertion to expect a deny (e.g., replace
assertions.assert_empty(sbom_cyclonedx.deny) with an assertion that the deny
list is non-empty) so that sbom_cyclonedx.deny flags the proxy URL when using
_cdx_proxy_component.
---
Outside diff comments:
In `@antora/docs/modules/ROOT/pages/packages/release_hermetic_task.adoc`:
- Around line 11-34: There are duplicate documentation sections using the same
anchor hermetic_task__hermeto_proxy_enabled (and corresponding duplicate deny
rules) causing duplicate HTML IDs; remove the duplicated section from the
AsciiDoc source so only one section remains and then deduplicate the
corresponding policy rule(s) in hermetic_task.rego (look for rule name/code
hermetic_task.hermeto_proxy_enabled or any duplicated deny entries) so the docs
generator only emits a single entry; regenerate docs with make generate-docs to
confirm the duplicate anchor is gone.
---
Duplicate comments:
In `@policy/release/sbom_spdx/sbom_spdx.rego`:
- Around line 274-276: The SPDX allowed_proxy_urls rule currently restricts
proxy URL validation to the `redhat` collection only; update the rule
`allowed_proxy_urls` in sbom_spdx.rego to include the `redhat_rpms` and
`policy_data` collections alongside `redhat` so it mirrors the CycloneDX
behavior and ensures proxy URL checks run for `redhat_rpms` and `policy_data`
SPDX builds as well.
🪄 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: Pro Plus
Run ID: 6b5baffd-5bb3-41fd-9c09-87aded918c3a
📒 Files selected for processing (14)
acceptance/samples/policy-input-golden-container.jsonantora/docs/modules/ROOT/pages/packages/release_hermetic_task.adocantora/docs/modules/ROOT/pages/packages/release_sbom_cyclonedx.adocantora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adocantora/docs/modules/ROOT/pages/release_policy.adocantora/docs/modules/ROOT/partials/release_policy_nav.adocpolicy/lib/sbom/sbom.regopolicy/release/hermetic_task/hermetic_task.regopolicy/release/hermetic_task/hermetic_task_test.regopolicy/release/sbom/sbom_test.regopolicy/release/sbom_cyclonedx/sbom_cyclonedx.regopolicy/release/sbom_cyclonedx/sbom_cyclonedx_test.regopolicy/release/sbom_spdx/sbom_spdx.regopolicy/release/sbom_spdx/sbom_spdx_test.rego
✅ Files skipped from review due to trivial changes (3)
- acceptance/samples/policy-input-golden-container.json
- antora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adoc
- antora/docs/modules/ROOT/pages/packages/release_sbom_cyclonedx.adoc
🚧 Files skipped from review as they are similar to previous changes (2)
- policy/release/hermetic_task/hermetic_task_test.rego
- policy/release/sbom_spdx/sbom_spdx_test.rego
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove extra blank line to pass opa fmt check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add effective_on: 2026-06-01T00:00:00Z to the allowed_proxy_urls rules in sbom_cyclonedx and sbom_spdx to match the hermetic_task rule. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
simonbaird
previously approved these changes
Apr 16, 2026
simonbaird
left a comment
simonbaird
left a comment
Member
There was a problem hiding this comment.
Lgtm, but I didn't follow some of the discussion.
st3penta
reviewed
Apr 17, 2026
st3penta
left a comment
st3penta
left a comment
Contributor
There was a problem hiding this comment.
Looks good now, just one nitpick about rule collections
st3penta
previously approved these changes
Apr 17, 2026
Rule must have redhat, redhat_rpms, policy_data collections
st3penta
approved these changes
Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
allowed_proxy_urlsrule tosbom_cyclonedx— checksexternalReferencesURLs where type isdistributionagainstallowed_proxy_url_patternsforPURL types listed in
proxy_enabled_purl_typesallowed_proxy_urlsrule tosbom_spdx— checksdownloadLocationagainst the same patterns (skipsNOASSERTION)proxy_enabled_purl_typesare validated; all others are ignoredNote: This branch includes merge commits from EC-1722 and EC-1724 as it depends on both.
Jira
https://redhat.atlassian.net/browse/EC-1725