feat(rules): corpus shakedown rule pass — coverage for Gitea, Ghost, Cal.com, OpenMRS, Zulip#50
Merged
feat(rules): corpus shakedown rule pass — coverage for Gitea, Ghost, Cal.com, OpenMRS, Zulip#50
Conversation
…cates Sister rule to go-has-role-call/go-permission-check-call. Those require a string-literal argument; this rule covers the very common Go shape where the call is a boolean predicate (IsAdmin(), IsOwner(), CanRead(unit), HasAnyUnitAccess()). Surfaced by the corpus shakedown on Gitea, where the entire permission model is hand-rolled around predicates of this shape and none matched any existing rule. No rego template — the role/permission isn't recoverable from the AST, so any stub would mislead. Predicate names are explicit alternations rather than a broad Has[A-Z]... to avoid duplicate findings with the existing string-literal rules. Confidence is medium.
…ip helpers Two changes from the corpus shakedown on Ghost and Cal.com: - New ts-chained-permission-call: matches the shape `<callee>(...).<verb>.<resource>(...)` used by Ghost's permission DSL (canThis(user).edit.post(post)) and by CASL/Pundit ports. Predicate is a prefix match on can/may/allow/check/ability. Medium confidence; no rego template (verb/resource are identifiers, not literals). - Widen ts-permission-check-call predicate to include belongsTo, isTeamAdmin, isOrgOwner, hasMembership, and the userIs<Role> family. Cal.com uses these as first-class authz checks but they were previously unmatched.
Cal.com Next.js handlers use `if (session.user.role !== "ADMIN")` as a forbidden-guard pattern. The existing query already supported nested member access on the LHS (the rightmost property is captured), but restricted the operator to `==`/`===`. Extend operators to include `!=` and `!==`, capture the operator as @op for future use, and add `roleName` to the predicate alternation (common in NextAuth-style session shapes). Note: the rego template still emits `==`. For `!==` matches the generated stub will be operator-inverted from the original intent — the policy decision point itself is the value here; operator polarity is a small manual fix at extract time. Documented inline.
Surfaced by the corpus shakedown on Zulip, where the actual policy
decision points are spelled `check_can_*`, `check_basic_*`,
`check_message_*access`, `check_stream_*access`, `check_*_access`,
`check_*_permission`, `check_*_authz` — bare module functions invoked
from views/lib code. Sister rule to py-permission-check-call (which
only matches attribute calls like user.can("delete")).
No rego template: the policy intent is in the function name itself.
Anchored alternation requires a permission/access keyword in the
suffix to avoid validation helpers like check_message_format. Medium
confidence.
…ized OpenMRS's primary policy surface is `@Authorized({"Manage Users"})` applied to service methods. Surfaced by the corpus shakedown — 24 files use it; we matched zero. Two query patterns cover the single- and array-string forms; both feed the same role_value capture so the rego template emits a single privilege-check shape against input.user.privileges.
Surfaced by the corpus shakedown on Zulip, where view functions are gated by @require_realm_admin, @require_organization_member, @require_member_or_admin, etc. Sister rule to py-login-required-decorator (which matches the exact name login_required); this matches the open-ended require_<role> family. Accepts both decorator forms (marker/call) as bare identifier or attribute reference, mirroring py-login-required-decorator's shape. Predicate uses a lowercase suffix to exclude HTTP method gates like @require_GET / @require_POST from django.views.decorators.http.
tRPC's idiomatic authz pattern is a procedure builder (protectedProcedure, adminProcedure, authedProcedure, ...) chained into each endpoint declaration. Each chain is a distinct policy decision point — the builder injects the authz middleware into everything built from it. Surfaced by the corpus shakedown on Cal.com. Match shape fires exactly once per chain (leftmost member access whose object is a bare identifier), so multi-method chains yield a single finding. Predicate is [a-z][A-Za-z]*Procedure so well-known and project-specific builders both qualify. No rego template — the policy lives in the builder's middleware, not the call site.
Surfaced by the corpus shakedown on Ghost: the report was dominated by
jwt.sign(...) calls — token issuance for outbound integrations, not
authz decisions. The original ts-jwt-token-check rule conflated:
- verify/decode: extract claims a request will be authorized against
(real authz decision point)
- sign: issue a token to send outbound (crypto, not authz)
Split into two rules:
- ts-jwt-verify-decode: middleware category, medium confidence,
drop-in replacement for the authz signal of the old rule.
- ts-jwt-sign-issue: custom category, low confidence, kept (rather
than dropped) because key handling / audience scoping are still
relevant during a security review and users can disable a single
rule by ID.
BREAKING CHANGE: rule ID `ts-jwt-token-check` is removed. Users
referencing it in config/filters should switch to the verify-decode
or sign-issue replacements.
|
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 (4)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis PR adds multiple new authorization detection rules across Go, Java, Python, and TypeScript; splits TypeScript JWT checks into verify/decode and sign-issue rules; updates two TypeScript permission-role rules; and registers all rule TOML files in the embedded rules list. ChangesAuthorization rule additions & registry wiring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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
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 authorization rule coverage for Gitea, Ghost, Cal.com, OpenMRS, and Zulip. The implementation is solid with 6 new rules, 2 widened rules, and a well-justified breaking change to split JWT token handling. All 288 rule tests pass, and the code follows established patterns. The changes are ready to merge.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@rules/typescript/chained-permission-call.toml`:
- Around line 13-19: The TOML rule lacks a Rego/OPA template stub so the
`extract` output has no policy artifact; add a minimal conservative Rego
template in the same "template" section of
rules/typescript/chained-permission-call.toml that defines a simple package
(e.g., package rules.chained_permission_call) and a single stub rule (e.g., deny
or allow) that accepts inputs and returns a decision, referencing the captured
identifiers (the verb/resource pair such as canThis/mayDo/allowFor/checkAbility)
as input.action/input.resource or as metadata so the generated policy compiles;
keep it intentionally imprecise (no enforcement logic), include a short comment
explaining it's a stub for manual review, and ensure the template key name
matches the rule’s existing template reference so `extract` will emit the policy
artifact.
In `@rules/typescript/permission-check-call.toml`:
- Around line 7-14: The ts-permission-check-call rule currently mixes
role/membership helper method families (belongsTo, isTeamAdmin, isOrgOwner,
hasMembership, userIs*) into its permission-extraction regex; remove those names
from the permission-oriented regex and related capture handling in
ts-permission-check-call and instead create a separate rule (e.g.,
ts-role-membership-check or similar) that contains a regex and extraction logic
specifically for belongsTo|isTeamAdmin|isOrgOwner|hasMembership|userIs*; update
the original permission rule to only match permission-oriented verbs (can,
hasPermission, checkPermission, isAllowed, allows, hasAccess, checkAccess,
hasClaim and deny-style variants) and ensure the new role rule validates that
the captured argument is a resource/role identifier rather than a permission
value.
🪄 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: bfb08787-5f0a-48e7-beb5-a9f3ec2ff96c
📒 Files selected for processing (12)
rules/go/permission-predicate-call.tomlrules/java/authorized-annotation.tomlrules/python/check-helper-call.tomlrules/python/require-decorator.tomlrules/typescript/chained-permission-call.tomlrules/typescript/jwt-sign-issue.tomlrules/typescript/jwt-token-check.tomlrules/typescript/jwt-verify-decode.tomlrules/typescript/permission-check-call.tomlrules/typescript/role-check-conditional.tomlrules/typescript/trpc-procedure.tomlsrc/rules/embedded.rs
💤 Files with no reviewable changes (1)
- rules/typescript/jwt-token-check.toml
boorad
changed the title
Two Major findings, both fixed: 1. ts-chained-permission-call lacked a rego template. Added a stub that substitutes the captured @verb / @resource identifiers, with a TODO comment noting the captures are identifiers (not runtime values) and manual review is required. Mirrors the convention already used by java-spring-preauthorize. 2. ts-permission-check-call mixed role/membership helpers (belongsTo, isTeamAdmin, isOrgOwner, hasMembership, userIs<Role>) with permission verbs. The membership shape's literal arg is a resource identifier (e.g. "team-42"), not a permission name, so the existing template `input.action == "{{permission}}"` would emit misleading rego. Reverted the predicate widening on ts-permission-check-call and moved the membership family to a new ts-membership-check-call rule with category=rbac and a membership-shaped stub template (`"{{resource_id}}" in input.user.memberships`). Preserves the project's role-vs-permission separation.
This was referenced May 4, 2026
Closed
Closed
Closed
Closed
Merged
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
Rule-coverage pass driven by plans/todo/05-corpus-shakedown-followups.md (P0 #1–#4, P1 #5–#7, P2 #9). One item per commit so any can be dropped in review without blocking the rest.
Net: 8 rule changes; rule count 53 → 59; rule tests 225 → 288 (all passing). Clippy clean, fmt clean.
New rules
go-permission-predicate-call— Gitea-shapeIsAdmin(),IsOwner(),CanRead(unit),HasAnyUnitAccess()(no string-literal arg).ts-chained-permission-call— Ghost/CASL/Pundit-stylecanThis(user).edit.post(post).py-check-helper-call— Zulip-shapecheck_can_*,check_basic_*_access,check_*_permission.java-authorized-annotation— OpenMRS@Authorized({"PRIV"}).py-require-decorator— Zulip@require_realm_admin,@require_member_or_admin, ... (excludes@require_GET/@require_POST).ts-trpc-procedure— tRPCprotectedProcedure/adminProcedurechained builders.Widened rules
ts-permission-check-call— addsbelongsTo,isTeamAdmin,isOrgOwner,hasMembership,userIs<Role>.ts-role-check-conditional— accepts!=/!==(Cal.com guard pattern) androleName.Breaking
ts-jwt-token-checkis split into:ts-jwt-verify-decode(middleware, medium) — drop-in for the authz signal.ts-jwt-sign-issue(custom, low) — outbound token issuance, kept rather than dropped so users can disable a single rule by ID.Marked as
feat!:to bump0.1.x → 0.2.0per the version policy in CLAUDE.md.Sequencing
Scanner/output items from the same plan doc (P2 #10, P3 #11a, #11, #12) are deferred to a follow-up PR — different code area, different review surface.
Test plan
cargo buildcargo test(325 unit + 30 integration + doc tests)cargo run -- rules validate(59 rules)cargo run -- rules test(288 / 288 passing)cargo fmt --checkcargo clippy --all-targets -- -D warningscargo run -- scan <gitea-clone>to confirmgo-permission-predicate-calllights up the Gitea perm/access tree (target: ~200+ findings vs 18 before).ts-jwt-sign-issueseparates cleanly fromts-jwt-verify-decodeandts-chained-permission-calllights upcore/server/.ts-role-check-conditionalmatchessession.user.role !== "ADMIN"cases andts-trpc-procedurematches the procedure-builder endpoints.java-authorized-annotationmatches the 24@Authorizedfiles.py-check-helper-callandpy-require-decoratorlight upzerver/views/andzerver/lib/.Summary by CodeRabbit
New Features
Changes
Removed