feat: add Cedar as a second policy engine#72
Merged
Merged
Conversation
Phase A of #27 — adds Cedar/AVP as a peer to Rego/OPA without breaking the existing JSON or MCP API. - `Finding::cedar_stub` parallel to `rego_stub` (serde-defaulted, BC) - `[rule.<id>.cedar_template]` parallel to `rego_template`; 45 existing rules carry an inferred Cedar template, the rest fall through to a category-default Cedar stub so coverage stays at 100% - new `src/cedar/` module: templates, validator (cedar-policy 4.x), per-source-file grouping with `.cedar` extension - `zift extract --engine rego|cedar` (default rego); `--package-prefix` renamed to `--policy-prefix` with the old name kept as an alias - new MCP tools `suggest_policy` / `validate_policy` taking an `engine` arg; `suggest_rego` / `validate_rego` retained as Rego-pinned aliases so existing agent host wiring keeps working - `zift rules validate` now type-checks Cedar templates too
Phase B is now tracked as its own work item (#71). Removed the 'wait for real users' gate and the 'JSON-schema break needs a major version bump' gate. The *_stub → policy_outputs migration ships with a deserialization shim that reads the legacy fields, so it doesn't need a coordinated version cut — that shim is part of Phase B's scope, not a separate migration.
Several rule cedar_template blocks (mostly C#) hardcoded "TODO" instead of substituting captured values, while their Rego siblings expanded placeholders correctly. Output for those rules was strictly worse than the category-default stub. Use the captured placeholders, and add a cedar-flavored derived-vars helper (`add_cedar_template_derived_values` producing `cedar_roles_set`) so multi-role rules like `csharp-aspnet-authorize-roles` can emit a real Cedar set membership check. Wrap Cedar regexes in OnceLock and hoist `output_dir.canonicalize()` out of the per-file extract loop.
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughAdds Cedar policy engine support: new dependency, cedar module (templates, grouping, validator), CLI engine flag, cedar_template fields in rule TOMLs, cedar_stub on Finding, engine-dispatched extraction and validation, and updates across scanner, MCP tools, tests, and docs. ChangesCedar Policy Engine Integration
Sequence DiagramsequenceDiagram
participant CLI as CLI
participant Scanner as Scanner/Matcher
participant CedarMod as cedar::(templates/validator/grouping)
participant FileSys as Output Files
CLI->>Scanner: run scan with --engine=cedar
Scanner->>Scanner: match patterns, capture vars
Scanner->>CedarMod: render `cedar_template` → `cedar_stub`
CedarMod->>CedarMod: validate_template(rendered)
Scanner->>CedarMod: pass findings with cedar_stub
CedarMod->>CedarMod: group_findings → per-file CedarFile(s)
CedarMod->>FileSys: write_policy_file(per-file content)
FileSys->>CLI: return summary (files written, validation warnings)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR successfully adds Cedar policy engine support alongside Rego, implementing a well-architected parallel pipeline. The implementation is comprehensive with 444 passing tests and includes proper validation, template rendering, and MCP tool integration.
Key Strengths:
- Clean architecture mirroring the existing Rego implementation
- Comprehensive test coverage for Cedar templates, validation, and grouping
- Proper TOCTOU-safe path traversal security checks maintained
- Backward compatibility preserved with
#[serde(default)]for new fields - Engine-agnostic MCP tools (
suggest_policy,validate_policy)
The code is production-ready and no blocking issues were identified.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
5 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/cedar/templates.rs (1)
18-21: 💤 Low valueMinor: Regex can match mismatched quotes.
The pattern
["']([^"']+)["']will match strings with mismatched opening/closing quotes (e.g.,"foo'or'bar"). In practice this is unlikely to occur in real code snippets, but for correctness you could use backreferences or alternation.🔧 Optional fix using alternation
fn string_literal_re() -> &'static Regex { static RE: OnceLock<Regex> = OnceLock::new(); - RE.get_or_init(|| Regex::new(r#"["']([^"']+)["']"#).unwrap()) + RE.get_or_init(|| Regex::new(r#""([^"]+)"|'([^']+)'"#).unwrap()) }Note: This would require adjusting
extract_string_literalsto check both capture groups.🤖 Prompt for 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. In `@src/cedar/templates.rs` around lines 18 - 21, The regex in string_literal_re() allows mismatched quotes (e.g., "foo') because it uses ["']([^"']+)["']; update the pattern to enforce matching open/close quotes (use a backreference like (?P<q>["'])(?P<s>[^"']+)(?P=q) or alternation like "([^"]+)"|'([^']+)' in the Regex::new call inside string_literal_re(), and then update any consumer (e.g., extract_string_literals) to read the correct capture group (either the named group "s" or the appropriate numbered group when using alternation) so string values are extracted reliably with matching quotes.
🤖 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 `@rules/java/authorized-annotation.toml`:
- Around line 43-53: The Cedar template currently hardcodes principal.role ==
"TODO" which ignores the captured `@role_value`; update the [rule.cedar_template]
so the principal.role comparison uses the captured authorization value (the
`@Authorized`(...) capture, e.g. role_value) instead of the literal "TODO",
ensuring the template injects/quotes the role_value correctly when rendering;
look for the template string containing principal.role and replace the literal
with the template variable that maps to the captured `@role_value`.
In `@rules/java/has-role-call.toml`:
- Around line 39-49: The Cedar template in the [rule.cedar_template] block
currently hardcodes principal.role == "TODO"; change this to use the captured
role variable instead of the literal "TODO" by substituting the capture (the
`@role_value` captured by the rule) into the template so the condition reads
principal.role == <captured role value>, keeping the Cedar output consistent
with the Rego capture and the rule's `@role_value` identifier.
In `@rules/java/shiro-requires-roles-array.toml`:
- Around line 29-39: The Cedar template in [rule.cedar_template] uses a singular
capture `{{role_value}}` causing multi-role arrays to collapse; change the
capture/templating flow so all captured role_value entries are aggregated into a
single roles set (e.g., derive a `cedar_roles_set` from all role_value captures)
and render the condition using set membership (either
`{{cedar_roles_set}}.contains(principal.role)` or `principal.role in
[{{cedar_roles_set}}]`) instead of `principal.role == "{{role_value}}"`; follow
the approach used in the `csharp-aspnet-authorize-roles` rule where a
comma-separated roles string is turned into `cedar_roles_set` before template
rendering, or modify the capture step to produce a `roles` variable so the
template can directly use `cedar_roles_set`.
In `@rules/java/spring-roles-allowed-array.toml`:
- Around line 29-39: The Cedar template under rule.cedar_template currently uses
singular equality (principal.role == "{{role_value}}"); update the template in
the template string so the condition matches the multi-role derived value used
by the Rego sibling by replacing the equality check with a membership check
using the derived set placeholder (e.g., principal.role in
[{{cedar_roles_set}}]); ensure you reference the existing template symbol and
the cedar_roles_set placeholder when making the change so arrays of roles are
handled correctly.
In `@rules/python/has-perm-call.toml`:
- Around line 36-46: The Cedar template in the rule.cedar_template section is
using a hardcoded principal.role == "TODO" instead of the captured permission
variable; update the template to interpolate the extracted perm_name (the
captured permission) into the condition so generated stubs reflect the matched
permission (use the perm_name placeholder from your rule extraction logic inside
the template string where "TODO" currently appears).
---
Nitpick comments:
In `@src/cedar/templates.rs`:
- Around line 18-21: The regex in string_literal_re() allows mismatched quotes
(e.g., "foo') because it uses ["']([^"']+)["']; update the pattern to enforce
matching open/close quotes (use a backreference like
(?P<q>["'])(?P<s>[^"']+)(?P=q) or alternation like "([^"]+)"|'([^']+)' in the
Regex::new call inside string_literal_re(), and then update any consumer (e.g.,
extract_string_literals) to read the correct capture group (either the named
group "s" or the appropriate numbered group when using alternation) so string
values are extracted reliably with matching quotes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 96beec8a-68ef-4eaf-8a3f-1c3928350014
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
Cargo.tomldocs/CEDAR_SUPPORT.mdrules/csharp/aspnet-authorize-attribute.tomlrules/csharp/aspnet-authorize-policy-shorthand.tomlrules/csharp/aspnet-authorize-policy.tomlrules/csharp/aspnet-authorize-roles.tomlrules/csharp/aspnet-require-authorization.tomlrules/csharp/authorization-service-authorize-async.tomlrules/csharp/has-claim-call.tomlrules/csharp/is-in-role-call.tomlrules/go/access-descriptor-builder.tomlrules/go/casbin-enforce.tomlrules/go/feature-gate-check.tomlrules/go/gin-auth-middleware.tomlrules/go/has-role-call.tomlrules/go/opa-rego-eval.tomlrules/go/ownership-check.tomlrules/go/permission-check-call.tomlrules/go/role-check-conditional.tomlrules/java/authorized-annotation.tomlrules/java/has-role-call.tomlrules/java/is-user-in-role.tomlrules/java/role-equals-check.tomlrules/java/shiro-is-permitted.tomlrules/java/shiro-requires-permissions.tomlrules/java/shiro-requires-roles-array.tomlrules/java/shiro-requires-roles.tomlrules/java/spring-preauthorize.tomlrules/java/spring-roles-allowed-array.tomlrules/java/spring-roles-allowed.tomlrules/java/spring-secured.tomlrules/python/django-permission-required.tomlrules/python/has-perm-call.tomlrules/python/has-role-call.tomlrules/python/ownership-check.tomlrules/python/permission-check-call.tomlrules/python/role-check-conditional.tomlrules/typescript/authorize-function-call.tomlrules/typescript/chained-permission-call.tomlrules/typescript/feature-gate-check.tomlrules/typescript/has-role-call.tomlrules/typescript/membership-check-call.tomlrules/typescript/nestjs-roles-decorator.tomlrules/typescript/ownership-check.tomlrules/typescript/permission-check-call.tomlrules/typescript/role-check-conditional.tomlrules/typescript/role-includes-check.tomlsrc/cedar/grouping.rssrc/cedar/mod.rssrc/cedar/templates.rssrc/cedar/validator.rssrc/cli.rssrc/commands/extract.rssrc/commands/rules.rssrc/deep/candidate.rssrc/deep/context.rssrc/deep/finding.rssrc/deep/merge.rssrc/deep/prompt.rssrc/lib.rssrc/mcp/tools.rssrc/output/json.rssrc/output/text.rssrc/rego/grouping.rssrc/rules/mod.rssrc/scanner/matcher.rssrc/types.rstests/deep_http_integration.rstests/deep_subprocess_integration.rs
- Replace hardcoded "TODO" with captured values in Cedar templates for java-authorized-annotation, java-has-role-call, py-has-perm-call. - Switch java array rules (shiro-requires-roles-array, spring-roles-allowed-array) to cedar_roles_set membership, mirroring the C# aspnet-authorize-roles approach. - Extend add_cedar_template_derived_values to derive cedar_roles_set from role_value when no roles capture exists. - Tighten string_literal_re to require matching quote delimiters via alternation, preventing mismatched-quote pairs.
boorad
commented
May 6, 2026
Contributor
Author
|
@coderabbitai Re: review-body nitpick on |
|
Tip For best results, initiate chat on the files or code changes.
|
Contributor
Author
|
@coderabbitai approve |
✅ Actions performedComments resolved and changes approved. |
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.
Summary
Adds Cedar (the AWS-originated policy language) as a second supported policy engine alongside Rego/OPA. Architecture mirrors the existing Rego pipeline: per-rule
cedar_templateblocks, category-default stubs, avalidate_cedarparser-based validator, and grouping into.cedarfiles. The CLI exposes engine selection viaextract --engine {rego,cedar}and the MCP server gains engine-agnosticsuggest_policy/validate_policytools (the Rego-pinned tools stay as aliases).Changes
src/cedar/{mod,grouping,templates,validator}.rs— Cedar emission and parser-based validation using thecedar-policycratesrc/cli.rs—--engine {rego,cedar}onextract;--policy-prefixwith--package-prefixretained as aliassrc/commands/extract.rs— Cedar pipeline; canonicalizeoutput_dironce per run instead of per-filesrc/mcp/tools.rs—suggest_policy/validate_policyengine-agnostic toolssrc/scanner/matcher.rs—cedar_stubrendering, plus a Cedar-flavored derived-vars helper (cedar_roles_set) so multi-role rules emit a real Cedar set membership checksrc/types.rs—Finding.cedar_stub(serde-default for backward compat);Surfacefield for frontend/backend classificationrules/**/*.toml—cedar_templateblocks added across all C#, Java, Python, Go, and TypeScript rulesdocs/CEDAR_SUPPORT.md— design memoTest plan
cargo build && cargo clippy --all-features -- -D warnings && cargo fmt --checkcargo test --all-features(444 tests, including new cedar grouping/templates/validator tests and a regression test thatcsharp-aspnet-authorize-rolesexpandsRoles = "Admin,Manager"intoprincipal.role in ["Admin", "Manager"])cargo run -- scan tests/fixtures -f json | cargo run -- extract --engine cedar --output-dir /tmp/cedar-outcargo run -- mcpand callsuggest_policy/validate_policywithengine: "cedar"Summary by CodeRabbit
New Features
Documentation
Chores