Conversation
Wire the C# tree-sitter grammar into structural scanning and add ASP.NET Core authorization rules for attributes, endpoint authorization, claims, roles, and IAuthorizationService checks. Update language support docs so C# is listed as structurally supported.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR adds C# structural language support and a set of ASP.NET Core authorization rules. It introduces the tree-sitter C# dependency, enables ChangesC# Language Support & ASP.NET Core Authorization Rules
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 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 |
There was a problem hiding this comment.
This PR successfully adds comprehensive C# structural scanning support following established patterns from other languages. The implementation includes tree-sitter parser integration, policy import detection with DI propagation, and complete test coverage for ASP.NET Core authorization patterns.
The code quality is high with proper error handling, comprehensive test coverage for import detection, propagation logic, and edge cases. No blocking defects 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.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/DESIGN.md`:
- Line 217: The C# entry ("| C# | ASP.NET Core `[Authorize]`, policy-based
authorization, `ClaimsPrincipal` checks |") is incorrectly placed in Priority 2
(v0.2); move that table row into the Priority 1 (v0.1) section (or alternatively
add an explicit inline note next to the C# row clarifying it is available now)
so documentation matches shipped support; update the table row under the v0.1
heading or add a parenthetical status note to the existing C# line accordingly.
In `@rules/csharp/aspnet-authorize-policy-shorthand.toml`:
- Around line 23-24: The current not_match only excludes explicit Roles/Policy
named args but still allows any string that follows a named argument (e.g.,
AuthenticationSchemes = "Bearer") to be treated as a shorthand; update the
predicate to also exclude any named-argument pattern by expanding not_match to
include a generic named-argument regex (for example add "\\b\\w+\\s*=" to the
alternation) so strings that are values of named args are ignored; apply the
same change to the other rule block referenced (lines 44-57) and keep references
to the existing rule.predicates.match and its not_match key when making the
edits.
In `@rules/csharp/authorization-service-authorize-async.toml`:
- Around line 18-19: The rule in rule.predicates.arguments currently only
matches string literals, Operations.* and capitalized identifiers; update the
match regex to also accept variable-backed identifiers used in real
IAuthorizationService calls (e.g. AuthorizeAsync(User, resource, policyName) and
AuthorizeAsync(User, resource, requirements)) by adding an alternative that
matches identifier names containing "policy" or "requirement" (case-insensitive)
and their plural forms; modify the match value (the match = "...") to include
something like an additional branch:
|(?i:[a-zA-Z_][a-zA-Z0-9_]*?(policy|requirement)s?) so variable names like
policyName or requirements are captured by the rule.
In `@src/rego/templates.rs`:
- Around line 14-16: The current branch uses key.ends_with("_set") to allow raw
interpolation (variables key, caps, strip_quotes), which is too broad; instead,
restrict to an explicit allowlist of derived keys (e.g., "roles_set") by
replacing the condition with a check against a static/const list or a matches!
pattern (for example allowed_keys.contains(key.as_str()) or
matches!(key.as_str(), "roles_set")). Update the match arm that returns
val.to_string() to only trigger for those explicit keys and keep the other arms
unchanged so unknown "_set" keys still go through strip_quotes or return the
placeholder.
In `@src/scanner/imports.rs`:
- Around line 464-473: The code currently calls node.utf8_text(source) and tests
that whole `using` directive text with is_policy_path, causing false positives;
instead locate the right-hand-side target node (the aliased type/path) from the
`using` node (e.g. via node.child_by_field_name for the target/path/value node
or by traversing children) and call utf8_text(source) on that target node, then
pass that target text to is_policy_path; only insert alias into policy_names
when the RHS text matches is_policy_path (keep existing references to
alias_node, alias, is_policy_path, and policy_names).
- Around line 477-494: The code is incorrectly treating plain namespace using
directives as bindings; change the logic so you only insert a policy binding
when the qualified name comes from an alias or a type reference declaration
rather than a bare using namespace. Concretely, when walking the node (the
variable named `target` returned from node.named_children), first detect the
surrounding construct (e.g. check node.kind() / parent node kind) and only call
policy_names.insert(binding) if the node represents an alias using (a
using-alias directive) or if the qualified name is used as a type in
declarations/parameters/fields (not a plain using directive); update the branch
that currently calls trailing_qualified_name_segment(target, source) and
policy_names.insert(binding) to require that the original node is an alias or a
type reference before inserting, leaving is_policy_path(full_text) as an early
filter.
🪄 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: 78a0586c-1462-4a7e-9cf8-fd3ecfe2e7d9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
.githooks/pre-commit.github/PULL_REQUEST_TEMPLATE.mdAGENTS.mdCONTRIBUTING.mdCargo.tomlREADME.mddocs/DESIGN.mdrules/csharp/aspnet-allow-anonymous.tomlrules/csharp/aspnet-authorize-attribute.tomlrules/csharp/aspnet-authorize-policy-shorthand.tomlrules/csharp/aspnet-authorize-policy.tomlrules/csharp/aspnet-authorize-roles.tomlrules/csharp/aspnet-endpoint-allow-anonymous.tomlrules/csharp/aspnet-require-authorization.tomlrules/csharp/authorization-service-authorize-async.tomlrules/csharp/has-claim-call.tomlrules/csharp/is-in-role-call.tomlsrc/deep/candidate.rssrc/deep/context.rssrc/rego/templates.rssrc/rules/embedded.rssrc/scanner/discovery.rssrc/scanner/imports.rssrc/scanner/matcher.rssrc/scanner/parser.rstests/scanner_enforcement_points.rs
Summary
Adds C# structural scanning for ASP.NET Core authorization patterns and wires C# into policy-import enforcement-point detection.
Changes
.csdiscovery.[Authorize],[AllowAnonymous], endpoint authorization,AuthorizeAsync,IsInRole, andHasClaim.[Authorize("PolicyName")]detection and fixes generated Rego for comma-separated roles and claim values.usingaliases, constructor parameters, assignments, fields, and object initializers.Testing
cargo check --all-featurescargo clippy --all-features -- -D warningscargo fmt --checkcargo test --all-featurescargo testcargo run -- rules validatecargo run -- rules testSummary by CodeRabbit
New Features
Documentation
Chores