From efabda5e5ec39729ec269f9c660e670fbd7332cd Mon Sep 17 00:00:00 2001 From: wei9072 Date: Wed, 6 May 2026 04:01:38 +0000 Subject: [PATCH] fix(security): SEC010 reads enclosing function name + walks past inner blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 9 production validation surfaced two related bugs in enclosing_token_context that hid SEC010 from real Java code: 1. **Inner blocks terminated the upward walk too early.** `for (...) { int idx = new Random().nextInt(...) }` walks up from the call through `variable_declarator` → `local_variable_declaration` → `block` (for body). The previous code matched any `block` and called `break` after running the opaque-id heuristic, never reaching the surrounding `method_declaration`. Fix: only `break` at function-shape nodes; inner blocks (for / if / while bodies) just contribute their text to the opaque-id check and let the walk continue. 2. **Function-name needle check was missing.** Once the walk reaches the function-shape node, read its `name` field and match against the same security-needle list used on assignments. `generateSessionToken` contains `session` and `token`; `make_short_code` contains `short_code` (already covered via the assignment text in many cases, but now reliable when the call uses a generic loop variable like `idx`). 3. **Java/C# `method_declaration` and `constructor_declaration` were not in the function-shape list.** Added. Tests: 3 new — Java `generateSessionToken`-style production case (was the FN that triggered this fix); Python `make_session_token` parallel; `roll_dice` negative case to confirm we don't FP on non-security functions. 156 / 156 tests pass. Co-Authored-By: Claude Opus 4.7 --- crates/aegis-core/src/security.rs | 102 ++++++++++++++++++++++++++++-- 1 file changed, 95 insertions(+), 7 deletions(-) diff --git a/crates/aegis-core/src/security.rs b/crates/aegis-core/src/security.rs index 40e75f3..839406f 100644 --- a/crates/aegis-core/src/security.rs +++ b/crates/aegis-core/src/security.rs @@ -736,13 +736,39 @@ fn enclosing_token_context(node: Node, src: &[u8]) -> Option<&'static str> { // the random.choices() call (`chars = string.ascii_letters + ...`), // so the assignment text alone misses it. Bounded to small // scopes (<1500 bytes) to keep false-positive rate near zero. - if saw_assignment && matches!( + let is_function_shape = matches!( n.kind(), - "block" | "function_definition" | "function_body" - | "function_declaration" | "method_definition" - | "function_expression" | "arrow_function" - | "lambda_expression" | "function_item" - ) { + "function_definition" | "function_declaration" + | "method_definition" | "method_declaration" + | "function_expression" | "lambda_expression" | "function_item" + | "constructor_declaration" + ); + let is_block_shape = matches!(n.kind(), "block" | "function_body"); + + if saw_assignment && (is_function_shape || is_block_shape) { + // Function-NAME needle check. Round 9 surfaced this: + // Java's `generateSessionToken` calls `new Random().nextInt` + // through a generic loop variable `idx`; the enclosing + // assignment `int idx = ...` has no security-flavoured + // name, but the function name itself contains `session` / + // `token`. Only checked at the function-shape level (block + // alone has no name field). + if is_function_shape { + if let Some(name_node) = n.child_by_field_name("name") { + if let Ok(name_text) = name_node.utf8_text(src) { + let lower = name_text.to_ascii_lowercase(); + for needle in needles { + if lower.contains(needle) { + return Some(needle); + } + } + } + } + } + // Opaque-id heuristic across the bounded body — catches + // the URL-shortener "chars = string.ascii_letters + + // string.digits" pattern even when defined one line + // above the RNG call. if let Ok(text) = n.utf8_text(src) { if text.len() < 1500 { let lower = text.to_ascii_lowercase(); @@ -751,7 +777,16 @@ fn enclosing_token_context(node: Node, src: &[u8]) -> Option<&'static str> { } } } - break; + // Inner blocks (for / if / while bodies) don't terminate + // the upward walk — keep climbing until we either find + // a function-shape node or hit the loop limit. Breaking + // at every block was the bug Round 9 surfaced: it + // stopped at the for-body block before reaching the + // method declaration carrying the security-flavoured + // function name. + if is_function_shape { + break; + } } cur = n.parent(); } @@ -1740,6 +1775,59 @@ mod tests { assert!(v.iter().any(|v| v.rule_id == "SEC010"), "got {v:?}"); } + #[test] + fn sec010_java_new_random_in_session_token_function_blocks() { + // Round 9 production case: `int idx = new Random().nextInt(...)`. + // The local variable `idx` doesn't match a needle, but the + // enclosing function `generateSessionToken` does (contains + // "session" / "token"). Function-name check gates this in. + let v = check( + ".java", + "import java.util.Random;\n\ + public class Auth {\n \ + public static String generateSessionToken() {\n \ + String chars = \"abcdef0123456789\";\n \ + StringBuilder token = new StringBuilder();\n \ + for (int i = 0; i < 16; i++) {\n \ + int idx = new Random().nextInt(chars.length());\n \ + token.append(chars.charAt(idx));\n \ + }\n \ + return token.toString();\n \ + }\n\ + }\n", + ); + assert!(v.iter().any(|v| v.rule_id == "SEC010"), "got {v:?}"); + } + + #[test] + fn sec010_python_random_in_token_function_blocks() { + // Same pattern, Python: function name `make_session_token` + // contains needle `token` even though the inner call site + // uses an unnamed comprehension. + let v = check( + ".py", + "import random\n\ + def make_session_token():\n \ + chars = 'abcdef0123456789'\n \ + idx = random.randint(0, len(chars) - 1)\n \ + return chars[idx]\n", + ); + assert!(v.iter().any(|v| v.rule_id == "SEC010"), "got {v:?}"); + } + + #[test] + fn sec010_random_in_dice_function_does_not_block() { + // Function name has no security needle; opaque-id heuristic + // doesn't fire either. Must stay silent. + let v = check( + ".py", + "import random\n\ + def roll_dice():\n \ + return random.randint(1, 6)\n", + ); + assert!(!v.iter().any(|v| v.rule_id == "SEC010"), "got {v:?}"); + } + #[test] fn sec010_java_new_secure_random_does_not_block() { let v = check(