From 42770f8169d7386d2f798cfed4799132fe283e51 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Tue, 28 Apr 2026 13:30:09 -0400 Subject: [PATCH 1/6] fix(scanner/imports): use code-side alias for named ES imports `import { authorize as auth } from "../policy"` previously captured `authorize` (the original name) instead of `auth` (the binding actually used in code). The word-boundary regex in `is_enforcement_point` then failed to match calls like `auth(...)`, misclassifying enforcement points as inline auth findings. Update `TS_NAMED_IMPORT_QUERY` to alternate on the `import_specifier`: capture `alias` when present, otherwise capture `name` (using the `!alias` negative-field operator). Add tests for purely-aliased, mixed, and enforcement-point-via-alias cases. Closes #8 --- src/scanner/imports.rs | 59 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/src/scanner/imports.rs b/src/scanner/imports.rs index 5cdb434..3a59d7a 100644 --- a/src/scanner/imports.rs +++ b/src/scanner/imports.rs @@ -21,12 +21,20 @@ fn is_policy_path(source: &str) -> bool { } /// Tree-sitter query for named imports: `import { foo } from 'bar'` +/// and aliased: `import { foo as bar } from 'baz'`. +/// Captures the binding actually used in code (the alias when renamed, +/// otherwise the original name). const TS_NAMED_IMPORT_QUERY: &str = r#" (import_statement (import_clause (named_imports - (import_specifier - name: (identifier) @name))) + [ + (import_specifier + alias: (identifier) @name) + (import_specifier + !alias + name: (identifier) @name) + ])) source: (string) @source) "#; @@ -189,6 +197,53 @@ import { Router } from 'express'; assert!(!imports.contains("Router")); } + #[test] + fn detects_aliased_named_policy_import() { + let source = r#" +import { authorize as auth, Permission as Perm } from "../policy"; +import { Router as R } from "express"; +"#; + let tree = parse_ts(source); + let imports = find_policy_imports(&tree, source.as_bytes(), Language::TypeScript); + // We capture the binding actually used in code (the alias), not the original name. + assert!(imports.contains("auth")); + assert!(imports.contains("Perm")); + assert!(!imports.contains("authorize")); + assert!(!imports.contains("Permission")); + assert!(!imports.contains("R")); + } + + #[test] + fn detects_mixed_aliased_and_plain_named_imports() { + let source = r#" +import { authorize, can as canDo, evaluate } from "../policy"; +"#; + let tree = parse_ts(source); + let imports = find_policy_imports(&tree, source.as_bytes(), Language::TypeScript); + assert!(imports.contains("authorize")); + assert!(imports.contains("canDo")); + assert!(imports.contains("evaluate")); + assert!(!imports.contains("can")); + } + + #[test] + fn enforcement_point_check_aliased_named_import() { + let source = r#" +import { authorize as auth } from "../policy"; +"#; + let tree = parse_ts(source); + let imports = find_policy_imports(&tree, source.as_bytes(), Language::TypeScript); + // The call uses the alias, so the regex must match the alias binding. + assert!(is_enforcement_point( + r#"if (!auth(req.user, "configs:read", req.params.id)) { return res.status(403).end(); }"#, + &imports, + )); + assert!(!is_enforcement_point( + r#"if (!authorize(req.user, "configs:read", req.params.id)) { return; }"#, + &imports, + )); + } + #[test] fn detects_default_policy_import() { let source = r#" From 5b22d23b4263429cfdbb82f2c306ba5582beb977 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Tue, 28 Apr 2026 13:30:16 -0400 Subject: [PATCH 2/6] test(scanner/matcher): consolidate security-interface-impl tests Collapse the four `java_security_interface_impl_*` tests (plain, fully-qualified, generic, no-false-positive) into a single table-driven test. Failure messages include the case name so per-case context is preserved. Addresses CodeRabbit nitpick on #11. --- src/scanner/matcher.rs | 87 ++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 45 deletions(-) diff --git a/src/scanner/matcher.rs b/src/scanner/matcher.rs index b552a0c..d15253e 100644 --- a/src/scanner/matcher.rs +++ b/src/scanner/matcher.rs @@ -574,67 +574,64 @@ public class Ctrl { } #[test] - fn java_security_interface_impl_matches() { - let findings = parse_and_match_java( - r#" + fn java_security_interface_impl_cases() { + // Table-driven: (case_name, source, expect_match) + let cases: &[(&str, &str, bool)] = &[ + ( + "plain UserDetailsService", + r#" public class MyUserService implements UserDetailsService { public UserDetails loadUserByUsername(String username) { return null; } } "#, - include_str!("../../rules/java/security-interface-impl.toml"), - ); - assert!( - !findings.is_empty(), - "should match implements UserDetailsService" - ); - } - - #[test] - fn java_security_interface_impl_scoped_matches() { - let findings = parse_and_match_java( - r#" + true, + ), + ( + "fully-qualified UserDetailsService", + r#" public class MyUserService implements org.springframework.security.core.userdetails.UserDetailsService { public UserDetails loadUserByUsername(String username) { return null; } } "#, - include_str!("../../rules/java/security-interface-impl.toml"), - ); - assert!( - !findings.is_empty(), - "should match fully-qualified UserDetailsService" - ); - } - - #[test] - fn java_security_interface_impl_generic_matches() { - let findings = parse_and_match_java( - r#" + true, + ), + ( + "generic AuthorizationManager", + r#" public class MyAuthManager implements AuthorizationManager { public AuthorizationDecision check() { return null; } } "#, - include_str!("../../rules/java/security-interface-impl.toml"), - ); - assert!( - !findings.is_empty(), - "should match generic AuthorizationManager" - ); - } - - #[test] - fn java_security_interface_impl_no_false_positive() { - let findings = parse_and_match_java( - r#" + true, + ), + ( + "unrelated Serializable (no false positive)", + r#" public class MyService implements Serializable { public void doWork() { } } "#, - include_str!("../../rules/java/security-interface-impl.toml"), - ); - assert!( - findings.is_empty(), - "should not match unrelated interface like Serializable" - ); + false, + ), + ]; + + for (case_name, source, expect_match) in cases { + let findings = parse_and_match_java( + source, + include_str!("../../rules/java/security-interface-impl.toml"), + ); + if *expect_match { + assert!( + !findings.is_empty(), + "case `{case_name}`: expected at least one finding", + ); + } else { + assert!( + findings.is_empty(), + "case `{case_name}`: expected no findings, got {findings:?}", + ); + } + } } #[test] From 717f480891d569484f670e3832b86d3b6f4ba641 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Tue, 28 Apr 2026 13:30:25 -0400 Subject: [PATCH 3/6] test(rules/java): pin can* and hasAny* in custom-authz-call The rule's inclusive `is|has|can` matcher and exclusive `hasAny*` not_match weren't directly fixture-tested. Add positive fixtures for `canEditRole` and `canManagePermission`, and explicit negative fixtures for `hasAnyRole` and `hasAnyAuthority` so neither side of the predicate can silently regress. Addresses CodeRabbit nitpick on #13. --- rules/java/custom-authz-call.toml | 49 +++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/rules/java/custom-authz-call.toml b/rules/java/custom-authz-call.toml index 7d567d4..bc49b32 100644 --- a/rules/java/custom-authz-call.toml +++ b/rules/java/custom-authz-call.toml @@ -82,6 +82,32 @@ public class ReportController { """ expect_match = true +# `can*` predicates (e.g. canEditRole, canManagePermission) are part of the +# inclusive `is|has|can` family — keep them locked by fixture. +[[rule.tests]] +input = """ +public class RoleController { + public void edit(Account actor, Org org, Role role) { + if (!privService.canEditRole(actor, org, role)) { + throw new ForbiddenException(); + } + } +} +""" +expect_match = true + +[[rule.tests]] +input = """ +public class PermissionController { + public void update(Account actor, Permission p) { + if (!privService.canManagePermission(actor, p)) { + throw new ForbiddenException(); + } + } +} +""" +expect_match = true + [[rule.tests]] input = """ public class Service { @@ -114,6 +140,29 @@ public class SecurityConfig { """ expect_match = false +# `hasAnyRole` and `hasAnyAuthority` are framework APIs covered by other rules; +# pin them with explicit negative fixtures so the not_match exclusion can't +# silently regress. +[[rule.tests]] +input = """ +public class SecurityConfig { + public void configure(HttpSecurity http) { + http.authorizeRequests().antMatchers("/admin/**").hasAnyRole("ADMIN", "MANAGER"); + } +} +""" +expect_match = false + +[[rule.tests]] +input = """ +public class SecurityConfig { + public void configure(HttpSecurity http) { + http.authorizeRequests().antMatchers("/api/**").hasAnyAuthority("SCOPE_read", "SCOPE_write"); + } +} +""" +expect_match = false + [[rule.tests]] input = """ public class Servlet { From f2c9ea9f16e333db879d0d30665b7f7d332fdcc1 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Tue, 28 Apr 2026 19:18:07 -0400 Subject: [PATCH 4/6] test(scanner/imports): tighten aliased-import test to exclude original name `detects_aliased_named_policy_import` asserted that the alias from a non-policy path (`Router as R`) is not captured, but didn't explicitly assert that the original name (`Router`) is also excluded. Add that assertion so both halves of the path-filtering contract on aliased imports are pinned, and split the inline comment into its two intents (policy-path alias capture vs. non-policy path filtering). --- src/scanner/imports.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/scanner/imports.rs b/src/scanner/imports.rs index 3a59d7a..9f5c76e 100644 --- a/src/scanner/imports.rs +++ b/src/scanner/imports.rs @@ -205,12 +205,16 @@ import { Router as R } from "express"; "#; let tree = parse_ts(source); let imports = find_policy_imports(&tree, source.as_bytes(), Language::TypeScript); - // We capture the binding actually used in code (the alias), not the original name. + // For policy paths: capture the binding actually used in code (the alias), + // not the original name. assert!(imports.contains("auth")); assert!(imports.contains("Perm")); assert!(!imports.contains("authorize")); assert!(!imports.contains("Permission")); + // For non-policy paths: neither the alias nor the original name is captured, + // regardless of how the import is renamed. assert!(!imports.contains("R")); + assert!(!imports.contains("Router")); } #[test] From aa9a824313c7774057182a7e050c35deb3e3630f Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Tue, 28 Apr 2026 19:18:11 -0400 Subject: [PATCH 5/6] test(rules/java): add can* keyword-in-middle fixture in custom-authz-call The rule's non-greedy inclusive regex `^(is|has|can)\w*?(Admin|...|Role|...)` is exercised for `is*` with keyword-in-middle (via `isAdminForAccount`) but not for `can*`. Add a `canAssignAdminRole` fixture so the `can`-prefixed keyword-in-middle case can't silently regress. --- rules/java/custom-authz-call.toml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/rules/java/custom-authz-call.toml b/rules/java/custom-authz-call.toml index bc49b32..6954f1d 100644 --- a/rules/java/custom-authz-call.toml +++ b/rules/java/custom-authz-call.toml @@ -108,6 +108,22 @@ public class PermissionController { """ expect_match = true +# `can*` with the keyword in the interior of the identifier (parallel to the +# existing `isAdminForAccount` fixture) — pin so the non-greedy inclusive +# regex doesn't regress for `can`-prefixed names where the keyword sits +# between two camelCase words. +[[rule.tests]] +input = """ +public class RoleController { + public void assign(Account actor, Role role, User target) { + if (!privService.canAssignAdminRole(actor, role, target)) { + throw new ForbiddenException(); + } + } +} +""" +expect_match = true + [[rule.tests]] input = """ public class Service { From 7919ba3c2c9935c6d789ed513478a662df464b74 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Tue, 28 Apr 2026 19:18:18 -0400 Subject: [PATCH 6/6] fix(rules/java): match generic and scoped AccessDecisionVoter interfaces The `access-decision-voter` rule only matched `(type_list (type_identifier))`, so generic Spring Security interfaces like `AccessDecisionVoter` (which parse as `generic_type`) and fully-qualified names like `org.springframework.security.access. PermissionEvaluator` (which parse as `scoped_type_identifier`) were missed. The existing `AccessDecisionVoter` fixture in the rule's own test block had been failing on main as a result. Add `generic_type` and `scoped_type_identifier` query branches, mirroring `security-interface-impl.toml`, and pin the fully-qualified case with a new fixture. --- rules/java/access-decision-voter.toml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/rules/java/access-decision-voter.toml b/rules/java/access-decision-voter.toml index 6654a27..8d7b3d0 100644 --- a/rules/java/access-decision-voter.toml +++ b/rules/java/access-decision-voter.toml @@ -15,6 +15,20 @@ query = """ (type_list (type_identifier) @parent_class)) ) @match + +(class_declaration + (super_interfaces + (type_list + (generic_type + (type_identifier) @parent_class))) +) @match + +(class_declaration + (super_interfaces + (type_list + (scoped_type_identifier + (type_identifier) @parent_class .))) +) @match """ [rule.predicates.parent_class] @@ -28,6 +42,9 @@ public class CustomVoter extends AbstractAccessDecisionManager { """ expect_match = true +# Generic interface: `implements AccessDecisionVoter` parses as a +# `generic_type` wrapper around the `type_identifier` — pin so the rule keeps +# matching parameterized Spring Security interfaces. [[rule.tests]] input = """ public class CustomVoter implements AccessDecisionVoter { @@ -36,6 +53,15 @@ public class CustomVoter implements AccessDecisionVoter { """ expect_match = true +# Fully-qualified interface name parses as `scoped_type_identifier`. +[[rule.tests]] +input = """ +public class CustomEvaluator implements org.springframework.security.access.PermissionEvaluator { + public boolean hasPermission(Object o, Object p) { return false; } +} +""" +expect_match = true + [[rule.tests]] input = """ public class MyService extends BaseService {