Warn when regex patterns in rule data lack anchoring

[jsmid1]

regex.match in OPA performs substring matching, so unanchored patterns like "console.redhat.com" also match "evil-console.redhat.com".

Add rule_data_errors warnings (non-blocking) when user-supplied patterns lack ^ anchoring in: allowed_package_sources, allowed_external_references, disallowed_external_references, allowed_proxy_url_patterns, allowed_target_branch_patterns, allowed_rpm_build_dependency_sources, and disallowed_platform_patterns.

Also document the anchoring requirement in the policy authoring guide.

Ref: https://issues.redhat.com/browse/EC-1905

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:31 AM UTC · Completed 7:37 AM UTC
Commit: 87c4a29 · View workflow run →

[coderabbitai]

Warning

Review limit reached

@jsmid1, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.

How can I continue?

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.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: d7d94a5d-d372-4af7-870c-6fae867b25cf

📥 Commits

Reviewing files that changed from the base of the PR and between bad37bf and 548f65d.

📒 Files selected for processing (14)

• antora/docs/modules/ROOT/pages/authoring.adoc
• antora/docs/modules/ROOT/pages/packages/release_git_branch.adoc
• antora/docs/modules/ROOT/pages/packages/release_rpm_build_deps.adoc
• antora/docs/modules/ROOT/pages/release_policy.adoc
• antora/docs/modules/ROOT/partials/release_policy_nav.adoc
• policy/lib/rule_data/rule_data.rego
• policy/lib/sbom/sbom.rego
• policy/release/buildah_build_task/buildah_build_task.rego
• policy/release/buildah_build_task/buildah_build_task_test.rego
• policy/release/git_branch/git_branch.rego
• policy/release/git_branch/git_branch_test.rego
• policy/release/rpm_build_deps/rpm_build_deps.rego
• policy/release/rpm_build_deps/rpm_build_deps_test.rego
• policy/release/sbom/sbom_test.rego

📝 Walkthrough

Walkthrough

Changes

Pattern anchoring validation

