fix(rules/java): match non-literal args in role/permission checks

[boorad]

Summary

Five Java method-call rules required the role/permission/feature argument to be a string literal. In idiomatic Java that argument is usually a constant (Role.ADMIN, Permissions.USER_DELETE) or a variable — parsed by tree-sitter as field_access or identifier, not string_literal. As a result, calls like acct.hasRole(Role.ADMIN) and account.hasRole(role) were silently missed even though the same call with a string literal was caught.

Verified against the endorsed_resume_src.tar.gz attached to #7:

Before this PR — 4 findings, no hasRole calls detected.

After this PR — 11 findings, including all 7 previously-missed hasRole call sites:

AdminApiController.java:337  target.hasRole(Role.ADMIN)
AdminApiController.java:391  target.hasRole(Role.EMPLOYEE)
SecureControllerBase.java:47 acct.hasRole(Role.ADMIN)
SecureControllerBase.java:60 acct.hasRole(Role.EMPLOYEE)
AdminService.java:122        account.hasRole(Role.ADMIN)
AdminService.java:124        account.hasRole(Role.EMPLOYEE)
AdminService.java:205        a.hasRole(role)              ← variable arg

Changes

For each affected rule, expand the argument pattern from a single string_literal to a tree-sitter [...] choice over string_literal | field_access | identifier, all captured under the same name:

• rules/java/has-role-call.toml — hasRole/hasAnyRole/hasAuthority/hasAnyAuthority
• rules/java/is-user-in-role.toml — request.isUserInRole(...)
• rules/java/role-equals-check.toml — getRole().equals(...)
• rules/java/shiro-is-permitted.toml — subject.isPermitted(...)/checkPermission(...)
• rules/java/feature-gate-check.toml — hasFeature/isFeatureEnabled/hasPlan/...

Annotation rules (@Secured, @PreAuthorize, @RolesAllowed, @RequiresPermissions, @RequiresRoles) are intentionally not changed — annotation arguments are nearly always string literals in practice and the existing pattern is correct for the common case.

Rego stub behaviour for non-literal args

The Rego stub substitutes the captured expression text verbatim:

# acct.hasRole(Role.ADMIN)  →
allow if {
    input.user.granted_authority in {"Role.ADMIN"}
}

# a.hasRole(role)  →
allow if {
    input.user.granted_authority in {"role"}
}

These are imperfect — they need manual review before they're useful — but the finding is now recorded with the original snippet, which was the user's main complaint in #7. Improving the stub for non-literal cases (e.g., emitting a # TODO: resolve constant value comment) is a separate, smaller change that would benefit from --deep LLM-assisted resolution down the road.

Tests

• 8 new cargo test cases in src/scanner/matcher.rs (one per non-literal form per affected rule)
• 8 new [[rule.tests]] toml fixtures (run via zift rules test)

$ cargo test
test result: ok. 92 passed; 0 failed; 0 ignored
$ cargo run --quiet -- rules test
… 99 passed, 5 failed.

The 5 rules test failures are pre-existing and unrelated to this PR:

• 4 from java-security-interface-impl — fixed in fix(rules/java): repair invalid query in security-interface-impl #11 (already merged), so this branch will rebase clean.
• 1 from java-access-decision-voter[1] — flagged in fix(rules/java): repair invalid query in security-interface-impl #11 as a separate follow-up.

Fixes #7.

Test plan

• cargo fmt -- --check
• cargo clippy --tests -- -D warnings
• cargo test
• Re-scan endorsed_resume_src.tar.gz from Zift did not discover all instances of authorization, even though they follow patterns similar to the ones that it did find #7 — all 7 missed hasRole calls now detected
• Re-scan vbench-server-src.tar.gz from Zift did not find the major authorization logic in this app #6 — same baseline findings (no false positives introduced)

Summary by CodeRabbit

• Improvements
  • Enhanced Java security rule detection to capture authorization arguments (roles, permissions, feature gates) when provided as constants or variables, not just string literals.
• Tests
  • Expanded test coverage with positive cases for non-literal argument forms to ensure broader matching across authorization checks.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 160f14b3-00fc-43a8-b56f-d74edf591e93

📥 Commits

Reviewing files that changed from the base of the PR and between b30c167 and d86d78d.

📒 Files selected for processing (3)

• rules/java/is-user-in-role.toml
• rules/java/shiro-is-permitted.toml
• src/scanner/matcher.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• rules/java/is-user-in-role.toml
• src/scanner/matcher.rs

