Skip to content

feat: add unless conditions to disallowed_attributes rule data#1744

Closed
lcarva wants to merge 1 commit into
conforma:mainfrom
lcarva:disallowed-attributes-unless
Closed

feat: add unless conditions to disallowed_attributes rule data#1744
lcarva wants to merge 1 commit into
conforma:mainfrom
lcarva:disallowed-attributes-unless

Conversation

@lcarva

@lcarva lcarva commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Allow exempting packages from disallowed attribute checks based on PURL patterns. Each disallowed_attributes entry now accepts an optional "unless" array of objects with a "purl" regex field. When a package's PURL matches any pattern, the attribute is permitted. Multiple unless entries are OR'd — any match exempts the package.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@lcarva, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 47 minutes and 27 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: e56468b8-e617-450d-9484-fb1130e00522

📥 Commits

Reviewing files that changed from the base of the PR and between d8deb10 and 822eadc.

📒 Files selected for processing (8)
  • antora/docs/modules/ROOT/pages/packages/release_sbom_cyclonedx.adoc
  • antora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adoc
  • policy/lib/sbom/sbom.rego
  • policy/release/sbom/sbom_test.rego
  • policy/release/sbom_cyclonedx/sbom_cyclonedx.rego
  • policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego
  • policy/release/sbom_spdx/sbom_spdx.rego
  • policy/release/sbom_spdx/sbom_spdx_test.rego
📝 Walkthrough

Walkthrough

This PR introduces conditional per-attribute exceptions to SBOM policy rules via an unless field containing purl regex patterns. The shared library defines the schema extension and exclusion logic, which both CycloneDX and SPDX implementations apply to skip denials when a component's purl matches any unless condition. Documentation links are updated to reflect the implementation changes.

Changes

SBOM attribute conditional exclusion via unless patterns

Layer / File(s) Summary
Shared schema and exclusion logic
policy/lib/sbom/sbom.rego
Extended disallowed_attributes schema to accept optional unless array containing objects with required purl string fields. Added attribute_excluded(purls, disallowed) rule that returns true when any unless.purl regex matches any provided purl.
Shared validation test coverage for unless
policy/release/sbom/sbom_test.rego
Added valid and invalid unless field test cases: valid entry with unless purl regex, invalid entries where unless is non-array or missing required purl. Updated expected validation denial messages for the new unless validation errors.
CycloneDX attribute exclusion enforcement
policy/release/sbom_cyclonedx/sbom_cyclonedx.rego, policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego
Updated disallowed_package_attributes denial rule to construct a purls set from the component's purl and skip denial when sbom.attribute_excluded(purls, disallowed) is true. Added three test cases verifying attributes are allowed when unless purl patterns match, allowed when any of multiple patterns match, and disallowed when patterns do not match.
SPDX attribute exclusion enforcement
policy/release/sbom_spdx/sbom_spdx.rego, policy/release/sbom_spdx/sbom_spdx_test.rego
Updated "Disallowed package attributes" rule to extract purls from package externalRefs and skip denial when sbom.attribute_excluded(purls, disallowed) is false. Added three test cases verifying unless pattern matching behavior for SPDX attributes.
Documentation link updates
antora/docs/modules/ROOT/pages/packages/release_sbom_cyclonedx.adoc, antora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adoc
Updated Source hyperlinks for CycloneDX rules (allowed external references, allowed sources, allowed proxy URLs, disallowed external references, proxy metadata required) and SPDX rules (allowed proxy URLs, proxy metadata required) to reflect new implementation line numbers.

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding unless conditions to the disallowed_attributes rule data, which is the primary feature implemented across the changeset.
Description check ✅ Passed The description clearly explains the new unless conditions mechanism, including how PURL patterns work and the OR semantics of multiple entries, directly relating to the changeset.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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

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

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:29 PM UTC · Completed 5:38 PM UTC
Commit: 47d3320 · View workflow run →

@codecov

codecov Bot commented Jun 11, 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/sbom/sbom.rego 100.00% <100.00%> (ø)
policy/release/sbom/sbom_test.rego 100.00% <ø> (ø)
policy/release/sbom_cyclonedx/sbom_cyclonedx.rego 100.00% <100.00%> (ø)
...cy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego 100.00% <100.00%> (ø)
policy/release/sbom_spdx/sbom_spdx.rego 100.00% <100.00%> (ø)
policy/release/sbom_spdx/sbom_spdx_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.

@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

