Open
Conversation
Add support for an object-keyed format alongside the existing array
format for trusted_task_rules allow/deny entries. The object format
uses unique map keys, enabling OPA to deep-merge multiple data sources
with the same top-level keys without merge conflicts.
Object format: {"allow": {"my-task": {"pattern": "oci://..."}}}
Array format: {"allow": [{"name": "my-task", "pattern": "oci://..."}]}
Also adds test coverage for digest-only bundle references (no tag),
confirming that patterns must match the repo path without tag/digest
components since the matching key strips those.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
policy/lib/tekton/trusted.rego (1)
148-197: Simplify extraction comprehensions per Regal hint.The
some _, entry in ...pattern uses an unnecessary wildcard key (Regal flagged lines 151, 163, 178, 192). When only the value is needed, iterating withsome entry in ...is more idiomatic.♻️ Proposed simplification
_data_allow_array := [entry | is_object(data.trusted_task_rules) is_object(data.trusted_task_rules.allow) - some _, entry in data.trusted_task_rules.allow + some entry in data.trusted_task_rules.allow ] if { is_object(data.trusted_task_rules) is_object(data.trusted_task_rules.allow) }Apply the same change to
_data_deny_array,_rule_data_allow_array, and_rule_data_deny_array.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@policy/lib/tekton/trusted.rego` around lines 148 - 197, Replace the unnecessary wildcard key in the comprehensions that extract arrays from trusted_task_rules: for _data_allow_array, _data_deny_array, _rule_data_allow_array, and _rule_data_deny_array change the generator `some _, entry in ...` to `some entry in ...` so the comprehensions iterate only the value (entry) as Regal suggests; keep the surrounding is_object checks and lib_rule_data calls intact.
🤖 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/lib/tekton/trusted.rego`:
- Around line 148-197: Replace the unnecessary wildcard key in the
comprehensions that extract arrays from trusted_task_rules: for
_data_allow_array, _data_deny_array, _rule_data_allow_array, and
_rule_data_deny_array change the generator `some _, entry in ...` to `some entry
in ...` so the comprehensions iterate only the value (entry) as Regal suggests;
keep the surrounding is_object checks and lib_rule_data calls intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b5ef74bc-439a-4541-aac2-b7310fe0c51d
📒 Files selected for processing (3)
policy/lib/tekton/trusted.regopolicy/lib/tekton/trusted_test.regopolicy/release/trusted_task/trusted_task_test.rego
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
Remove array format support for trusted_task_rules. Only the object
format (keyed by unique name) is now supported. This simplifies the
schema, extraction logic, and enables OPA to deep-merge multiple data
sources without merge conflicts.
Format: {"allow": {"my-task": {"pattern": "oci://..."}}, "deny": {...}}
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
joejstuart
force-pushed
the
trusted-task-refactor
branch
from
791dc91 to
121c88f
Compare
Test data was using the old format
joejstuart
force-pushed
the
trusted-task-refactor
branch
from
c4d3b21 to
8a9c3e6
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
policy/lib/tekton/trusted.rego (1)
268-276:⚠️ Potential issue | 🟠 MajorDon’t silently skip malformed
trusted_task_rulesvalues.Line 275 only validates objects, so a non-empty legacy array or scalar value is treated like missing data. That can hide migration/configuration mistakes instead of surfacing a policy data error.
Proposed validation guard
data_errors contains error if { - # Only validate if rule_data contains an object (skip when it's [] or not provided) + # lib_rule_data returns [] when absent; validate every other provided value. rule_data_rules := lib_rule_data("trusted_task_rules") - is_object(rule_data_rules) + rule_data_rules != [] + rule_data_rules != null some e in j.validate_schema(rule_data_rules, _trusted_task_rules_schema) error := { "message": sprintf("trusted_task_rules data has unexpected format: %s", [e.message]), "severity": e.severity, } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@policy/lib/tekton/trusted.rego` around lines 268 - 276, The current data_errors rule only validates trusted_task_rules when lib_rule_data("trusted_task_rules") returns an object, which silently skips malformed non-object values (e.g., legacy arrays or scalars); update the rule so it also detects when rule_data_rules exists but is not an object and emits a policy error. Concretely, inside the data_errors rule (and/or a new clause alongside the is_object check) add a guard that checks lib_rule_data("trusted_task_rules") (symbol: rule_data_rules) is present and not an object (use the existing is_object predicate and check for inequality to []/empty), and produce an appropriate error entry instead of silently skipping; keep the existing j.validate_schema(rule_data_rules, _trusted_task_rules_schema) path for the object case. Ensure references to lib_rule_data, _trusted_task_rules_schema, is_object and j.validate_schema remain unchanged.policy/lib/tekton/trusted_test.rego (1)
433-444:⚠️ Potential issue | 🟡 MinorMake this deny extraction test fail for the right reason.
The deny pattern doesn’t match the tested key (
oci://registry.local/crook:1.0), and the assertion still passes because there are no allow rules. Add an allow rule that would trust the task unless the deny rule is applied.Proposed test fix
- data_rules_deny := {"deny": {"Deny from data": {"pattern": "oci://registry.local/crook/*"}}} + data_rules_deny := { + "allow": {"Allow registry.local": {"pattern": "oci://registry.local/*"}}, + "deny": {"Deny from data": {"pattern": "oci://registry.local/crook:*"}}, + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@policy/lib/tekton/trusted_test.rego` around lines 433 - 444, The test currently only provides a deny rule so the task is untrusted for the wrong reason; update data_rules_deny to include an explicit allow rule that would match the tested bundle identifier so the task would be trusted except when the deny rule applies (i.e. add an "allow" entry in data_rules_deny whose pattern matches the task bundle key used in denied_task such as "oci://registry.local/crook:1.0" or a suitable wildcard like "oci://registry.local/crook:*"), keeping the deny rule as-is and leaving the tekton.is_trusted_task call and data.rule_data.trusted_task_rules as null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@policy/lib/tekton/trusted_test.rego`:
- Around line 433-444: The test currently only provides a deny rule so the task
is untrusted for the wrong reason; update data_rules_deny to include an explicit
allow rule that would match the tested bundle identifier so the task would be
trusted except when the deny rule applies (i.e. add an "allow" entry in
data_rules_deny whose pattern matches the task bundle key used in denied_task
such as "oci://registry.local/crook:1.0" or a suitable wildcard like
"oci://registry.local/crook:*"), keeping the deny rule as-is and leaving the
tekton.is_trusted_task call and data.rule_data.trusted_task_rules as null.
In `@policy/lib/tekton/trusted.rego`:
- Around line 268-276: The current data_errors rule only validates
trusted_task_rules when lib_rule_data("trusted_task_rules") returns an object,
which silently skips malformed non-object values (e.g., legacy arrays or
scalars); update the rule so it also detects when rule_data_rules exists but is
not an object and emits a policy error. Concretely, inside the data_errors rule
(and/or a new clause alongside the is_object check) add a guard that checks
lib_rule_data("trusted_task_rules") (symbol: rule_data_rules) is present and not
an object (use the existing is_object predicate and check for inequality to
[]/empty), and produce an appropriate error entry instead of silently skipping;
keep the existing j.validate_schema(rule_data_rules, _trusted_task_rules_schema)
path for the object case. Ensure references to lib_rule_data,
_trusted_task_rules_schema, is_object and j.validate_schema remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 74bdef59-50e7-4515-b002-597f21396e0c
📒 Files selected for processing (4)
acceptance/testdata/trusted-task-rules-data.ymlpolicy/lib/tekton/trusted.regopolicy/lib/tekton/trusted_test.regopolicy/release/trusted_task/trusted_task_test.rego
✅ Files skipped from review due to trivial changes (1)
- policy/release/trusted_task/trusted_task_test.rego
Add regal ignore directives for in-wildcard-key lint — the two-variable form (some _, entry in obj) is required to iterate over object values rather than keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
When multiple data sources each define trusted_task_rules entries, OPA fails to merge them because arrays at the same path produce a merge error at data load time, before Rego evaluation begins. This change switches allow and deny from arrays to objects keyed by a unique name, allowing OPA to deep-merge entries from multiple data sources without conflict.