diff --git a/src/config.rs b/src/config.rs index 5e3f461..cf11c0c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -35,14 +35,14 @@ pub struct McpServer { /// Returns true when `allowedTools` is present and provides meaningful restriction. /// -/// A list that contains global wildcards like `*` / `all` / `any` is considered -/// effectively unrestricted. +/// A list that contains wildcard entries (global or namespace patterns) is +/// considered effectively unrestricted. pub fn has_effective_allowed_tools(server: &McpServer) -> bool { match &server.allowed_tools { Some(tools) if !tools.is_empty() => { - let has_specific_tools = tools.iter().any(|tool| !is_global_tool_wildcard(tool)); - let has_global_wildcard = tools.iter().any(|tool| is_global_tool_wildcard(tool)); - has_specific_tools && !has_global_wildcard + let has_specific_tools = tools.iter().any(|tool| !is_tool_wildcard(tool)); + let has_wildcard = tools.iter().any(|tool| is_tool_wildcard(tool)); + has_specific_tools && !has_wildcard } _ => false, } @@ -57,6 +57,21 @@ pub fn has_global_wildcard_allowed_tools(server: &McpServer) -> bool { .is_some_and(|tools| tools.iter().any(|tool| is_global_tool_wildcard(tool))) } +/// Returns true when `allowedTools` includes wildcard patterns (for example, +/// `github:*` or `mcp__github__*`) that are broader than explicit tool names. +pub fn has_pattern_wildcard_allowed_tools(server: &McpServer) -> bool { + server.allowed_tools.as_ref().is_some_and(|tools| { + tools.iter().any(|tool| { + let trimmed = tool.trim(); + is_pattern_tool_wildcard(trimmed) && !is_global_tool_wildcard(trimmed) + }) + }) +} + +fn is_tool_wildcard(value: &str) -> bool { + is_global_tool_wildcard(value) || is_pattern_tool_wildcard(value) +} + fn is_global_tool_wildcard(value: &str) -> bool { let lower = value.trim().to_lowercase(); matches!( @@ -65,6 +80,11 @@ fn is_global_tool_wildcard(value: &str) -> bool { ) } +fn is_pattern_tool_wildcard(value: &str) -> bool { + let trimmed = value.trim(); + !trimmed.is_empty() && (trimmed.contains('*') || trimmed.contains('?')) +} + /// A config file that has been loaded and parsed. #[derive(Debug, Clone)] pub struct ParsedConfig { @@ -280,6 +300,30 @@ mod tests { assert!(has_global_wildcard_allowed_tools(&server)); } + #[test] + fn test_pattern_wildcard_allowlist_is_not_effective() { + let server = McpServer { + allowed_tools: Some(vec!["github:*".to_string()]), + ..Default::default() + }; + + assert!(!has_effective_allowed_tools(&server)); + assert!(!has_global_wildcard_allowed_tools(&server)); + assert!(has_pattern_wildcard_allowed_tools(&server)); + } + + #[test] + fn test_mixed_allowlist_with_pattern_wildcard_is_not_effective() { + let server = McpServer { + allowed_tools: Some(vec!["read_file".to_string(), "mcp__github__*".to_string()]), + ..Default::default() + }; + + assert!(!has_effective_allowed_tools(&server)); + assert!(!has_global_wildcard_allowed_tools(&server)); + assert!(has_pattern_wildcard_allowed_tools(&server)); + } + #[test] fn test_parse_basic_config() { let json = r#"{ diff --git a/src/rules/allowlist.rs b/src/rules/allowlist.rs index e54dcb1..01a840d 100644 --- a/src/rules/allowlist.rs +++ b/src/rules/allowlist.rs @@ -1,4 +1,7 @@ -use crate::config::{has_effective_allowed_tools, has_global_wildcard_allowed_tools, McpServer}; +use crate::config::{ + has_effective_allowed_tools, has_global_wildcard_allowed_tools, + has_pattern_wildcard_allowed_tools, McpServer, +}; use crate::rules::{Finding, Rule, Severity}; /// AW-007: Flag configs with no tool filtering (allowedTools). @@ -55,7 +58,8 @@ impl Rule for AllowlistRule { let mut findings = Vec::new(); let has_allowlist = has_effective_allowed_tools(server); - let has_wildcard_allowlist = has_global_wildcard_allowed_tools(server); + let has_global_wildcard_allowlist = has_global_wildcard_allowed_tools(server); + let has_pattern_wildcard_allowlist = has_pattern_wildcard_allowed_tools(server); if !has_allowlist { let high_risk = Self::is_high_risk(server_name, server); @@ -65,23 +69,34 @@ impl Rule for AllowlistRule { Severity::Medium }; - let title = if has_wildcard_allowlist { + let title = if has_global_wildcard_allowlist { if high_risk { "Wildcard tool allowlist on high-risk server" } else { "Wildcard tool allowlist is effectively unrestricted" } + } else if has_pattern_wildcard_allowlist { + if high_risk { + "Wildcard-pattern tool allowlist on high-risk server" + } else { + "Wildcard-pattern tool allowlist is too broad" + } } else if high_risk { "No tool allowlist on high-risk server" } else { "No tool allowlist configured" }; - let message = if has_wildcard_allowlist { + let message = if has_global_wildcard_allowlist { format!( "Server '{}' uses a global wildcard in allowedTools, which effectively exposes all tools", server_name ) + } else if has_pattern_wildcard_allowlist { + format!( + "Server '{}' uses wildcard patterns in allowedTools, which can expose more tools than intended", + server_name + ) } else { format!( "Server '{}' exposes all available tools with no filtering", @@ -89,9 +104,12 @@ impl Rule for AllowlistRule { ) }; - let fix = if has_wildcard_allowlist { + let fix = if has_global_wildcard_allowlist { "Replace wildcard entries in \"allowedTools\" with explicit least-privilege tool names" .to_string() + } else if has_pattern_wildcard_allowlist { + "Replace wildcard patterns in \"allowedTools\" with explicit least-privilege tool names" + .to_string() } else { "Add \"allowedTools\" to restrict exposed tools to least privilege".to_string() }; @@ -202,4 +220,37 @@ mod tests { assert_eq!(findings.len(), 1); assert_eq!(findings[0].severity, Severity::High); } + + #[test] + fn test_pattern_wildcard_allowlist_flagged() { + let rule = AllowlistRule; + let server = McpServer { + command: Some("npx".to_string()), + allowed_tools: Some(vec!["github:*".to_string()]), + ..Default::default() + }; + + let findings = rule.check("github", &server, "test.json"); + assert_eq!(findings.len(), 1); + assert!(findings[0].title.contains("Wildcard-pattern")); + assert_eq!(findings[0].severity, Severity::Medium); + } + + #[test] + fn test_pattern_wildcard_allowlist_on_high_risk_server_is_high() { + let rule = AllowlistRule; + let server = McpServer { + command: Some("npx".to_string()), + args: Some(vec![ + "-y".to_string(), + "@modelcontextprotocol/server-fetch".to_string(), + ]), + allowed_tools: Some(vec!["mcp__fetch__*".to_string()]), + ..Default::default() + }; + + let findings = rule.check("fetch", &server, "test.json"); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].severity, Severity::High); + } } diff --git a/testdata/allowlist-pattern-wildcard.json b/testdata/allowlist-pattern-wildcard.json new file mode 100644 index 0000000..bebcddd --- /dev/null +++ b/testdata/allowlist-pattern-wildcard.json @@ -0,0 +1,9 @@ +{ + "mcpServers": { + "github": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-fetch"], + "allowedTools": ["mcp__github__*"] + } + } +} diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index d900f2e..efc18a7 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -363,6 +363,33 @@ fn test_detects_missing_allowlist() { assert!(findings.iter().any(|f| f["rule_id"] == "AW-007")); } +#[test] +fn test_detects_pattern_wildcard_allowlist() { + let output = agentwise() + .args([ + "scan", + "testdata/allowlist-pattern-wildcard.json", + "--format", + "json", + ]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let findings = parsed["findings"].as_array().unwrap(); + + assert!( + findings.iter().any(|f| { + f["rule_id"] == "AW-007" + && f["title"] + .as_str() + .is_some_and(|title| title.contains("Wildcard-pattern")) + }), + "Expected AW-007 wildcard-pattern finding, got: {}", + stdout + ); +} + #[test] fn test_detects_network_access() { let output = agentwise()