diff --git a/src/config.rs b/src/config.rs index f89c769..5e3f461 100644 --- a/src/config.rs +++ b/src/config.rs @@ -33,6 +33,38 @@ pub struct McpServer { pub disabled: Option, } +/// Returns true when `allowedTools` is present and provides meaningful restriction. +/// +/// A list that contains global wildcards like `*` / `all` / `any` 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 + } + _ => false, + } +} + +/// Returns true when `allowedTools` includes a global wildcard that effectively +/// permits all tools. +pub fn has_global_wildcard_allowed_tools(server: &McpServer) -> bool { + server + .allowed_tools + .as_ref() + .is_some_and(|tools| tools.iter().any(|tool| is_global_tool_wildcard(tool))) +} + +fn is_global_tool_wildcard(value: &str) -> bool { + let lower = value.trim().to_lowercase(); + matches!( + lower.as_str(), + "*" | "all" | "any" | ".*" | "all-tools" | "all_tools" | "tool:*" | "tools:*" + ) +} + /// A config file that has been loaded and parsed. #[derive(Debug, Clone)] pub struct ParsedConfig { @@ -215,6 +247,39 @@ fn parse_package_version(s: &str) -> Option<(String, String)> { mod tests { use super::*; + #[test] + fn test_effective_allowed_tools_requires_specific_entries() { + let server = McpServer { + allowed_tools: Some(vec!["read_file".to_string(), "list_files".to_string()]), + ..Default::default() + }; + + assert!(has_effective_allowed_tools(&server)); + assert!(!has_global_wildcard_allowed_tools(&server)); + } + + #[test] + fn test_global_wildcard_allowed_tools_is_not_effective() { + let server = McpServer { + allowed_tools: Some(vec!["*".to_string()]), + ..Default::default() + }; + + assert!(!has_effective_allowed_tools(&server)); + assert!(has_global_wildcard_allowed_tools(&server)); + } + + #[test] + fn test_mixed_allowlist_with_global_wildcard_is_not_effective() { + let server = McpServer { + allowed_tools: Some(vec!["read_file".to_string(), "all".to_string()]), + ..Default::default() + }; + + assert!(!has_effective_allowed_tools(&server)); + assert!(has_global_wildcard_allowed_tools(&server)); + } + #[test] fn test_parse_basic_config() { let json = r#"{ diff --git a/src/inspect.rs b/src/inspect.rs index 7250bfd..059fe7f 100644 --- a/src/inspect.rs +++ b/src/inspect.rs @@ -1,4 +1,7 @@ -use crate::config::{extract_all_package_names, extract_package_info, McpServer}; +use crate::config::{ + extract_all_package_names, extract_package_info, has_effective_allowed_tools, + has_global_wildcard_allowed_tools, McpServer, +}; use crate::scanner; use serde::Serialize; use std::fmt::Write; @@ -100,7 +103,8 @@ fn inspect_server(server_name: &str, config_file: &str, server: &McpServer) -> I let remote = is_remote(server); let transport = if remote { "remote" } else { "stdio" }.to_string(); let auth_present = has_auth(server); - let allowlist_present = server.allowed_tools.as_ref().is_some_and(|t| !t.is_empty()); + let allowlist_present = has_effective_allowed_tools(server); + let wildcard_allowlist = has_global_wildcard_allowed_tools(server); let network_tool = is_network_tool(server_name, server); let network_restricted = if network_tool { @@ -134,6 +138,9 @@ fn inspect_server(server_name: &str, config_file: &str, server: &McpServer) -> I if !allowlist_present { risk_tags.push("no_allowlist".to_string()); } + if wildcard_allowlist { + risk_tags.push("wildcard_allowlist".to_string()); + } if !network_restricted { risk_tags.push("unrestricted_network".to_string()); } @@ -434,4 +441,25 @@ mod tests { assert!(out.contains("agentwise inspect")); assert!(out.contains("Scanned:")); } + + #[test] + fn test_wildcard_allowlist_marked_as_risk() { + let server = McpServer { + command: Some("npx".to_string()), + args: Some(vec![ + "-y".to_string(), + "@modelcontextprotocol/server-fetch".to_string(), + ]), + allowed_tools: Some(vec!["*".to_string()]), + ..Default::default() + }; + + let inspected = inspect_server("fetch", "test.json", &server); + assert!(!inspected.allowlist_present); + assert!(inspected + .risk_tags + .iter() + .any(|t| t == "wildcard_allowlist")); + assert!(inspected.risk_tags.iter().any(|t| t == "no_allowlist")); + } } diff --git a/src/rules/allowlist.rs b/src/rules/allowlist.rs index 6664908..e54dcb1 100644 --- a/src/rules/allowlist.rs +++ b/src/rules/allowlist.rs @@ -1,4 +1,4 @@ -use crate::config::McpServer; +use crate::config::{has_effective_allowed_tools, has_global_wildcard_allowed_tools, McpServer}; use crate::rules::{Finding, Rule, Severity}; /// AW-007: Flag configs with no tool filtering (allowedTools). @@ -54,7 +54,8 @@ impl Rule for AllowlistRule { fn check(&self, server_name: &str, server: &McpServer, config_file: &str) -> Vec { let mut findings = Vec::new(); - let has_allowlist = server.allowed_tools.as_ref().is_some_and(|t| !t.is_empty()); + let has_allowlist = has_effective_allowed_tools(server); + let has_wildcard_allowlist = has_global_wildcard_allowed_tools(server); if !has_allowlist { let high_risk = Self::is_high_risk(server_name, server); @@ -64,22 +65,43 @@ impl Rule for AllowlistRule { Severity::Medium }; - let title = if high_risk { + let title = if has_wildcard_allowlist { + if high_risk { + "Wildcard tool allowlist on high-risk server" + } else { + "Wildcard tool allowlist is effectively unrestricted" + } + } else if high_risk { "No tool allowlist on high-risk server" } else { "No tool allowlist configured" }; + let message = if has_wildcard_allowlist { + format!( + "Server '{}' uses a global wildcard in allowedTools, which effectively exposes all tools", + server_name + ) + } else { + format!( + "Server '{}' exposes all available tools with no filtering", + server_name + ) + }; + + let fix = if has_wildcard_allowlist { + "Replace wildcard entries in \"allowedTools\" with explicit least-privilege tool names" + .to_string() + } else { + "Add \"allowedTools\" to restrict exposed tools to least privilege".to_string() + }; + findings.push(Finding { rule_id: self.id().to_string(), severity, title: title.to_string(), - message: format!( - "Server '{}' exposes all available tools with no filtering", - server_name - ), - fix: "Add \"allowedTools\" to restrict exposed tools to least privilege" - .to_string(), + message, + fix, config_file: config_file.to_string(), server_name: server_name.to_string(), source: None, @@ -147,4 +169,37 @@ mod tests { let findings = rule.check("test", &server, "test.json"); assert_eq!(findings.len(), 1); } + + #[test] + fn test_wildcard_allowlist_flagged() { + let rule = AllowlistRule; + let server = McpServer { + command: Some("npx".to_string()), + allowed_tools: Some(vec!["*".to_string()]), + ..Default::default() + }; + + let findings = rule.check("test", &server, "test.json"); + assert_eq!(findings.len(), 1); + assert!(findings[0].title.contains("Wildcard")); + assert_eq!(findings[0].severity, Severity::Medium); + } + + #[test] + fn test_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!["all".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/src/rules/shell.rs b/src/rules/shell.rs index f3658ed..deac172 100644 --- a/src/rules/shell.rs +++ b/src/rules/shell.rs @@ -1,4 +1,4 @@ -use crate::config::McpServer; +use crate::config::{has_effective_allowed_tools, McpServer}; use crate::rules::{Finding, Rule, Severity}; const SHELL_PATTERNS: &[&str] = &[ @@ -51,7 +51,7 @@ impl Rule for ShellRule { } // Check if there are any restrictions (allowedTools, allowedCommands in env) - let has_restrictions = server.allowed_tools.as_ref().is_some_and(|t| !t.is_empty()) + let has_restrictions = has_effective_allowed_tools(server) || server.env.as_ref().is_some_and(|env| { env.keys().any(|k| { let k = k.to_lowercase(); @@ -122,4 +122,19 @@ mod tests { let findings = rule.check("shell", &server, "test.json"); assert!(findings.is_empty()); } + + #[test] + fn test_shell_with_wildcard_allowlist_is_flagged() { + let rule = ShellRule; + let server = McpServer { + command: Some("npx".to_string()), + args: Some(vec!["-y".to_string(), "mcp-shell-server".to_string()]), + allowed_tools: Some(vec!["*".to_string()]), + ..Default::default() + }; + + let findings = rule.check("shell", &server, "test.json"); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].severity, Severity::Critical); + } } diff --git a/src/rules/write_tools.rs b/src/rules/write_tools.rs index 4231f1a..d49b5c4 100644 --- a/src/rules/write_tools.rs +++ b/src/rules/write_tools.rs @@ -1,4 +1,4 @@ -use crate::config::McpServer; +use crate::config::{has_effective_allowed_tools, McpServer}; use crate::rules::{Finding, Rule, Severity}; const WRITE_TOOL_PATTERNS: &[&str] = &[ @@ -55,7 +55,7 @@ impl Rule for WriteToolsRule { } // Only flag if there's no tool restriction - let has_allowlist = server.allowed_tools.as_ref().is_some_and(|t| !t.is_empty()); + let has_allowlist = has_effective_allowed_tools(server); if !has_allowlist { findings.push(Finding { @@ -126,4 +126,21 @@ mod tests { let findings = rule.check("memory", &server, "test.json"); assert!(findings.is_empty()); } + + #[test] + fn test_write_server_with_wildcard_allowlist_is_flagged() { + let rule = WriteToolsRule; + let server = McpServer { + command: Some("npx".to_string()), + args: Some(vec![ + "-y".to_string(), + "@modelcontextprotocol/server-postgres".to_string(), + ]), + allowed_tools: Some(vec!["all".to_string()]), + ..Default::default() + }; + + let findings = rule.check("database", &server, "test.json"); + assert_eq!(findings.len(), 1); + } }