From b30c16738aa1c947c3c42d7b57f38ddb89a52b83 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Tue, 28 Apr 2026 13:02:01 -0400 Subject: [PATCH 1/2] fix(rules/java): match non-literal args in role/permission checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five 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, both parsed 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. Expand the argument pattern to a tree-sitter `[...]` choice over `string_literal`, `field_access`, and `identifier` for: - has-role-call - is-user-in-role - role-equals-check - shiro-is-permitted - feature-gate-check The Rego stub for non-literal cases substitutes the captured expression text verbatim (e.g. `in {"Role.ADMIN"}`) — imperfect, but the finding is recorded and the snippet shows the original call site for manual review. Verified against the endorsed_resume tarball from #7: scan now reports all 7 previously-missed `hasRole(Role.X)` / `hasRole(role)` call sites across `AdminApiController`, `SecureControllerBase`, and `AdminService`. Adds 8 cargo-test cases (one per non-literal form per affected rule) plus 8 toml `rule.tests` fixtures. Fixes #7. --- rules/java/feature-gate-check.toml | 29 ++++++++++- rules/java/has-role-call.toml | 31 ++++++++++- rules/java/is-user-in-role.toml | 19 ++++++- rules/java/role-equals-check.toml | 29 ++++++++++- rules/java/shiro-is-permitted.toml | 19 ++++++- src/scanner/matcher.rs | 84 ++++++++++++++++++++++++++++++ 6 files changed, 201 insertions(+), 10 deletions(-) diff --git a/rules/java/feature-gate-check.toml b/rules/java/feature-gate-check.toml index 017ec39..5b0e0be 100644 --- a/rules/java/feature-gate-check.toml +++ b/rules/java/feature-gate-check.toml @@ -4,12 +4,17 @@ languages = ["java"] category = "feature_gate" confidence = "medium" description = "Feature flag or plan-based gating in Java" +# Accept string literals as well as field accesses and bare identifiers +# as the feature key; non-literal stubs need manual rego review. query = """ (method_invocation name: (identifier) @method_name arguments: (argument_list - (string_literal - (string_fragment) @feature)) + [ + (string_literal (string_fragment) @feature) + (field_access) @feature + (identifier) @feature + ]) ) @match """ @@ -46,6 +51,26 @@ public class Service { """ expect_match = true +[[rule.tests]] +input = """ +public class Service { + public void check(String featureKey) { + if (featureFlags.hasFeature(featureKey)) { enable(); } + } +} +""" +expect_match = true + +[[rule.tests]] +input = """ +public class Service { + public void check() { + if (featureFlags.isFeatureEnabled(Features.BETA_DASHBOARD)) { enable(); } + } +} +""" +expect_match = true + [[rule.tests]] input = """ public class Service { diff --git a/rules/java/has-role-call.toml b/rules/java/has-role-call.toml index 28787f1..3ac861f 100644 --- a/rules/java/has-role-call.toml +++ b/rules/java/has-role-call.toml @@ -4,12 +4,19 @@ languages = ["java"] category = "rbac" confidence = "high" description = "Spring Security hasRole/hasAuthority method call" +# Accept string literals as well as field accesses (e.g. `Role.ADMIN`) and +# bare identifiers (e.g. `role`) as the role argument. The rego stub treats +# whatever text is captured as the role name; for non-literal expressions +# the stub will need manual review, but the finding is still recorded. query = """ (method_invocation name: (identifier) @method_name arguments: (argument_list - (string_literal - (string_fragment) @role_value)) + [ + (string_literal (string_fragment) @role_value) + (field_access) @role_value + (identifier) @role_value + ]) ) @match """ @@ -68,6 +75,26 @@ public class SecurityConfig { """ expect_match = true +[[rule.tests]] +input = """ +public class Service { + public void check(Account acct) { + if (!acct.hasRole(Role.ADMIN)) { throw new ForbiddenException(); } + } +} +""" +expect_match = true + +[[rule.tests]] +input = """ +public class Service { + public void check(Account acct, String role) { + if (acct.hasRole(role)) { allow(); } + } +} +""" +expect_match = true + [[rule.tests]] input = """ public class SecurityConfig { diff --git a/rules/java/is-user-in-role.toml b/rules/java/is-user-in-role.toml index d1b5ce9..748da29 100644 --- a/rules/java/is-user-in-role.toml +++ b/rules/java/is-user-in-role.toml @@ -4,12 +4,17 @@ languages = ["java"] category = "rbac" confidence = "high" description = "Servlet API isUserInRole() call" +# Accept string literals as well as field accesses and bare identifiers +# as the role argument; non-literal stubs need manual rego review. query = """ (method_invocation name: (identifier) @method_name arguments: (argument_list - (string_literal - (string_fragment) @role_value)) + [ + (string_literal (string_fragment) @role_value) + (field_access) @role_value + (identifier) @role_value + ]) ) @match """ @@ -35,6 +40,16 @@ public class MyServlet { """ expect_match = true +[[rule.tests]] +input = """ +public class MyServlet { + public void doGet() { + if (request.isUserInRole(Role.ADMIN)) { doSomething(); } + } +} +""" +expect_match = true + [[rule.tests]] input = """ public class MyServlet { diff --git a/rules/java/role-equals-check.toml b/rules/java/role-equals-check.toml index c9d3112..3ff7dbc 100644 --- a/rules/java/role-equals-check.toml +++ b/rules/java/role-equals-check.toml @@ -4,14 +4,19 @@ languages = ["java"] category = "rbac" confidence = "medium" description = "Role comparison via .equals() or string equality" +# Accept string literals as well as field accesses (e.g. `Role.ADMIN`) and +# bare identifiers (e.g. `roleVar`) as the comparison argument. query = """ (method_invocation object: (method_invocation name: (identifier) @getter) name: (identifier) @method_name arguments: (argument_list - (string_literal - (string_fragment) @role_value)) + [ + (string_literal (string_fragment) @role_value) + (field_access) @role_value + (identifier) @role_value + ]) ) @match """ @@ -50,6 +55,26 @@ public class Service { """ expect_match = false +[[rule.tests]] +input = """ +public class Service { + public void check() { + if (user.getRole().equals(Role.ADMIN)) { doSomething(); } + } +} +""" +expect_match = true + +[[rule.tests]] +input = """ +public class Service { + public void check(String roleName) { + if (user.getRole().equals(roleName)) { doSomething(); } + } +} +""" +expect_match = true + [[rule.tests]] input = """ public class Service { diff --git a/rules/java/shiro-is-permitted.toml b/rules/java/shiro-is-permitted.toml index 2f224fc..a83169d 100644 --- a/rules/java/shiro-is-permitted.toml +++ b/rules/java/shiro-is-permitted.toml @@ -4,12 +4,17 @@ languages = ["java"] category = "rbac" confidence = "high" description = "Apache Shiro Subject.isPermitted() call" +# Accept string literals as well as field accesses and bare identifiers +# as the permission argument; non-literal stubs need manual rego review. query = """ (method_invocation name: (identifier) @method_name arguments: (argument_list - (string_literal - (string_fragment) @perm_value)) + [ + (string_literal (string_fragment) @perm_value) + (field_access) @perm_value + (identifier) @perm_value + ]) ) @match """ @@ -45,6 +50,16 @@ public class Service { """ expect_match = true +[[rule.tests]] +input = """ +public class Service { + public void delete() { + if (subject.isPermitted(Permissions.USER_DELETE)) { doIt(); } + } +} +""" +expect_match = true + [[rule.tests]] input = """ public class Service { diff --git a/src/scanner/matcher.rs b/src/scanner/matcher.rs index a5dc0ea..e3f762c 100644 --- a/src/scanner/matcher.rs +++ b/src/scanner/matcher.rs @@ -378,6 +378,18 @@ public class Ctrl { assert!(!findings.is_empty(), "should match isUserInRole"); } + #[test] + fn java_is_user_in_role_non_literal_arg_matches() { + let findings = parse_and_match_java( + r#"request.isUserInRole(Role.ADMIN);"#, + include_str!("../../rules/java/is-user-in-role.toml"), + ); + assert!( + !findings.is_empty(), + "should match isUserInRole with field-access arg" + ); + } + #[test] fn java_has_role_call_matches() { let findings = parse_and_match_java( @@ -387,6 +399,30 @@ public class Ctrl { assert!(!findings.is_empty(), "should match hasRole"); } + #[test] + fn java_has_role_call_field_access_arg_matches() { + let findings = parse_and_match_java( + r#"if (!acct.hasRole(Role.ADMIN)) { throw new ForbiddenException(); }"#, + include_str!("../../rules/java/has-role-call.toml"), + ); + assert!( + !findings.is_empty(), + "should match hasRole with field-access arg (e.g., Role.ADMIN)" + ); + } + + #[test] + fn java_has_role_call_identifier_arg_matches() { + let findings = parse_and_match_java( + r#"if (acct.hasRole(roleName)) { allow(); }"#, + include_str!("../../rules/java/has-role-call.toml"), + ); + assert!( + !findings.is_empty(), + "should match hasRole with identifier arg (variable role name)" + ); + } + #[test] fn java_shiro_requires_permissions_matches() { let findings = parse_and_match_java( @@ -410,6 +446,18 @@ public class Ctrl { assert!(!findings.is_empty(), "should match isPermitted"); } + #[test] + fn java_shiro_is_permitted_non_literal_arg_matches() { + let findings = parse_and_match_java( + r#"subject.isPermitted(Permissions.USER_DELETE);"#, + include_str!("../../rules/java/shiro-is-permitted.toml"), + ); + assert!( + !findings.is_empty(), + "should match isPermitted with field-access arg" + ); + } + #[test] fn java_marker_annotation_matches() { let findings = parse_and_match_java( @@ -450,6 +498,30 @@ public class Ctrl { assert!(!findings.is_empty(), "should match getRole().equals()"); } + #[test] + fn java_role_equals_check_field_access_arg_matches() { + let findings = parse_and_match_java( + r#"user.getRole().equals(Role.ADMIN);"#, + include_str!("../../rules/java/role-equals-check.toml"), + ); + assert!( + !findings.is_empty(), + "should match getRole().equals(field-access)" + ); + } + + #[test] + fn java_role_equals_check_identifier_arg_matches() { + let findings = parse_and_match_java( + r#"user.getRole().equals(roleVar);"#, + include_str!("../../rules/java/role-equals-check.toml"), + ); + assert!( + !findings.is_empty(), + "should match getRole().equals(identifier)" + ); + } + #[test] fn java_role_equals_check_no_false_positive() { let findings = parse_and_match_java( @@ -486,6 +558,18 @@ public class Ctrl { assert!(!findings.is_empty(), "should match hasFeature()"); } + #[test] + fn java_feature_gate_non_literal_arg_matches() { + let findings = parse_and_match_java( + r#"featureFlags.hasFeature(Features.BETA_DASHBOARD);"#, + include_str!("../../rules/java/feature-gate-check.toml"), + ); + assert!( + !findings.is_empty(), + "should match hasFeature with field-access arg" + ); + } + #[test] fn dedup_removes_duplicates() { let f = Finding { From d86d78daad9b416638f64339b81e8c7fef3d947b Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Tue, 28 Apr 2026 13:14:10 -0400 Subject: [PATCH 2/2] test(rules/java): cover identifier-arg path for non-literal rules CodeRabbit flagged that the new non-literal-arg branches matched `(identifier)` in the tree-sitter query, but tests only exercised the `(field_access)` form. Add identifier-arg coverage so a regression on the bare-variable path can't slip through unnoticed. - rules/java/is-user-in-role.toml: positive fixture for `roleVar` - rules/java/shiro-is-permitted.toml: positive fixture for `permVar` - src/scanner/matcher.rs: three sibling `_identifier_arg_matches` cargo tests for is-user-in-role, shiro-is-permitted, feature-gate (has-role-call and role-equals-check already had both forms). --- rules/java/is-user-in-role.toml | 10 +++++++++ rules/java/shiro-is-permitted.toml | 10 +++++++++ src/scanner/matcher.rs | 36 ++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/rules/java/is-user-in-role.toml b/rules/java/is-user-in-role.toml index 748da29..885404b 100644 --- a/rules/java/is-user-in-role.toml +++ b/rules/java/is-user-in-role.toml @@ -50,6 +50,16 @@ public class MyServlet { """ expect_match = true +[[rule.tests]] +input = """ +public class MyServlet { + public void doGet(String roleVar) { + if (request.isUserInRole(roleVar)) { doSomething(); } + } +} +""" +expect_match = true + [[rule.tests]] input = """ public class MyServlet { diff --git a/rules/java/shiro-is-permitted.toml b/rules/java/shiro-is-permitted.toml index a83169d..d099611 100644 --- a/rules/java/shiro-is-permitted.toml +++ b/rules/java/shiro-is-permitted.toml @@ -60,6 +60,16 @@ public class Service { """ expect_match = true +[[rule.tests]] +input = """ +public class Service { + public void delete(String permVar) { + if (subject.isPermitted(permVar)) { doIt(); } + } +} +""" +expect_match = true + [[rule.tests]] input = """ public class Service { diff --git a/src/scanner/matcher.rs b/src/scanner/matcher.rs index e3f762c..88a8885 100644 --- a/src/scanner/matcher.rs +++ b/src/scanner/matcher.rs @@ -390,6 +390,18 @@ public class Ctrl { ); } + #[test] + fn java_is_user_in_role_identifier_arg_matches() { + let findings = parse_and_match_java( + r#"request.isUserInRole(roleVar);"#, + include_str!("../../rules/java/is-user-in-role.toml"), + ); + assert!( + !findings.is_empty(), + "should match isUserInRole with identifier arg (variable role name)" + ); + } + #[test] fn java_has_role_call_matches() { let findings = parse_and_match_java( @@ -458,6 +470,18 @@ public class Ctrl { ); } + #[test] + fn java_shiro_is_permitted_identifier_arg_matches() { + let findings = parse_and_match_java( + r#"subject.isPermitted(permVar);"#, + include_str!("../../rules/java/shiro-is-permitted.toml"), + ); + assert!( + !findings.is_empty(), + "should match isPermitted with identifier arg (variable permission)" + ); + } + #[test] fn java_marker_annotation_matches() { let findings = parse_and_match_java( @@ -570,6 +594,18 @@ public class Ctrl { ); } + #[test] + fn java_feature_gate_identifier_arg_matches() { + let findings = parse_and_match_java( + r#"featureFlags.hasFeature(featureKey);"#, + include_str!("../../rules/java/feature-gate-check.toml"), + ); + assert!( + !findings.is_empty(), + "should match hasFeature with identifier arg (variable feature key)" + ); + } + #[test] fn dedup_removes_duplicates() { let f = Finding {