Layer / File(s)	Summary
SBOM pattern warning validation policy/lib/sbom/sbom.rego, policy/release/sbom/sbom_test.rego	SBOM rule-data checks and tests now cover anchoring warnings for external references, package sources, proxy URLs, and except_when patterns.
Release policy anchoring checks policy/release/buildah_build_task/*, policy/release/git_branch/*, policy/release/rpm_build_deps/*	Release policies emit warning-severity errors for unanchored platform, branch, and RPM dependency source patterns, with corresponding tests.
Authoring guidance antora/docs/modules/ROOT/pages/authoring.adoc	Documentation describes leading ^ anchoring, optional trailing $, examples, and warning behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: robnester-rh, simonbaird

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: warning on unanchored regex patterns in rule data.
Description check	✅ Passed	The description is directly related to the changeset and matches the anchoring-warning updates.
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

---

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

[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/rule_data/rule_data.rego	100.00% <100.00%> (ø)
policy/lib/sbom/sbom.rego	100.00% <100.00%> (ø)
...release/buildah_build_task/buildah_build_task.rego	100.00% <100.00%> (ø)
...se/buildah_build_task/buildah_build_task_test.rego	100.00% <100.00%> (ø)
policy/release/git_branch/git_branch.rego	100.00% <100.00%> (ø)
policy/release/git_branch/git_branch_test.rego	100.00% <100.00%> (ø)
policy/release/rpm_build_deps/rpm_build_deps.rego	100.00% <100.00%> (ø)
...cy/release/rpm_build_deps/rpm_build_deps_test.rego	100.00% <100.00%> (ø)
policy/release/sbom/sbom_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 — approve

PR: #1781 — Warn when regex patterns in rule data lack anchoring

Summary

This PR addresses a genuine security concern: OPA's regex.match performs substring matching, so unanchored patterns like console\.redhat\.com also match evil-console.redhat.com. The change adds warning-severity (non-blocking) errors when user-supplied regex patterns in rule data lack ^ anchoring. The implementation is consistent across multiple policy packages and includes comprehensive tests and documentation updates.

What was reviewed

• policy/lib/rule_data/rule_data.rego — New anchoring_errors(key) helper using set comprehension. Correctly checks is_string(pattern) before startswith, handles deduplication via sets, and emits "warning" severity.
• policy/lib/sbom/sbom.rego — Four new rule_data_errors rules for nested pattern structures (allowed_external_references, disallowed_external_references, allowed_package_sources, allowed_proxy_url_patterns, disallowed_attributes.except_when). Each correctly navigates the object structure to extract string patterns and checks for ^ anchoring.
• policy/release/buildah_build_task/buildah_build_task.rego — Delegates to rule_data.anchoring_errors for disallowed_platform_patterns. Integrates cleanly with existing _rule_data_errors set.
• policy/release/git_branch/git_branch.rego — New deny rule with proper METADATA annotations (title, description, short_name, failure_msg, collections). Uses rule_data.anchoring_errors for allowed_target_branch_patterns.
• policy/release/rpm_build_deps/rpm_build_deps.rego — Same pattern as git_branch for allowed_rpm_build_dependency_sources.
• Test files — All new rules have dedicated test cases for both anchored (no warning) and unanchored (warning emitted) scenarios. Existing tests updated to expect anchoring warnings where test data uses unanchored patterns.
• Documentation — Authoring guide documents the anchoring requirement. Generated docs updated for new rules. Navigation partials and release policy pages updated.

Findings

No blocking findings. The change is well-structured, follows established codebase patterns, and the warnings are appropriately non-blocking.

[low · style] anchoring_errors lacks direct unit tests in rule_data_test.rego
File: policy/lib/rule_data/rule_data.rego, line ~204

The new anchoring_errors function is tested indirectly through consumer tests (buildah_build_task_test, git_branch_test, rpm_build_deps_test), which provides adequate coverage. A direct unit test in rule_data_test.rego would be marginally more robust for future refactors but is not necessary given the thorough integration coverage.

[low · correctness] Anchoring warnings fire on already-invalid regexes
File: policy/lib/sbom/sbom.rego

When a pattern is both syntactically invalid (e.g., "[") and unanchored, both the existing "invalid regex" failure and the new "unanchored" warning are emitted. This creates slight noise, but since the failure severity blocks while the warning doesn't, there's no practical impact. The test at sbom_test.rego correctly expects both.

[low · design] Only ^ anchoring is checked, not $
File: policy/lib/rule_data/rule_data.rego, line ~204

The documentation correctly states $ anchoring is optional ("where appropriate"). The primary security risk (prefix spoofing like evil-console.redhat.com) is mitigated by ^ anchoring alone. This is a reasonable design decision.

Previous run

Review — approve

PR: #1781 — Warn when regex patterns in rule data lack anchoring

Summary

This PR adds non-blocking warnings when user-supplied regex patterns in rule data lack ^ anchoring. OPA's regex.match performs substring matching, so unanchored patterns like console\.redhat\.com also match evil-console.redhat.com. The change is a well-targeted security improvement.

Changes reviewed across 9 files (346+/14−):

• policy/lib/sbom/sbom.rego — Four new rule_data_errors rules check anchoring for allowed_external_references, disallowed_external_references, allowed_package_sources, allowed_proxy_url_patterns, and disallowed_attributes except_when patterns.
• policy/release/buildah_build_task/buildah_build_task.rego — One new _rule_data_errors rule for disallowed_platform_patterns.
• policy/release/git_branch/git_branch.rego — New _rule_data_errors rule, new deny rule with proper METADATA annotations, and refactored matches_any to use a _branch_patterns_key constant.
• policy/release/rpm_build_deps/rpm_build_deps.rego — Same pattern as git_branch: new validation, new annotated deny rule, key constant refactoring.
• Test files — Comprehensive tests added for all anchoring checks, including positive/negative cases and updates to existing tests to account for new warnings.
• antora/docs/modules/ROOT/pages/authoring.adoc — Documentation updated with anchoring guidance.

Correctness

The implementation is sound. All anchoring checks follow the established rule_data_errors / _rule_data_errors pattern used throughout the codebase. The new rules emit "severity": "warning" so they are non-blocking. The METADATA annotations on the new deny rules in git_branch.rego and rpm_build_deps.rego correctly include short_name, failure_msg, and collections, following the required conventions.

The refactoring of hardcoded key strings into _branch_patterns_key and _sources_key constants is a clean improvement that keeps the code DRY.

Tests are comprehensive: each module has dedicated anchoring tests for both the unanchored (warning expected) and anchored (no warning) cases, plus existing validation tests are updated to include the new expected warnings.

Security

This is a net security improvement. The unanchored regex patterns it detects represent a real supply chain risk — malicious sources could bypass allowlists by registering domains that contain the allowed domain as a substring. The warnings guide policy authors toward safer patterns.

Style & conventions

The code follows existing codebase patterns faithfully. METADATA annotations are present and well-formed. The # regal ignore:line-length pragmas are used where needed. Test naming follows the test_<feature>_<scenario> convention.

Findings

#	Severity	File	Description
1	low	policy/lib/sbom/sbom.rego	Missing is_string(pattern) guard in external references anchoring check — see below

Finding 1: The new anchoring check for external references (lines ~425–434 in the post-merge file) extracts the URL via object.get(ref, "url", "") and checks not startswith(pattern, "^"), but omits the is_string(pattern) guard that all three other new anchoring checks include (allowed_package_sources, allowed_proxy_url_patterns, disallowed_attributes except_when). While the JSON schema validation upstream ensures url is typed as string, adding the guard would maintain defensive consistency across all four rules. Low risk since schema validation catches type mismatches first, but worth aligning for maintainability.

Previous run (2)

Review — approve

Overall: This is a well-structured enhancement that adds non-blocking warnings when user-supplied regex patterns in rule data lack ^ anchoring — mitigating the real risk that OPA's regex.match performs substring matching and unanchored patterns silently match unintended values (e.g., console\.redhat\.com matching evil-console.redhat.com).

The implementation is consistent across all affected rule data keys, follows existing codebase patterns for rule_data_errors / _rule_data_errors validation, and includes thorough test coverage for both the warning and the anchored (no-warning) cases. The documentation addition in authoring.adoc is clear and actionable.

What was checked

• Correctness of anchoring checks: All five new rule_data_errors rules (external references, package sources, proxy URL patterns, except_when, platform patterns) correctly use not startswith(pattern, "^") and produce "severity": "warning" results. The is_string(pattern) guard prevents errors on non-string values.
• New deny rules in git_branch.rego and rpm_build_deps.rego: Both follow the established pattern from buildah_build_task.rego — _rule_data_errors set consumed by a deny rule with proper METADATA annotations (short_name, failure_msg: "%s", appropriate collections). The policy_data collection assignment is correct for rule data validation rules.
• Test coverage: Each rule data key has explicit tests for both unanchored (warning emitted) and anchored (no warning) patterns. Existing tests are correctly updated to expect the new warnings where test data uses unanchored patterns.
• Test data fix in git_branch_test.rego: The single_test_case mock pattern branch[0-9]+-rhel-[0-9]+.[0-9]+.[0-9]+$ is updated to ^.*branch[0-9]+-rhel-[0-9]+.[0-9]+.[0-9]+$. This preserves matching semantics (^.* at the start is equivalent to unanchored for regex.match) while avoiding spurious anchoring warnings in unrelated tests.
• Security: The warning severity (non-blocking) is appropriate for a change that could affect existing deployments. Users can migrate at their own pace without policy enforcement breaking.
• No PR body injection: The PR body contains only descriptive content and an external issue reference.

Findings

1. Missing make generate-docs output (low)

The PR adds two new METADATA-annotated deny rules (allowed_target_branch_patterns_format in git_branch.rego and allowed_rpm_build_dependency_sources_format in rpm_build_deps.rego). Per project conventions (AGENTS.md: "Run make generate-docs after changing policy annotations. Commit the regenerated files."), the generated Antora docs at antora/docs/modules/ROOT/pages/packages/release_git_branch.adoc and release_rpm_build_deps.adoc should be regenerated to include these new rule entries. The current docs only list the pre-existing rules. CI's make ci pipeline includes generate-docs and should flag this.

2. ^.* escape hatch does not improve anchoring (low)

The check only verifies that patterns start with ^. A user could suppress the warning by prepending ^.* to an existing unanchored pattern (e.g., ^.*console\.redhat\.com), which has identical matching behavior to the original unanchored pattern. This is an inherent limitation of a syntactic check. The warning message clearly guides toward proper anchoring ("Use anchored patterns to avoid unintended substring matches"), and patterns like ^.*suffix$ are legitimate for suffix matching, so stricter heuristics would produce false positives. This is a design trade-off, not a defect.

Previous run (3)

Review

Verdict: approve ✅

Summary

This PR adds non-blocking warning-severity rule_data_errors when user-supplied regex patterns in rule data lack ^ anchoring. This addresses a real security concern: OPA's regex.match performs substring matching by default, so an unanchored pattern like console\.redhat\.com also matches evil-console.redhat.com.

The changes are well-structured and consistent across all affected policy files:

File	What's added
policy/lib/sbom/sbom.rego	Anchoring checks for 4 pattern types: external references (allowed/disallowed), package sources, proxy URL patterns, and except_when patterns
policy/release/buildah_build_task/buildah_build_task.rego	Anchoring check for disallowed_platform_patterns
policy/release/git_branch/git_branch.rego	New deny rule with metadata for allowed_target_branch_patterns anchoring
policy/release/rpm_build_deps/rpm_build_deps.rego	New deny rule with metadata for allowed_rpm_build_dependency_sources anchoring
antora/docs/modules/ROOT/pages/authoring.adoc	Documents the anchoring requirement

Correctness

Pattern followed is sound. The new rules use "severity": "warning" with result_helper_with_severity, matching the established pattern in buildah_build_task.rego for non-blocking data validation warnings. Warnings go through deny (not warn) — this is correct per codebase convention; the framework distinguishes blocking vs. non-blocking via the severity field, not the rule name.

METADATA annotations are appropriate. The new rules in git_branch.rego and rpm_build_deps.rego include collections: [redhat_rpms, policy_data], which is consistent with existing data-validation rules.

Test coverage is comprehensive. Each new check has both positive (warning produced for unanchored patterns) and negative (no warning for ^-anchored patterns) test cases. Existing tests that use unanchored mock data are correctly updated to expect the new warnings.

Security

The PR correctly addresses the substring-matching risk. Using non-blocking warnings (rather than failures) is a good incremental approach that flags the issue without breaking existing configurations.

No new attack surfaces are introduced. PR body content is descriptive and contains no injection attempts.

Low-severity observations

1. Missing is_string guard on anchoring checks — In buildah_build_task.rego, the anchoring check iterates some r in rule_data.get(...) and calls not startswith(r, "^"). If r is a non-string (e.g., the integer 1 in the existing test data), startswith is undefined in Rego, not undefined evaluates to true, and the warning fires with a confusing message (Pattern '\x01' in ...). While the schema validation already catches type errors at failure severity, adding is_string(r) before the startswith check would avoid the noisy duplicate. The same pattern applies to the sbom.rego checks, though object.get(ref, "url", "") provides a string default that partially mitigates this. Non-blocking — the warning is harmless alongside the higher-severity type-error failure.

2. Test mock data uses ^.* prefix for mechanical anchoring — In git_branch_test.rego (line 20), the pattern changes from "branch[0-9]+-rhel-..." to "^.*branch[0-9]+-rhel-..." to suppress the new warning. While ^.* is semantically equivalent (matches the same strings as no anchor), this is a pragmatic and reasonable choice for test fixtures where the pattern intentionally matches mid-string content. No production impact.

---

Labels: PR adds policy validation warnings and updates documentation

Previous run (4)

Review of PR #1781 — Warn when regex patterns in rule data lack anchoring

Verdict: request-changes

The PR adds warning-level rule_data_errors checks for unanchored regex patterns across several rule data keys. The motivation is sound — OPA's regex.match performs substring matching, so console\.redhat\.com would match evil-console.redhat.com. The sbom.rego and buildah_build_task.rego integrations are correct. However, two of the four policy files have dead-code warnings that will never surface during policy evaluation.

Findings

🔴 High: rule_data_errors not wired into deny/warn in git_branch.rego

File: policy/release/git_branch/git_branch.rego

The PR adds a public rule_data_errors rule, but there is no deny or warn rule in this package that iterates over rule_data_errors. Compare with buildah_build_task.rego which has:

# METADATA annotation with short_name, failure_msg, collections...
deny contains result if {
    some error in _rule_data_errors
    result := metadata.result_helper_with_severity(rego.metadata.chain(), [error.message], error.severity)
}

git_branch.rego has no equivalent consumer. The rule_data_errors set is computed but never used during policy evaluation. The tests pass because they directly assert on git_branch.rule_data_errors, not through deny, masking the integration gap.

Additionally, the existing test fixture mock_rule_data already contains an unanchored pattern ("branch[0-9]+-rhel-[0-9]+.[0-9]+.[0-9]+$" — no ^ prefix), which would produce spurious warnings if rule_data_errors were wired into deny, breaking existing tests.

Remediation: Add a deny rule with proper METADATA annotations (including short_name, failure_msg: "%s", and relevant collections) that consumes rule_data_errors, following the pattern in buildah_build_task.rego. Update existing tests that use unanchored patterns in their fixtures.

🔴 High: rule_data_errors not wired into deny/warn in rpm_build_deps.rego

File: policy/release/rpm_build_deps/rpm_build_deps.rego

Same issue as git_branch.rego. The rule_data_errors rule is defined but no deny or warn rule consumes it. The existing package only exposes warn (not deny), so the anchoring warnings would never surface during evaluation. The tests pass because they directly check rpm_build_deps.rule_data_errors.

Remediation: Add a deny (or warn) rule with METADATA annotations that consumes rule_data_errors, consistent with how other packages wire up rule data validation.

🟡 Medium: Inconsistent rule_data_errors visibility convention

Files: policy/release/git_branch/git_branch.rego, policy/release/rpm_build_deps/rpm_build_deps.rego

The codebase convention for package-internal rule data validation uses _rule_data_errors (private, underscore-prefixed) consumed by a local deny rule — see buildah_build_task.rego, schedule.rego, cve.rego, etc. The PR uses rule_data_errors (public, no underscore) in git_branch.rego and rpm_build_deps.rego, but nothing imports these packages to consume the public rule. If the intent is internal consumption, use _rule_data_errors for consistency. If external consumption is intended, a consumer must be established.

Remediation: Rename to _rule_data_errors and add a local deny/warn consumer, consistent with the rest of the codebase.

🟢 Low: Tests verify rule internals rather than observable policy behavior

Files: policy/release/git_branch/git_branch_test.rego, policy/release/rpm_build_deps/rpm_build_deps_test.rego

The new tests for git_branch and rpm_build_deps directly assert on rule_data_errors rather than on deny or warn output. While unit-testing individual rules is valid, the absence of integration tests through deny/warn means the tests don't catch the wiring gap described above. The sbom_test.rego and buildah_build_task_test.rego tests correctly assert through deny, which is why those integrations are verified.

Remediation: After wiring rule_data_errors into deny/warn, add or update tests that assert through the public deny/warn interface.

What looks good

• policy/lib/sbom/sbom.rego: All four anchoring checks (external_references, package_sources, proxy_url_patterns, disallowed_attributes except_when) are correctly implemented and properly consumed through the existing deny rule in policy/release/sbom/sbom.rego via lib.sbom.rule_data_errors. Good coverage of different data shapes.
• policy/release/buildah_build_task/buildah_build_task.rego: The _rule_data_errors addition follows the established pattern correctly — private rule, consumed by the existing annotated deny rule. Tests updated appropriately.
• antora/docs/modules/ROOT/pages/authoring.adoc: Clear documentation of the anchoring requirement with a concrete example.
• Test coverage for the working integrations (sbom_test.rego, buildah_build_task_test.rego): Thorough positive and negative test cases.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:47 AM UTC · Completed 7:55 AM UTC
Commit: 87c4a29 · View workflow run →

Superseded by updated review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:01 AM UTC · Completed 8:08 AM UTC
Commit: 87c4a29 · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 8:10 AM UTC · Ended 8:11 AM UTC
Commit: 87c4a29 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:12 AM UTC · Completed 8:18 AM UTC
Commit: 87c4a29 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:28 AM UTC · Completed 8:35 AM UTC
Commit: 87c4a29 · View workflow run →