From f37a5ffe6e5a48ee2b8d48c1d778cd07d96062c0 Mon Sep 17 00:00:00 2001 From: oudecheng <13802883547@139.com> Date: Mon, 3 Aug 2026 22:45:36 +0800 Subject: [PATCH] =?UTF-8?q?fix(platform):=20=E6=94=B6=E7=B4=A7=20safety=20?= =?UTF-8?q?guard=20=E7=9A=84=20format=20=E6=AD=A3=E5=88=99=E5=B9=B6?= =?UTF-8?q?=E6=8C=89=E5=B9=B3=E5=8F=B0=E5=88=86=E7=BB=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 原 \bformat\s+ 正则精度不足，会误伤 dart format、buf format、 pytest --format 等合法命令；Remove-Item 大写正则经 to_lowercase 后永远匹配不到，是潜在 bug。 改动： - format 正则收紧为 \bformat\s+.*[a-z]:，要求出现盘符才拦截 - 按 Platform 分组注入规则，Unix 不再注入 Windows 专用规则 - Remove-Item 正则改为小写，与 guard_command 的大小写处理一致 - 平台相关测试加 #[cfg] 属性，避免跨平台失败 - 新增 test_format_pattern_precision 覆盖危险命令与误伤场景 - 新增 test_legitimate_format_commands_not_blocked 回归测试 验证： - cargo test --lib platform:: 11/11 通过 - cargo test --lib tools::bash 13/13 通过 --- src/platform/mod.rs | 112 +++++++++++++++++++++++++++++++++++++------- src/tools/bash.rs | 24 ++++++++++ 2 files changed, 120 insertions(+), 16 deletions(-) diff --git a/src/platform/mod.rs b/src/platform/mod.rs index be72978..f763c9e 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -93,22 +93,47 @@ impl ShellInfo { } /// Dangerous command patterns for safety guards. +/// +/// Returns patterns filtered by the current platform. Platform-specific +/// rules (e.g. `format` on Windows, `rm` on Unix) are only injected on +/// their target platform to avoid false positives. pub fn dangerous_command_patterns() -> Vec { - vec![ - // Unix dangerous commands - r"\brm\s+-[rf]{1,2}\b".to_string(), - r"\bchmod\s+-[Rr]".to_string(), - r"\bchown\s+-[Rr]".to_string(), - // Windows dangerous commands - r"\bdel\s+/[fq]\b".to_string(), - r"\brmdir\s+/s\b".to_string(), - r"\bformat\s+".to_string(), - // PowerShell dangerous commands - r"\bRemove-Item\s+.*-Recurse".to_string(), - r"\bRemove-Item\s+.*-Force".to_string(), - // Fork bomb (cross-platform) - r":\(\)\s*\{.*\};\s*:".to_string(), - ] + dangerous_command_patterns_for_platform(Platform::current()) +} + +/// Platform-specific dangerous command patterns. +/// +/// Exposed primarily for testing. Callers should prefer +/// [`dangerous_command_patterns`] which auto-detects the platform. +pub fn dangerous_command_patterns_for_platform(platform: Platform) -> Vec { + let mut patterns: Vec = Vec::new(); + + // Cross-platform: fork bomb + patterns.push(r":\(\)\s*\{.*\};\s*:".to_string()); + + match platform { + Platform::Unix => { + // Unix dangerous commands + patterns.push(r"\brm\s+-[rf]{1,2}\b".to_string()); + patterns.push(r"\bchmod\s+-[Rr]".to_string()); + patterns.push(r"\bchown\s+-[Rr]".to_string()); + } + Platform::Windows => { + // Windows cmd dangerous commands. + // `format` requires a drive letter (`[a-z]:`) somewhere after it, + // so legitimate uses like `dart format lib/` or + // `pytest --format json` (no drive letter) are not matched. + patterns.push(r"\bformat\s+.*[a-z]:".to_string()); + patterns.push(r"\bdel\s+/[fq]\b".to_string()); + patterns.push(r"\brmdir\s+/s\b".to_string()); + // PowerShell dangerous commands. Patterns are lowercase because + // `guard_command` lowercases the command string before matching. + patterns.push(r"\bremove-item\s+.*-recurse".to_string()); + patterns.push(r"\bremove-item\s+.*-force".to_string()); + } + } + + patterns } /// Check whether a child process is blocked waiting for stdin input. @@ -435,9 +460,64 @@ mod tests { fn test_dangerous_patterns() { let patterns = dangerous_command_patterns(); assert!(!patterns.is_empty()); - // Should contain patterns for both platforms + // Cross-platform fork bomb rule is always present + assert!(patterns.iter().any(|p| p.contains(r":\(\)"))); + } + + #[test] + fn test_dangerous_patterns_unix() { + let patterns = dangerous_command_patterns_for_platform(Platform::Unix); + // Unix-specific rules assert!(patterns.iter().any(|p| p.contains("rm"))); + assert!(patterns.iter().any(|p| p.contains("chmod"))); + assert!(patterns.iter().any(|p| p.contains("chown"))); + // Windows-specific rules must NOT be present on Unix + assert!(!patterns.iter().any(|p| p.contains("format"))); + assert!(!patterns.iter().any(|p| p.contains("del"))); + assert!(!patterns.iter().any(|p| p.contains("remove-item"))); + } + + #[test] + fn test_dangerous_patterns_windows() { + let patterns = dangerous_command_patterns_for_platform(Platform::Windows); + // Windows-specific rules assert!(patterns.iter().any(|p| p.contains("del"))); + assert!(patterns.iter().any(|p| p.contains("format"))); + assert!(patterns.iter().any(|p| p.contains("remove-item"))); + // Unix-specific rules must NOT be present on Windows. + // Use r"\brm\s" to match rm-as-command without matching rmdir. + assert!(!patterns.iter().any(|p| p.contains(r"\brm\s"))); + assert!(!patterns.iter().any(|p| p.contains("chmod"))); + assert!(!patterns.iter().any(|p| p.contains("chown"))); + } + + #[test] + fn test_format_pattern_precision() { + let patterns = dangerous_command_patterns_for_platform(Platform::Windows); + let format_pat = patterns + .iter() + .find(|p| p.contains("format")) + .expect("format pattern should exist on Windows"); + let re = regex::Regex::new(format_pat).unwrap(); + + // Helper: guard_command lowercases before matching, so tests must too. + let m = |cmd: &str| re.is_match(&cmd.to_lowercase()); + + // Truly dangerous commands — should match + assert!(m("format c:")); + assert!(m("format /q c:")); + assert!(m("format d: /fs:ntfs")); + assert!(m("echo ok | format c:")); + assert!(m("echo ok; format c:")); + // Sub-shell invocation must also be caught + assert!(m(r#"cmd /c "format C:""#)); + + // Legitimate commands containing literal "format " — should NOT match + assert!(!m("dart format lib/")); + assert!(!m("buf format -w")); + assert!(!m("pytest --format json")); + assert!(!m(r#"echo "please format the disk""#)); + assert!(!m(r#"git log --pretty=format:"%h""#)); } #[test] diff --git a/src/tools/bash.rs b/src/tools/bash.rs index 0ac5361..73e1837 100644 --- a/src/tools/bash.rs +++ b/src/tools/bash.rs @@ -750,6 +750,7 @@ mod tests { assert!(result.success); } + #[cfg(unix)] #[tokio::test] async fn test_dangerous_rm() { let tool = BashTool::default(); @@ -763,6 +764,7 @@ mod tests { assert!(result.error.unwrap().contains("blocked")); } + #[cfg(windows)] #[tokio::test] async fn test_dangerous_windows_commands() { let tool = BashTool::default(); @@ -788,6 +790,28 @@ mod tests { assert!(result.error.unwrap().contains("blocked")); } + #[tokio::test] + async fn test_legitimate_format_commands_not_blocked() { + // Commands that merely contain the literal "format " but are NOT + // Windows disk-format operations must not be blocked by safety guard. + let tool = BashTool::default(); + let commands = [ + r#"echo "please format the disk""#, + r#"git log --pretty=format:"%h""#, + ]; + for cmd in commands { + let result = tool.execute(json!({ "command": cmd })).await.unwrap(); + if let Some(err) = &result.error { + assert!( + !err.contains("blocked"), + "command {:?} should not be blocked, but got: {}", + cmd, + err + ); + } + } + } + #[tokio::test] async fn test_missing_command() { let tool = BashTool::default();