From f7310b20bad240c8b663877ee346e86481e2e76d Mon Sep 17 00:00:00 2001 From: elipeter Date: Sun, 24 May 2026 13:36:03 -0500 Subject: [PATCH] refactor(dynamic): improve SSA receiver type validation, refactor framework binding logic, and expand test coverage --- .../framework/adapters/middleware_rails.rs | 124 ++++++++++++--- .../framework/adapters/migration_laravel.rs | 117 +++++++++++++-- .../framework/adapters/migration_rails.rs | 121 ++++++++++++--- .../framework/adapters/php_codeigniter.rs | 120 ++++++++++++--- src/dynamic/framework/adapters/php_routes.rs | 5 +- src/dynamic/framework/adapters/php_symfony.rs | 12 ++ src/dynamic/framework/adapters/ruby_hanami.rs | 15 ++ src/dynamic/framework/adapters/ruby_rails.rs | 141 ++++++++++++++++++ .../framework/adapters/ruby_sinatra.rs | 20 ++- 9 files changed, 593 insertions(+), 82 deletions(-) diff --git a/src/dynamic/framework/adapters/middleware_rails.rs b/src/dynamic/framework/adapters/middleware_rails.rs index 68e467b2..c11d0c9a 100644 --- a/src/dynamic/framework/adapters/middleware_rails.rs +++ b/src/dynamic/framework/adapters/middleware_rails.rs @@ -14,6 +14,7 @@ use crate::dynamic::framework::{FrameworkAdapter, FrameworkBinding}; use crate::evidence::EntryKind; use crate::summary::FuncSummary; +use crate::summary::ssa_summary::SsaFuncSummary; use crate::symbol::Lang; pub struct MiddlewareRailsAdapter; @@ -67,35 +68,69 @@ impl FrameworkAdapter for MiddlewareRailsAdapter { fn detect( &self, summary: &FuncSummary, - _ast: tree_sitter::Node<'_>, + ast: tree_sitter::Node<'_>, file_bytes: &[u8], ) -> Option { - if looks_like_rails_controller(file_bytes) { - return None; - } - let has_middleware_shape = source_has_rack_middleware_shape(file_bytes); - let name_matches = name_is_rack_entry(&summary.name); - let body_mounts_middleware = super::any_callee_matches(summary, callee_is_rails_middleware); - let binds = (name_matches && has_middleware_shape) || body_mounts_middleware; - if !binds { - return None; - } - Some(FrameworkBinding { - adapter: ADAPTER_NAME.to_owned(), - kind: EntryKind::Middleware { - name: summary.name.clone(), - }, - route: None, - request_params: Vec::new(), - response_writer: None, - middleware: Vec::new(), - }) + detect_rails_middleware(summary, None, ast, file_bytes) } + + fn detect_with_context( + &self, + summary: &FuncSummary, + ssa_summary: Option<&SsaFuncSummary>, + ast: tree_sitter::Node<'_>, + file_bytes: &[u8], + ) -> Option { + detect_rails_middleware(summary, ssa_summary, ast, file_bytes) + } +} + +fn detect_rails_middleware( + summary: &FuncSummary, + ssa_summary: Option<&SsaFuncSummary>, + _ast: tree_sitter::Node<'_>, + file_bytes: &[u8], +) -> Option { + if looks_like_rails_controller(file_bytes) { + return None; + } + let has_middleware_shape = source_has_rack_middleware_shape(file_bytes); + let name_matches = name_is_rack_entry(&summary.name); + let receiver_facts_allow = super::typed_receiver_facts_allow( + summary, + ssa_summary, + callee_is_rails_middleware, + typed_container_allows_rack_middleware, + ); + if !receiver_facts_allow { + return None; + } + let body_mounts_middleware = super::any_callee_matches(summary, callee_is_rails_middleware); + let binds = (name_matches && has_middleware_shape) || body_mounts_middleware; + if !binds { + return None; + } + Some(FrameworkBinding { + adapter: ADAPTER_NAME.to_owned(), + kind: EntryKind::Middleware { + name: summary.name.clone(), + }, + route: None, + request_params: Vec::new(), + response_writer: None, + middleware: Vec::new(), + }) +} + +fn typed_container_allows_rack_middleware(container: &str) -> bool { + let lc = container.to_ascii_lowercase(); + lc.contains("rack") || lc.contains("rails") || lc.ends_with("middleware") || lc == "app" } #[cfg(test)] mod tests { use super::*; + use crate::summary::CalleeSite; fn parse_ruby(src: &[u8]) -> tree_sitter::Tree { let mut parser = tree_sitter::Parser::new(); @@ -134,4 +169,51 @@ mod tests { "controller action must not bind as Rack middleware", ); } + + #[test] + fn ssa_receiver_type_rejects_proc_call_collision() { + let src: &[u8] = b"def call(env)\n proc = env['callback']\n proc.call('x')\nend\n"; + let tree = parse_ruby(src); + let mut summary = FuncSummary { + name: "call".into(), + ..Default::default() + }; + summary.callees.push(CalleeSite { + name: "proc.call".into(), + receiver: Some("proc".into()), + ordinal: 0, + ..Default::default() + }); + let mut ssa = SsaFuncSummary::default(); + ssa.typed_call_receivers.push((0, "Proc".to_owned())); + assert!( + MiddlewareRailsAdapter + .detect_with_context(&summary, Some(&ssa), tree.root_node(), src) + .is_none(), + "Proc#call must not bind as Rack middleware", + ); + } + + #[test] + fn ssa_receiver_type_allows_rack_middleware_call() { + let src: &[u8] = b"def mount(app)\n app.call({})\nend\n"; + let tree = parse_ruby(src); + let mut summary = FuncSummary { + name: "mount".into(), + ..Default::default() + }; + summary.callees.push(CalleeSite { + name: "app.call".into(), + receiver: Some("app".into()), + ordinal: 0, + ..Default::default() + }); + let mut ssa = SsaFuncSummary::default(); + ssa.typed_call_receivers + .push((0, "Rack::Builder".to_owned())); + let binding = MiddlewareRailsAdapter + .detect_with_context(&summary, Some(&ssa), tree.root_node(), src) + .expect("Rack receiver should bind"); + assert_eq!(binding.adapter, "middleware-rails"); + } } diff --git a/src/dynamic/framework/adapters/migration_laravel.rs b/src/dynamic/framework/adapters/migration_laravel.rs index 236d88bc..d0ed6afa 100644 --- a/src/dynamic/framework/adapters/migration_laravel.rs +++ b/src/dynamic/framework/adapters/migration_laravel.rs @@ -13,6 +13,7 @@ use crate::dynamic::framework::{FrameworkAdapter, FrameworkBinding}; use crate::evidence::EntryKind; use crate::summary::FuncSummary; +use crate::summary::ssa_summary::SsaFuncSummary; use crate::symbol::Lang; pub struct MigrationLaravelAdapter; @@ -54,30 +55,64 @@ impl FrameworkAdapter for MigrationLaravelAdapter { fn detect( &self, summary: &FuncSummary, - _ast: tree_sitter::Node<'_>, + ast: tree_sitter::Node<'_>, file_bytes: &[u8], ) -> Option { - let has_shape = source_has_migration_shape(file_bytes); - let name_matches = name_is_migration_entry(&summary.name); - let body_runs_ddl = super::any_callee_matches(summary, callee_is_laravel_migration_ddl); - let binds = (name_matches || body_runs_ddl) && has_shape; - if !binds { - return None; - } - Some(FrameworkBinding { - adapter: ADAPTER_NAME.to_owned(), - kind: EntryKind::Migration { version: None }, - route: None, - request_params: Vec::new(), - response_writer: None, - middleware: Vec::new(), - }) + detect_laravel_migration(summary, None, ast, file_bytes) } + + fn detect_with_context( + &self, + summary: &FuncSummary, + ssa_summary: Option<&SsaFuncSummary>, + ast: tree_sitter::Node<'_>, + file_bytes: &[u8], + ) -> Option { + detect_laravel_migration(summary, ssa_summary, ast, file_bytes) + } +} + +fn detect_laravel_migration( + summary: &FuncSummary, + ssa_summary: Option<&SsaFuncSummary>, + _ast: tree_sitter::Node<'_>, + file_bytes: &[u8], +) -> Option { + let has_shape = source_has_migration_shape(file_bytes); + let name_matches = name_is_migration_entry(&summary.name); + let receiver_facts_allow = super::typed_receiver_facts_allow( + summary, + ssa_summary, + callee_is_laravel_migration_ddl, + typed_container_allows_laravel_migration, + ); + if !receiver_facts_allow { + return None; + } + let body_runs_ddl = super::any_callee_matches(summary, callee_is_laravel_migration_ddl); + let binds = (name_matches || body_runs_ddl) && has_shape; + if !binds { + return None; + } + Some(FrameworkBinding { + adapter: ADAPTER_NAME.to_owned(), + kind: EntryKind::Migration { version: None }, + route: None, + request_params: Vec::new(), + response_writer: None, + middleware: Vec::new(), + }) +} + +fn typed_container_allows_laravel_migration(container: &str) -> bool { + let lc = container.to_ascii_lowercase(); + lc.contains("schema") || lc.contains("db") || lc.contains("migration") } #[cfg(test)] mod tests { use super::*; + use crate::summary::CalleeSite; fn parse_php(src: &[u8]) -> tree_sitter::Tree { let mut parser = tree_sitter::Parser::new(); @@ -100,4 +135,54 @@ mod tests { assert_eq!(binding.adapter, "migration-laravel"); assert!(matches!(binding.kind, EntryKind::Migration { .. })); } + + #[test] + fn ssa_receiver_type_rejects_non_schema_table_collision() { + let src: &[u8] = + b"table('users'); }\n"; + let tree = parse_php(src); + let mut summary = FuncSummary { + name: "up".into(), + ..Default::default() + }; + summary.callees.push(CalleeSite { + name: "builder.table".into(), + receiver: Some("builder".into()), + ordinal: 0, + ..Default::default() + }); + let mut ssa = SsaFuncSummary::default(); + ssa.typed_call_receivers + .push((0, "HtmlTableBuilder".to_owned())); + assert!( + MigrationLaravelAdapter + .detect_with_context(&summary, Some(&ssa), tree.root_node(), src) + .is_none(), + "non-Schema table builders must not bind as Laravel migration DDL", + ); + } + + #[test] + fn ssa_receiver_type_allows_schema_builder() { + let src: &[u8] = + b"table('users'); }\n"; + let tree = parse_php(src); + let mut summary = FuncSummary { + name: "helper".into(), + ..Default::default() + }; + summary.callees.push(CalleeSite { + name: "schema.table".into(), + receiver: Some("schema".into()), + ordinal: 0, + ..Default::default() + }); + let mut ssa = SsaFuncSummary::default(); + ssa.typed_call_receivers + .push((0, "Illuminate\\Database\\Schema\\Builder".to_owned())); + let binding = MigrationLaravelAdapter + .detect_with_context(&summary, Some(&ssa), tree.root_node(), src) + .expect("Schema builder receiver should bind"); + assert_eq!(binding.adapter, "migration-laravel"); + } } diff --git a/src/dynamic/framework/adapters/migration_rails.rs b/src/dynamic/framework/adapters/migration_rails.rs index 83628d04..e79a033c 100644 --- a/src/dynamic/framework/adapters/migration_rails.rs +++ b/src/dynamic/framework/adapters/migration_rails.rs @@ -12,6 +12,7 @@ use crate::dynamic::framework::{FrameworkAdapter, FrameworkBinding}; use crate::evidence::EntryKind; use crate::summary::FuncSummary; +use crate::summary::ssa_summary::SsaFuncSummary; use crate::symbol::Lang; pub struct MigrationRailsAdapter; @@ -66,32 +67,66 @@ impl FrameworkAdapter for MigrationRailsAdapter { fn detect( &self, summary: &FuncSummary, - _ast: tree_sitter::Node<'_>, + ast: tree_sitter::Node<'_>, file_bytes: &[u8], ) -> Option { - let has_shape = source_has_migration_shape(file_bytes); - let name_matches = name_is_migration_entry(&summary.name); - let body_runs_ddl = super::any_callee_matches(summary, callee_is_rails_migration); - let binds = (name_matches || body_runs_ddl) && has_shape; - if !binds { - return None; - } - Some(FrameworkBinding { - adapter: ADAPTER_NAME.to_owned(), - kind: EntryKind::Migration { - version: extract_version(file_bytes), - }, - route: None, - request_params: Vec::new(), - response_writer: None, - middleware: Vec::new(), - }) + detect_rails_migration(summary, None, ast, file_bytes) } + + fn detect_with_context( + &self, + summary: &FuncSummary, + ssa_summary: Option<&SsaFuncSummary>, + ast: tree_sitter::Node<'_>, + file_bytes: &[u8], + ) -> Option { + detect_rails_migration(summary, ssa_summary, ast, file_bytes) + } +} + +fn detect_rails_migration( + summary: &FuncSummary, + ssa_summary: Option<&SsaFuncSummary>, + _ast: tree_sitter::Node<'_>, + file_bytes: &[u8], +) -> Option { + let has_shape = source_has_migration_shape(file_bytes); + let name_matches = name_is_migration_entry(&summary.name); + let receiver_facts_allow = super::typed_receiver_facts_allow( + summary, + ssa_summary, + callee_is_rails_migration, + typed_container_allows_rails_migration, + ); + if !receiver_facts_allow { + return None; + } + let body_runs_ddl = super::any_callee_matches(summary, callee_is_rails_migration); + let binds = (name_matches || body_runs_ddl) && has_shape; + if !binds { + return None; + } + Some(FrameworkBinding { + adapter: ADAPTER_NAME.to_owned(), + kind: EntryKind::Migration { + version: extract_version(file_bytes), + }, + route: None, + request_params: Vec::new(), + response_writer: None, + middleware: Vec::new(), + }) +} + +fn typed_container_allows_rails_migration(container: &str) -> bool { + let lc = container.to_ascii_lowercase(); + lc.contains("activerecord") || lc.contains("migration") || lc.contains("connection") } #[cfg(test)] mod tests { use super::*; + use crate::summary::CalleeSite; fn parse_ruby(src: &[u8]) -> tree_sitter::Tree { let mut parser = tree_sitter::Parser::new(); @@ -132,4 +167,54 @@ mod tests { "db/schema.rb dump must not bind as migration", ); } + + #[test] + fn ssa_receiver_type_rejects_non_migration_execute_collision() { + let src: &[u8] = b"# class AddIndex < ActiveRecord::Migration[7.0]\ndef helper(builder)\n builder.execute('x')\nend\n"; + let tree = parse_ruby(src); + let mut summary = FuncSummary { + name: "up".into(), + ..Default::default() + }; + summary.callees.push(CalleeSite { + name: "builder.execute".into(), + receiver: Some("builder".into()), + ordinal: 0, + ..Default::default() + }); + let mut ssa = SsaFuncSummary::default(); + ssa.typed_call_receivers + .push((0, "SqlStringBuilder".to_owned())); + assert!( + MigrationRailsAdapter + .detect_with_context(&summary, Some(&ssa), tree.root_node(), src) + .is_none(), + "builder.execute should not bind as an ActiveRecord migration DDL call", + ); + } + + #[test] + fn ssa_receiver_type_allows_active_record_connection() { + let src: &[u8] = b"# class AddIndex < ActiveRecord::Migration[7.0]\ndef helper(conn)\n conn.execute('x')\nend\n"; + let tree = parse_ruby(src); + let mut summary = FuncSummary { + name: "helper".into(), + ..Default::default() + }; + summary.callees.push(CalleeSite { + name: "conn.execute".into(), + receiver: Some("conn".into()), + ordinal: 0, + ..Default::default() + }); + let mut ssa = SsaFuncSummary::default(); + ssa.typed_call_receivers.push(( + 0, + "ActiveRecord::ConnectionAdapters::DatabaseStatements".to_owned(), + )); + let binding = MigrationRailsAdapter + .detect_with_context(&summary, Some(&ssa), tree.root_node(), src) + .expect("ActiveRecord receiver should bind"); + assert_eq!(binding.adapter, "migration-rails"); + } } diff --git a/src/dynamic/framework/adapters/php_codeigniter.rs b/src/dynamic/framework/adapters/php_codeigniter.rs index 952548cb..a5451ab0 100644 --- a/src/dynamic/framework/adapters/php_codeigniter.rs +++ b/src/dynamic/framework/adapters/php_codeigniter.rs @@ -16,6 +16,7 @@ use crate::dynamic::framework::HttpMethod; use crate::dynamic::framework::{FrameworkAdapter, FrameworkBinding, RouteShape}; use crate::evidence::EntryKind; use crate::summary::FuncSummary; +use crate::summary::ssa_summary::SsaFuncSummary; use crate::symbol::Lang; use tree_sitter::Node; @@ -43,33 +44,71 @@ impl FrameworkAdapter for PhpCodeIgniterAdapter { ast: Node<'_>, file_bytes: &[u8], ) -> Option { - if !source_imports_codeigniter(file_bytes) { - return None; - } - let (func_node, class) = find_php_function(ast, file_bytes, &summary.name)?; - let controller = class.and_then(|c| php_class_name(c, file_bytes)); - - let (method, path) = find_codeigniter_route(ast, file_bytes, &summary.name, controller)?; - - let formals = php_formal_names(func_node, file_bytes); - let request_params = bind_php_path_params(&formals, &path); - let middleware = collect_php_middleware(ast, file_bytes); - - Some(FrameworkBinding { - adapter: ADAPTER_NAME.to_owned(), - kind: EntryKind::HttpRoute, - route: Some(RouteShape::single(method, path)), - request_params, - response_writer: None, - middleware, - }) + detect_codeigniter(summary, None, ast, file_bytes) } + + fn detect_with_context( + &self, + summary: &FuncSummary, + ssa_summary: Option<&SsaFuncSummary>, + ast: Node<'_>, + file_bytes: &[u8], + ) -> Option { + detect_codeigniter(summary, ssa_summary, ast, file_bytes) + } +} + +fn detect_codeigniter( + summary: &FuncSummary, + ssa_summary: Option<&SsaFuncSummary>, + ast: Node<'_>, + file_bytes: &[u8], +) -> Option { + if !source_imports_codeigniter(file_bytes) { + return None; + } + if !super::typed_receiver_facts_allow( + summary, + ssa_summary, + callee_is_codeigniter_route_registration, + typed_container_allows_codeigniter_routes, + ) { + return None; + } + let (func_node, class) = find_php_function(ast, file_bytes, &summary.name)?; + let controller = class.and_then(|c| php_class_name(c, file_bytes)); + + let (method, path) = find_codeigniter_route(ast, file_bytes, &summary.name, controller)?; + + let formals = php_formal_names(func_node, file_bytes); + let request_params = bind_php_path_params(&formals, &path); + let middleware = collect_php_middleware(ast, file_bytes); + + Some(FrameworkBinding { + adapter: ADAPTER_NAME.to_owned(), + kind: EntryKind::HttpRoute, + route: Some(RouteShape::single(method, path)), + request_params, + response_writer: None, + middleware, + }) +} + +fn callee_is_codeigniter_route_registration(name: &str) -> bool { + let last = name.rsplit_once('.').map(|(_, s)| s).unwrap_or(name); + matches!(last, "get" | "post" | "put" | "patch" | "delete" | "add") +} + +fn typed_container_allows_codeigniter_routes(container: &str) -> bool { + let lc = container.to_ascii_lowercase(); + lc.contains("codeigniter") || lc.contains("routecollection") } #[cfg(test)] mod tests { use super::*; use crate::dynamic::framework::ParamSource; + use crate::summary::CalleeSite; fn parse(src: &[u8]) -> tree_sitter::Tree { let mut parser = tree_sitter::Parser::new(); @@ -137,4 +176,45 @@ mod tests { .is_none() ); } + + #[test] + fn ssa_receiver_type_rejects_non_codeigniter_routes_property() { + let src: &[u8] = b"get('users/(:num)', 'UserController::show');\nclass UserController extends BaseController {\n public function show($num) { return $num; }\n}\n"; + let tree = parse(src); + let mut func = summary("show"); + func.callees.push(CalleeSite { + name: "routes.get".into(), + receiver: Some("routes".into()), + ordinal: 0, + ..Default::default() + }); + let mut ssa = SsaFuncSummary::default(); + ssa.typed_call_receivers.push((0, "App\\Cache".to_owned())); + assert!( + PhpCodeIgniterAdapter + .detect_with_context(&func, Some(&ssa), tree.root_node(), src) + .is_none(), + "a typed non-CodeIgniter `$routes` receiver must suppress the route binding", + ); + } + + #[test] + fn ssa_receiver_type_allows_codeigniter_route_collection() { + let src: &[u8] = b"get('users/(:num)', 'UserController::show');\nclass UserController extends BaseController {\n public function show($num) { return $num; }\n}\n"; + let tree = parse(src); + let mut func = summary("show"); + func.callees.push(CalleeSite { + name: "routes.get".into(), + receiver: Some("routes".into()), + ordinal: 0, + ..Default::default() + }); + let mut ssa = SsaFuncSummary::default(); + ssa.typed_call_receivers + .push((0, "CodeIgniter\\Router\\RouteCollection".to_owned())); + let binding = PhpCodeIgniterAdapter + .detect_with_context(&func, Some(&ssa), tree.root_node(), src) + .expect("CodeIgniter route receiver should bind"); + assert_eq!(binding.adapter, "php-codeigniter"); + } } diff --git a/src/dynamic/framework/adapters/php_routes.rs b/src/dynamic/framework/adapters/php_routes.rs index c69ad6e3..a18654f0 100644 --- a/src/dynamic/framework/adapters/php_routes.rs +++ b/src/dynamic/framework/adapters/php_routes.rs @@ -43,8 +43,8 @@ pub fn source_imports_laravel(bytes: &[u8]) -> bool { } /// True when `bytes` carries any of the well-known Symfony import -/// stanzas (the `Symfony\…` namespace, the `#[Route]` attribute, the -/// `AbstractController` base class). +/// stanzas (the `Symfony\…` namespace, the routing attribute import, +/// or an explicit fixture marker). pub fn source_imports_symfony(bytes: &[u8]) -> bool { contains_any( bytes, @@ -55,7 +55,6 @@ pub fn source_imports_symfony(bytes: &[u8]) -> bool { b"use Symfony\\", b"Symfony\\Component\\Routing\\Annotation\\Route", b"Symfony\\Component\\Routing\\Attribute\\Route", - b"AbstractController", b"// nyx-shape: symfony", ], ) diff --git a/src/dynamic/framework/adapters/php_symfony.rs b/src/dynamic/framework/adapters/php_symfony.rs index 6b05dc04..ebbbbf1f 100644 --- a/src/dynamic/framework/adapters/php_symfony.rs +++ b/src/dynamic/framework/adapters/php_symfony.rs @@ -184,6 +184,18 @@ mod tests { ); } + #[test] + fn skips_custom_abstract_controller_without_symfony_import() { + let src: &[u8] = b", file_bytes: &[u8], ) -> Option { + if summary.name != "call" { + return None; + } if !source_imports_hanami(file_bytes) { return None; } @@ -388,6 +391,18 @@ mod tests { ); } + #[test] + fn skips_non_call_helpers_even_when_class_mixes_in_hanami_action() { + let src: &[u8] = b"require 'hanami/action'\nclass Helper\n include Hanami::Action\n def sanitize(req)\n req\n end\nend\n"; + let tree = parse(src); + assert!( + RubyHanamiAdapter + .detect(&summary("sanitize"), tree.root_node(), src) + .is_none(), + "Hanami actions dispatch through `call`; helper methods are not route entries", + ); + } + #[test] fn skips_files_without_hanami_marker() { let src: &[u8] = b"class Show < Hanami::Action\n def call(req)\n 'ok'\n end\nend\n"; diff --git a/src/dynamic/framework/adapters/ruby_rails.rs b/src/dynamic/framework/adapters/ruby_rails.rs index 98871b71..3f6786bf 100644 --- a/src/dynamic/framework/adapters/ruby_rails.rs +++ b/src/dynamic/framework/adapters/ruby_rails.rs @@ -255,6 +255,120 @@ fn snake_case(input: &str) -> String { out } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum RubyVisibility { + Public, + Private, + Protected, +} + +fn method_is_public_action(class: Node<'_>, method: Node<'_>, bytes: &[u8]) -> bool { + let Some(target) = super::ruby_routes::method_identifier(method, bytes) else { + return true; + }; + let mut class_cursor = class.walk(); + let Some(body) = class + .named_children(&mut class_cursor) + .find(|c| c.kind() == "body_statement") + else { + return true; + }; + + if let Some(visibility) = explicit_visibility_for_method(body, bytes, target) { + return visibility == RubyVisibility::Public; + } + + visibility_at_method(body, method, bytes) == RubyVisibility::Public +} + +fn explicit_visibility_for_method( + body: Node<'_>, + bytes: &[u8], + target: &str, +) -> Option { + let mut out = None; + let mut cur = body.walk(); + for member in body.named_children(&mut cur) { + let Some((visibility, args)) = visibility_call(member, bytes) else { + continue; + }; + let Some(args) = args else { + continue; + }; + if argument_list_mentions(args, bytes, target) { + out = Some(visibility); + } + } + out +} + +fn visibility_at_method(body: Node<'_>, method: Node<'_>, bytes: &[u8]) -> RubyVisibility { + let mut visibility = RubyVisibility::Public; + let mut cur = body.walk(); + for member in body.named_children(&mut cur) { + if member.byte_range() == method.byte_range() { + return visibility; + } + let Some((next, args)) = visibility_call(member, bytes) else { + continue; + }; + if args.is_none() { + visibility = next; + } + } + RubyVisibility::Public +} + +fn visibility_call<'a>( + node: Node<'a>, + bytes: &'a [u8], +) -> Option<(RubyVisibility, Option>)> { + if node.kind() == "identifier" { + let visibility = match node.utf8_text(bytes).ok()? { + "public" => RubyVisibility::Public, + "private" => RubyVisibility::Private, + "protected" => RubyVisibility::Protected, + _ => return None, + }; + return Some((visibility, None)); + } + if node.kind() != "call" { + return None; + } + let mut cur = node.walk(); + let mut ident = None; + let mut args = None; + for child in node.named_children(&mut cur) { + match child.kind() { + "identifier" if ident.is_none() => ident = child.utf8_text(bytes).ok(), + "argument_list" => args = Some(child), + _ => {} + } + } + let visibility = match ident? { + "public" => RubyVisibility::Public, + "private" => RubyVisibility::Private, + "protected" => RubyVisibility::Protected, + _ => return None, + }; + Some((visibility, args)) +} + +fn argument_list_mentions(args: Node<'_>, bytes: &[u8], target: &str) -> bool { + let mut cur = args.walk(); + for arg in args.named_children(&mut cur) { + let raw = arg.utf8_text(bytes).unwrap_or("").trim(); + let normalized = raw + .trim_start_matches(':') + .trim_matches('"') + .trim_matches('\''); + if normalized == target { + return true; + } + } + false +} + impl FrameworkAdapter for RubyRailsAdapter { fn name(&self) -> &'static str { ADAPTER_NAME @@ -277,6 +391,9 @@ impl FrameworkAdapter for RubyRailsAdapter { if !class_is_rails_controller(class, file_bytes) { return None; } + if !method_is_public_action(class, method, file_bytes) { + return None; + } let controller = class_name(class, file_bytes)?; let (http_method, path) = find_route_mapping(ast, file_bytes, controller, &summary.name) @@ -452,6 +569,30 @@ mod tests { ); } + #[test] + fn skips_private_helper_named_explicitly() { + let src: &[u8] = b"class UsersController < ApplicationController\n def index\n 'ok'\n end\n def sanitize_inputs(value)\n value\n end\n private :sanitize_inputs\nend\n"; + let tree = parse(src); + assert!( + RubyRailsAdapter + .detect(&summary("sanitize_inputs"), tree.root_node(), src) + .is_none(), + "private controller helpers are not routable Rails actions", + ); + } + + #[test] + fn skips_methods_below_private_visibility() { + let src: &[u8] = b"class UsersController < ApplicationController\n private\n def sanitize_inputs(value)\n value\n end\nend\n"; + let tree = parse(src); + assert!( + RubyRailsAdapter + .detect(&summary("sanitize_inputs"), tree.root_node(), src) + .is_none(), + "methods declared under `private` are not routable Rails actions", + ); + } + #[test] fn skips_files_without_rails_marker() { let src: &[u8] = b"class UsersController < Object\n def index\n 'ok'\n end\nend\n"; diff --git a/src/dynamic/framework/adapters/ruby_sinatra.rs b/src/dynamic/framework/adapters/ruby_sinatra.rs index 20c428ec..48b70a6b 100644 --- a/src/dynamic/framework/adapters/ruby_sinatra.rs +++ b/src/dynamic/framework/adapters/ruby_sinatra.rs @@ -147,7 +147,7 @@ impl FrameworkAdapter for RubySinatraAdapter { let route = routes .iter() .find(|r| path_stem(&r.path) == target) - .or_else(|| routes.first())?; + .or_else(|| (routes.len() == 1).then(|| &routes[0]))?; let request_params = bind_path_params(&route.block_params, &route.path); let middleware = collect_ruby_middleware(ast, file_bytes); Some(FrameworkBinding { @@ -234,9 +234,8 @@ mod tests { } #[test] - fn falls_back_to_first_route_when_name_does_not_match_stem() { - let src: &[u8] = - b"require 'sinatra'\nget '/alpha' do |p|\n p\nend\nget '/beta' do |p|\n p\nend\n"; + fn falls_back_to_first_route_when_single_route_name_does_not_match_stem() { + let src: &[u8] = b"require 'sinatra'\nget '/alpha' do |p|\n p\nend\n"; let tree = parse(src); let binding = RubySinatraAdapter .detect(&summary("gamma"), tree.root_node(), src) @@ -244,6 +243,19 @@ mod tests { assert_eq!(binding.route.unwrap().path, "/alpha"); } + #[test] + fn skips_multi_route_files_when_name_does_not_match_any_stem() { + let src: &[u8] = + b"require 'sinatra'\nget '/alpha' do |p|\n p\nend\nget '/beta' do |p|\n p\nend\n"; + let tree = parse(src); + assert!( + RubySinatraAdapter + .detect(&summary("gamma"), tree.root_node(), src) + .is_none(), + "multi-route Sinatra files must not bind an unrelated summary to the first route", + ); + } + #[test] fn skips_when_sinatra_not_imported() { let src: &[u8] = b"get '/run' do |p|\n p\nend\n";