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 { diff --git a/rules/java/custom-authz-call.toml b/rules/java/custom-authz-call.toml index 7d567d4..6954f1d 100644 --- a/rules/java/custom-authz-call.toml +++ b/rules/java/custom-authz-call.toml @@ -82,6 +82,48 @@ 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 + +# `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 { @@ -114,6 +156,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 { diff --git a/src/scanner/imports.rs b/src/scanner/imports.rs index 5cdb434..9f5c76e 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,57 @@ 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); + // 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] + 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#" 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]