From bf51a41a35e5d0d9eb22d9f0b8afa03d4ab71431 Mon Sep 17 00:00:00 2001 From: Brandon Date: Mon, 20 Apr 2026 21:10:17 -0400 Subject: [PATCH 1/2] feat: flag insecure websocket transport in AW-005 --- README.md | 4 +- src/rules/transport.rs | 150 +++++++++++++++++++++++++++++-------- testdata/ws-no-auth.json | 15 ++++ tests/integration_tests.rs | 20 +++++ 4 files changed, 155 insertions(+), 34 deletions(-) create mode 100644 testdata/ws-no-auth.json diff --git a/README.md b/README.md index af2b72f..48a53b9 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ From a scan of **109 MCP server entries** collected from public GitHub configs + - **100%** missing tool allowlists (AW-007) - **8.26%** had unrestricted filesystem access (AW-002) - **1.83%** exposed hardcoded secrets (AW-004) -- Insecure HTTP transport still present in public configs (AW-005) +- Insecure cleartext transport (`http://` and `ws://`) still present in public configs (AW-005) Full methodology, source attribution, and raw output are in [`research/FINDINGS.md`](research/FINDINGS.md) and [`research/scan-results.json`](research/scan-results.json). @@ -196,7 +196,7 @@ agentwise auto-detects and scans: | AW-002 | Overpermissioned filesystem access | Critical | | AW-003 | Unrestricted shell/exec access | Critical | | AW-004 | Secrets in plaintext config | High | -| AW-005 | Insecure transport (HTTP) | High | +| AW-005 | Insecure transport (`http://` or `ws://`) | High | | AW-006 | Known CVE match (embedded + OSV) | Critical/High | | AW-007 | Missing tool allowlist | Medium | | AW-008 | Write-capable tools without opt-in | Medium | diff --git a/src/rules/transport.rs b/src/rules/transport.rs index 4aeac79..c7ab1e6 100644 --- a/src/rules/transport.rs +++ b/src/rules/transport.rs @@ -1,9 +1,70 @@ use crate::config::McpServer; use crate::rules::{Finding, Rule, Severity}; -/// AW-005: Flag http:// URLs (insecure transport) for remote MCP servers. +/// AW-005: Flag cleartext remote transport endpoints (`http://`, `ws://`). pub struct TransportRule; +impl TransportRule { + fn insecure_scheme(value: &str) -> Option<&'static str> { + let trimmed = value.trim(); + if trimmed.starts_with("http://") { + Some("http") + } else if trimmed.starts_with("ws://") { + Some("ws") + } else { + None + } + } + + fn make_finding( + &self, + server_name: &str, + config_file: &str, + value: &str, + from_args: bool, + ) -> Option { + let scheme = Self::insecure_scheme(value)?; + if is_localhost(value) { + return None; + } + + let (title, message) = if from_args { + ( + "Insecure cleartext URL in args".to_string(), + format!( + "Server '{}' has insecure {} endpoint in args: {}", + server_name, + scheme.to_uppercase(), + value + ), + ) + } else { + ( + "Insecure cleartext transport".to_string(), + format!( + "Server '{}' uses unencrypted {} transport: {}", + server_name, + scheme.to_uppercase(), + value + ), + ) + }; + + Some(Finding { + rule_id: self.id().to_string(), + severity: Severity::High, + title, + message, + fix: "Change endpoint to TLS transport (https:// or wss://)".to_string(), + config_file: config_file.to_string(), + server_name: server_name.to_string(), + source: None, + epss: None, + sub_items: None, + }) + } +} + impl Rule for TransportRule { fn id(&self) -> &'static str { "AW-005" @@ -13,41 +74,15 @@ impl Rule for TransportRule { let mut findings = Vec::new(); if let Some(url) = &server.url { - if url.starts_with("http://") && !is_localhost(url) { - findings.push(Finding { - rule_id: self.id().to_string(), - severity: Severity::High, - title: "Insecure HTTP transport".to_string(), - message: format!("Server '{}' uses unencrypted HTTP: {}", server_name, url), - fix: "Change URL to use https://".to_string(), - config_file: config_file.to_string(), - server_name: server_name.to_string(), - source: None, - epss: None, - sub_items: None, - }); + if let Some(finding) = self.make_finding(server_name, config_file, url, false) { + findings.push(finding); } } - // Also check args for HTTP URLs if let Some(args) = &server.args { for arg in args { - if arg.starts_with("http://") && !is_localhost(arg) { - findings.push(Finding { - rule_id: self.id().to_string(), - severity: Severity::High, - title: "Insecure HTTP URL in args".to_string(), - message: format!( - "Server '{}' has insecure HTTP URL in args: {}", - server_name, arg - ), - fix: "Change URL to use https://".to_string(), - config_file: config_file.to_string(), - server_name: server_name.to_string(), - source: None, - epss: None, - sub_items: None, - }); + if let Some(finding) = self.make_finding(server_name, config_file, arg, true) { + findings.push(finding); } } } @@ -58,7 +93,9 @@ impl Rule for TransportRule { fn is_localhost(url: &str) -> bool { let url_lower = url.to_lowercase(); - url_lower.contains("://localhost") || url_lower.contains("://127.0.0.1") + url_lower.contains("://localhost") + || url_lower.contains("://127.0.0.1") + || url_lower.contains("://[::1]") } #[cfg(test)] @@ -77,6 +114,18 @@ mod tests { assert_eq!(findings[0].severity, Severity::High); } + #[test] + fn test_ws_url_flagged() { + let rule = TransportRule; + let server = McpServer { + url: Some("ws://api.example.com:8080/mcp".to_string()), + ..Default::default() + }; + let findings = rule.check("remote", &server, "test.json"); + assert_eq!(findings.len(), 1); + assert!(findings[0].message.contains("WS")); + } + #[test] fn test_https_url_ok() { let rule = TransportRule; @@ -88,6 +137,17 @@ mod tests { assert!(findings.is_empty()); } + #[test] + fn test_wss_url_ok() { + let rule = TransportRule; + let server = McpServer { + url: Some("wss://api.example.com/mcp".to_string()), + ..Default::default() + }; + let findings = rule.check("remote", &server, "test.json"); + assert!(findings.is_empty()); + } + #[test] fn test_localhost_http_ok() { let rule = TransportRule; @@ -99,6 +159,32 @@ mod tests { assert!(findings.is_empty()); } + #[test] + fn test_localhost_ws_ok() { + let rule = TransportRule; + let server = McpServer { + url: Some("ws://127.0.0.1:3000/mcp".to_string()), + ..Default::default() + }; + let findings = rule.check("local", &server, "test.json"); + assert!(findings.is_empty()); + } + + #[test] + fn test_ws_arg_flagged() { + let rule = TransportRule; + let server = McpServer { + args: Some(vec![ + "--endpoint".to_string(), + "ws://api.example.com:8080/mcp".to_string(), + ]), + ..Default::default() + }; + let findings = rule.check("remote", &server, "test.json"); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].title, "Insecure cleartext URL in args"); + } + #[test] fn test_no_url_ok() { let rule = TransportRule; diff --git a/testdata/ws-no-auth.json b/testdata/ws-no-auth.json new file mode 100644 index 0000000..95c8cb4 --- /dev/null +++ b/testdata/ws-no-auth.json @@ -0,0 +1,15 @@ +{ + "mcpServers": { + "realtime-api": { + "url": "ws://realtime.example.com/mcp", + "transport": "sse" + }, + "secure-realtime": { + "url": "wss://secure.example.com/mcp", + "transport": "sse", + "headers": { + "Authorization": "Bearer redacted" + } + } + } +} diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 2625c94..8140a6a 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -291,6 +291,26 @@ fn test_detects_insecure_transport() { assert!(findings.iter().any(|f| f["rule_id"] == "AW-005")); } +#[test] +fn test_detects_insecure_websocket_transport() { + let output = agentwise() + .args(["scan", "testdata/ws-no-auth.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-005" + && f["message"].as_str().is_some_and(|m| m.contains("WS"))), + "Expected AW-005 finding for ws:// endpoint, got: {}", + stdout + ); +} + #[test] fn test_detects_secrets() { let output = agentwise() From 34b0a72bb34fcb50114e10eb417c656d8e125951 Mon Sep 17 00:00:00 2001 From: Brandon Date: Mon, 20 Apr 2026 21:15:42 -0400 Subject: [PATCH 2/2] chore: satisfy clippy sort lint on latest toolchain --- src/report/html.rs | 2 +- src/report/terminal.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/report/html.rs b/src/report/html.rs index 2b5b424..41f0e52 100644 --- a/src/report/html.rs +++ b/src/report/html.rs @@ -326,7 +326,7 @@ pub fn render(result: &ScanResult) -> String { let total_findings = result.findings.len(); let mut sorted = result.findings.clone(); - sorted.sort_by(|a, b| b.severity.cmp(&a.severity)); + sorted.sort_by_key(|finding| std::cmp::Reverse(finding.severity)); let findings_html = if sorted.is_empty() { String::from( diff --git a/src/report/terminal.rs b/src/report/terminal.rs index 8183364..154c908 100644 --- a/src/report/terminal.rs +++ b/src/report/terminal.rs @@ -25,7 +25,7 @@ pub fn render(result: &ScanResult) -> String { // Individual findings let mut sorted = result.findings.clone(); - sorted.sort_by(|a, b| b.severity.cmp(&a.severity)); + sorted.sort_by_key(|finding| std::cmp::Reverse(finding.severity)); for finding in &sorted { out.push_str(&render_finding(finding));