🤖 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 `@policy/lib/sbom/sbom.rego`:
- Around line 478-482: The shared helper attribute_excluded currently runs
regex.match(condition.purl, purl) against the raw PURL text, which causes
mismatches between percent-encoded SPDX PURLs and unencoded CycloneDX PURLs;
modify attribute_excluded to canonicalize candidate PURLs before matching (e.g.,
percent-decode percent-encoded characters like %3A → :, normalize case/scheme if
needed) by introducing/using a normalize_purl step on the purl variable prior to
regex.match so the same unless.purl regex applies consistently across both SBOM
formats; update any references to purl within attribute_excluded (and reuse
normalize_purl if you add a helper) so all matches use the normalized value.
- Around line 265-268: The schema for unless[].purl currently allows any string
but purl is later used as a regular expression; update the schema entry for purl
in policy/lib/sbom/sbom.rego so that purl is validated as a regex string (e.g.,
change the property from "type": "string" to include a regex validation such as
adding "format": "regex" or an appropriate "pattern" that enforces valid regex
syntax), ensuring the unless[].purl property rejects invalid regexes at
validation time so the runtime regex evaluation (unless[].purl usage) cannot
silently fail.
🪄 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: 6e136a93-ffe6-4d6a-a8d6-d4972b3be736

📥 Commits

Reviewing files that changed from the base of the PR and between 2a3c4fe and d8deb10.

📒 Files selected for processing (8)
  • antora/docs/modules/ROOT/pages/packages/release_sbom_cyclonedx.adoc
  • antora/docs/modules/ROOT/pages/packages/release_sbom_spdx.adoc
  • policy/lib/sbom/sbom.rego
  • policy/release/sbom/sbom_test.rego
  • policy/release/sbom_cyclonedx/sbom_cyclonedx.rego
  • policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego
  • policy/release/sbom_spdx/sbom_spdx.rego
  • policy/release/sbom_spdx/sbom_spdx_test.rego

Comment thread policy/lib/sbom/sbom.rego Outdated
Comment thread policy/lib/sbom/sbom.rego
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [edge-case] policy/release/sbom_cyclonedx/sbom_cyclonedx.rego:151 — The purls set is constructed as {p | p := component.purl} which produces an empty set when component.purl is undefined (e.g. OS-type components without a PURL). When the set is empty, attribute_excluded always returns false (because some purl in purls fails on an empty set), so not sbom.attribute_excluded(purls, disallowed) is true and the deny rule fires. This means unless clauses have no effect for components that lack a PURL field. The same pattern applies in sbom_spdx.rego if a package has no purl-type external refs.

Low

  • [authorization-bypass] policy/lib/sbom/sbom.rego:265 — The schema for unless[].purl requires it to be a string but does not enforce a minimum length. An empty string regex ("") matches every purl, which would silently exempt all packages from the disallowed attribute check.
    Remediation: Add "minLength": 1 to the purl property schema.

  • [test-adequacy] policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego — No test for the edge case where a component lacks a purl field and a disallowed attribute with an unless clause is configured. Adding such a test would document the expected behavior that purl-less components are never exempted.
    Remediation: Add a test configuring a disallowed attribute on a purl-less component with an unless clause, asserting the deny result still fires.

  • [test-adequacy] policy/release/sbom_cyclonedx/sbom_cyclonedx_test.rego — No multi-component test where one component is exempted by unless and another (with the same attribute but a different purl) is not.
    Remediation: Add a test with two components sharing the same disallowed attribute, where only one component's PURL matches the unless pattern.

  • [inconsistent-exemption-pattern] policy/lib/sbom/sbom.rego — The new attribute_excluded function uses raw regex.match on PURL strings, while the existing _excluded function uses structured PURL field comparison via ec.purl.parse. This creates two distinct exemption idioms in the same library file.

  • [code-organization] policy/lib/sbom/sbom.rego:477 — The new attribute_excluded function is placed after the rule_data key constants at the bottom of the file. All other public helper functions are grouped above the constants footer.
    Remediation: Move attribute_excluded above the rule_data_packages_key constant block.

Info

  • [idiom-consistency] policy/lib/sbom/sbom.rego:477 — The new attribute_excluded function does not use else := false, unlike other boolean predicate helpers (has_item, component_found_by_hermeto). Since it is only used with not, undefined and false behave identically, so this is purely stylistic.

  • [defense-in-depth] policy/lib/sbom/sbom.rego:479 — Unlike allowed_proxy_url_patterns validation which explicitly checks regex.is_valid(pattern) at rule-data-validation time, unless[].purl relies solely on schema-level "format": "regex" validation.

  • [documentation-convention] policy/lib/sbom/sbom.rego:477 — The new attribute_excluded function lacks a comment, unlike other public functions in this file.

  • [api-shape] policy/release/sbom_cyclonedx/sbom_cyclonedx.rego:152purls := {p | p := component.purl} always yields a single-element set for CycloneDX, while SPDX can genuinely have multiple purl refs. The set-based API is driven by the SPDX use case.

  • [architecture-fit] policy/release/sbom_cyclonedx/sbom_cyclonedx.rego — The feature follows the project's established pattern of modifying both CycloneDX and SPDX rule files symmetrically, placing shared logic in the lib. Good architectural alignment.