---

📝 Walkthrough

Walkthrough

Five Java rules broaden Tree-sitter queries to match method/field arguments as string literals, field accesses, or bare identifiers; corresponding tests and Rust matcher unit tests were added to assert detection of these non-literal argument patterns.

Changes

Cohort / File(s)	Summary
Authorization & Feature Rule TOMLs rules/java/feature-gate-check.toml, rules/java/has-role-call.toml, rules/java/is-user-in-role.toml, rules/java/role-equals-check.toml, rules/java/shiro-is-permitted.toml	Tree-sitter query captures expanded from only (string_literal ...) to include (field_access ...) and (identifier ...). Added positive [[rule.tests]] for field-access and identifier argument forms.
Matcher Tests src/scanner/matcher.rs	Added unit tests asserting findings for non-literal argument patterns (field-access constants and identifier variables) across multiple Java authorization/feature checks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: add Java language support with 18 authz detection rules #3: Introduced the original Java authorization/feature-gate rule queries and baseline matcher tests that this change extends.

Poem

🐰 I hopped through code with eager eyes,
Now Role.ADMIN and var replies,
Field and name both caught in sight,
No literal left out of the night. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: expanding Java rules to match non-literal arguments in role/permission checks.
Linked Issues check	✅ Passed	The PR directly addresses issue #7 by expanding five authorization rules to match non-literal role/permission/feature arguments (field_access and identifier), enabling detection of previously-missed hasRole and related calls.
Out of Scope Changes check	✅ Passed	All changes are scoped to expanding rule queries and adding tests; no unrelated refactoring or out-of-scope modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[amazon-q-developer]

This PR successfully expands Java authorization rule patterns to detect role/permission checks with non-literal arguments (field accesses and identifiers), addressing the issue where calls like acct.hasRole(Role.ADMIN) were previously missed.

The implementation is sound across all five affected rule files, with consistent pattern expansion from single string_literal nodes to tree-sitter choice syntax accepting string_literal | field_access | identifier. The addition of 8 comprehensive test cases validates the new matching behavior.

The Rego stub limitation (substituting non-literal expressions verbatim) is appropriately acknowledged and doesn't block functionality - findings are correctly captured for manual review.

Approval: No blocking issues found. The changes are well-structured, thoroughly tested, and 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]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@rules/java/is-user-in-role.toml`:
- Around line 43-52: Add a positive test case covering identifier/variable role
arguments so variable-based calls don't regress: in the [[rule.tests]] section
add a new input snippet where request.isUserInRole(roleVar) (e.g., roleVar set
to Role.ADMIN or assigned from Role.ADMIN) is used and set expect_match = true;
reference the existing request.isUserInRole usage and name the new fixture
roleVar to parallel the existing Role.ADMIN test.

In `@rules/java/shiro-is-permitted.toml`:
- Around line 53-62: The test suite is missing a positive test for the
identifier-argument branch introduced by the pattern `(identifier) `@perm_value``;
add a new [[rule.tests]] case that defines a variable (e.g., String permVar =
Permissions.USER_DELETE or var permVar = Permissions.USER_DELETE) and calls
subject.isPermitted(permVar) so the query binds `perm_value` via identifier;
ensure expect_match = true and the input source includes the identifier usage
(subject.isPermitted(permVar)) to verify the `(identifier) `@perm_value`` branch
matches.

In `@src/scanner/matcher.rs`:
- Around line 381-391: The tests like
java_is_user_in_role_non_literal_arg_matches currently only cover field-access
args; add parallel identifier-based test cases that call parse_and_match_java
with a call using a bare identifier (e.g., request.isUserInRole(role) or
isUserInRole(roleVar)) against the same rule file (is-user-in-role.toml), assert
findings is not empty, and do the same pattern for the two other Java tests at
the other locations; use the same test helpers (parse_and_match_java) and keep
test names indicating "identifier_arg_matches" to mirror the existing
field-access tests for future parity and safety.

🪄 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: 57323497-aa70-4a3a-be04-61974a4644b6

📥 Commits

Reviewing files that changed from the base of the PR and between 9b0134c and b30c167.

📒 Files selected for processing (6)

• rules/java/feature-gate-check.toml
• rules/java/has-role-call.toml
• rules/java/is-user-in-role.toml
• rules/java/role-equals-check.toml
• rules/java/shiro-is-permitted.toml
• src/scanner/matcher.rs