Previous run

Review

Findings

Medium

  • [error handling gap / injection] policy/lib/sbom/sbom.rego:479 — The attribute_excluded function passes condition.purl from rule data directly to regex.match() without validating that the value is a syntactically valid regular expression. Other rule data fields used as regex patterns in this same file include explicit regex.is_valid() checks (e.g., allowed_proxy_url_patterns at lines 388-396) or JSON Schema format: regex constraints (e.g., allowed_package_sources patterns at line 332-334). A malformed regex in unless[].purl would cause a runtime evaluation error in OPA.
    Remediation: Add "format": "regex" to the JSON schema definition for the purl property inside the unless items (consistent with the approach used for allowed_package_sources), or add a rule_data_errors rule that validates each unless[].purl value using regex.is_valid() (consistent with the approach used for allowed_proxy_url_patterns).

Info

  • [defense-in-depth] policy/release/sbom_spdx/sbom_spdx.rego:240 — The SPDX disallowed_package_attributes deny rule collects purls from externalRefs with referenceType == "purl", which could yield multiple purls. If any one purl matches an unless pattern, the entire package is exempted. This is a theoretical concern since SBOM content is typically build-system generated and signed.
  • [sub-agent-failure] — The intent-coherence, style-conventions, and docs-currency sub-agents did not return findings due to model unavailability (claude-sonnet-4-5@20250929 not available on vertex deployment).

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 11, 2026
Allow exempting packages from disallowed attribute checks based on PURL
patterns. Each disallowed_attributes entry now accepts an optional
"unless" array of objects with a "purl" regex field. When a package's
PURL matches any pattern, the attribute is permitted. Multiple unless
entries are OR'd — any match exempts the package.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lcarva lcarva force-pushed the disallowed-attributes-unless branch from d8deb10 to 822eadc Compare June 11, 2026 17:40
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:41 PM UTC · Completed 5:56 PM UTC
Commit: 47d3320 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 11, 2026
@lcarva

lcarva commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Closing this in favor of #1739

@lcarva lcarva closed this Jun 11, 2026
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 11, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 5:58 PM UTC · Completed 6:04 PM UTC
Commit: 47d3320 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #1744feat: add unless conditions to disallowed_attributes rule data

PR #1744 was a human-authored PR by lcarva (co-authored with Claude Opus 4.6) that added unless conditions to exempt packages from disallowed SBOM attribute checks via PURL regex matching. It was closed without merging after ~30 minutes in favor of PR #1739, which implements the same feature using a structured except_when qualifier-based approach via ec.purl.parse.

Workflow timeline

  1. 17:27 — PR opened
  2. 17:29 — First fullsend review started (run 27365233014)
  3. 17:33 — CodeRabbit posted 2 findings; author addressed one via force-push ("format": "regex")
  4. 17:38 — First review completed (partial — 3 sub-agents failed due to claude-sonnet-4-5@20250929 model unavailability on Vertex)
  5. 17:40 — Force-push triggered second review dispatch (run 27365951844)
  6. 17:56 — Second review completed with comprehensive findings
  7. 17:56:53 — PR closed by author in favor of PR feat(EC-1881): add except_when support to disallowed_attributes rule data #1739

Review quality assessment

The fullsend review findings were substantive and accurate:

  • Medium: empty purls edge-case — Correctly identified that components without PURLs would bypass unless clauses silently
  • Medium: missing regex.is_valid — Valid inconsistency with existing codebase patterns
  • Low: empty-string regex bypass — Real schema gap (missing minLength: 1)
  • Low: test adequacy gaps — Genuine coverage holes despite 100% line coverage
  • Low: inconsistent exemption pattern — Highlights the design difference that ultimately favored PR feat(EC-1881): add except_when support to disallowed_attributes rule data #1739's approach

Improvement opportunities (all already tracked)

All identified inefficiencies have existing open issues in fullsend-ai/fullsend:

Issue Topic
#1418, #1422, #1452, #1357 Deduplicate review runs on force-push
#1870, #1439, #885 Skip review dispatch on closed PRs
#1771 Sub-agent model fallback on unavailability
#2176, #1411 Skip retro on closed-without-merge PRs

No new proposals are warranted — this workflow exhibited known patterns that are already being addressed.

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

Labels

requires-manual-review Review requires human judgment size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant