From 3bdcbc72927acd3e0071da3df3247e1da7ec578c Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Fri, 21 Nov 2025 13:36:17 -0800 Subject: [PATCH] Windows: flag some invocations that launch browsers/URLs as dangerous (#7111) Prevent certain Powershell/cmd invocations from reaching the sandbox when they are trying to launch a browser, or run a command with a URL, etc. --- codex-rs/Cargo.lock | 3 + codex-rs/Cargo.toml | 3 +- codex-rs/core/Cargo.toml | 3 + .../command_safety/is_dangerous_command.rs | 10 + .../windows_dangerous_commands.rs | 316 ++++++++++++++++++ docs/windows_sandbox_security.md | 4 +- 6 files changed, 336 insertions(+), 3 deletions(-) create mode 100644 codex-rs/core/src/command_safety/windows_dangerous_commands.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c891e76fa..6ca084d18 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1131,11 +1131,13 @@ dependencies = [ "libc", "maplit", "mcp-types", + "once_cell", "openssl-sys", "os_info", "predicates", "pretty_assertions", "rand 0.9.2", + "regex", "regex-lite", "reqwest", "seccompiler", @@ -1161,6 +1163,7 @@ dependencies = [ "tracing-test", "tree-sitter", "tree-sitter-bash", + "url", "uuid", "walkdir", "which", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 226a91bef..6fb285c25 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -146,7 +146,7 @@ mime_guess = "2.0.5" multimap = "0.10.0" notify = "8.2.0" nucleo-matcher = "0.3.1" -once_cell = "1" +once_cell = "1.20.2" openssl-sys = "*" opentelemetry = "0.30.0" opentelemetry-appender-tracing = "0.30.0" @@ -165,6 +165,7 @@ rand = "0.9" ratatui = "0.29.0" ratatui-macros = "0.6.0" regex-lite = "0.1.7" +regex = "1.11.1" reqwest = "0.12" rmcp = { version = "0.8.5", default-features = false } schemars = "0.8.22" diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 669a9a63f..f0f1a430f 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -56,6 +56,9 @@ sha2 = { workspace = true } shlex = { workspace = true } similar = { workspace = true } strum_macros = { workspace = true } +url = { workspace = true } +once_cell = { workspace = true } +regex = { workspace = true } tempfile = { workspace = true } test-case = "3.3.1" test-log = { workspace = true } diff --git a/codex-rs/core/src/command_safety/is_dangerous_command.rs b/codex-rs/core/src/command_safety/is_dangerous_command.rs index 5df2023f0..96f73f3e8 100644 --- a/codex-rs/core/src/command_safety/is_dangerous_command.rs +++ b/codex-rs/core/src/command_safety/is_dangerous_command.rs @@ -5,6 +5,9 @@ use crate::sandboxing::SandboxPermissions; use crate::bash::parse_shell_lc_plain_commands; use crate::is_safe_command::is_known_safe_command; +#[cfg(windows)] +#[path = "windows_dangerous_commands.rs"] +mod windows_dangerous_commands; pub fn requires_initial_appoval( policy: AskForApproval, @@ -36,6 +39,13 @@ pub fn requires_initial_appoval( } pub fn command_might_be_dangerous(command: &[String]) -> bool { + #[cfg(windows)] + { + if windows_dangerous_commands::is_dangerous_command_windows(command) { + return true; + } + } + if is_dangerous_to_call_with_exec(command) { return true; } diff --git a/codex-rs/core/src/command_safety/windows_dangerous_commands.rs b/codex-rs/core/src/command_safety/windows_dangerous_commands.rs new file mode 100644 index 000000000..d4b418d93 --- /dev/null +++ b/codex-rs/core/src/command_safety/windows_dangerous_commands.rs @@ -0,0 +1,316 @@ +use std::path::Path; + +use once_cell::sync::Lazy; +use regex::Regex; +use shlex::split as shlex_split; +use url::Url; + +pub fn is_dangerous_command_windows(command: &[String]) -> bool { + // Prefer structured parsing for PowerShell/CMD so we can spot URL-bearing + // invocations of ShellExecute-style entry points before falling back to + // simple argv heuristics. + if is_dangerous_powershell(command) { + return true; + } + + if is_dangerous_cmd(command) { + return true; + } + + is_direct_gui_launch(command) +} + +fn is_dangerous_powershell(command: &[String]) -> bool { + let Some((exe, rest)) = command.split_first() else { + return false; + }; + if !is_powershell_executable(exe) { + return false; + } + // Parse the PowerShell invocation to get a flat token list we can scan for + // dangerous cmdlets/COM calls plus any URL-looking arguments. This is a + // best-effort shlex split of the script text, not a full PS parser. + let Some(parsed) = parse_powershell_invocation(rest) else { + return false; + }; + + let tokens_lc: Vec = parsed + .tokens + .iter() + .map(|t| t.trim_matches('\'').trim_matches('"').to_ascii_lowercase()) + .collect(); + let has_url = args_have_url(&parsed.tokens); + + if has_url + && tokens_lc.iter().any(|t| { + matches!( + t.as_str(), + "start-process" | "start" | "saps" | "invoke-item" | "ii" + ) || t.contains("start-process") + || t.contains("invoke-item") + }) + { + return true; + } + + if has_url + && tokens_lc + .iter() + .any(|t| t.contains("shellexecute") || t.contains("shell.application")) + { + return true; + } + + if let Some(first) = tokens_lc.first() { + // Legacy ShellExecute path via url.dll + if first == "rundll32" + && tokens_lc + .iter() + .any(|t| t.contains("url.dll,fileprotocolhandler")) + && has_url + { + return true; + } + if first == "mshta" && has_url { + return true; + } + if is_browser_executable(first) && has_url { + return true; + } + if matches!(first.as_str(), "explorer" | "explorer.exe") && has_url { + return true; + } + } + + false +} + +fn is_dangerous_cmd(command: &[String]) -> bool { + let Some((exe, rest)) = command.split_first() else { + return false; + }; + let Some(base) = executable_basename(exe) else { + return false; + }; + if base != "cmd" && base != "cmd.exe" { + return false; + } + + let mut iter = rest.iter(); + for arg in iter.by_ref() { + let lower = arg.to_ascii_lowercase(); + match lower.as_str() { + "/c" | "/r" | "-c" => break, + _ if lower.starts_with('/') => continue, + // Unknown tokens before the command body => bail. + _ => return false, + } + } + + let Some(first_cmd) = iter.next() else { + return false; + }; + // Classic `cmd /c start https://...` ShellExecute path. + if !first_cmd.eq_ignore_ascii_case("start") { + return false; + } + let remaining: Vec = iter.cloned().collect(); + args_have_url(&remaining) +} + +fn is_direct_gui_launch(command: &[String]) -> bool { + let Some((exe, rest)) = command.split_first() else { + return false; + }; + let Some(base) = executable_basename(exe) else { + return false; + }; + + // Explorer/rundll32/mshta or direct browser exe with a URL anywhere in args. + if matches!(base.as_str(), "explorer" | "explorer.exe") && args_have_url(rest) { + return true; + } + if matches!(base.as_str(), "mshta" | "mshta.exe") && args_have_url(rest) { + return true; + } + if (base == "rundll32" || base == "rundll32.exe") + && rest.iter().any(|t| { + t.to_ascii_lowercase() + .contains("url.dll,fileprotocolhandler") + }) + && args_have_url(rest) + { + return true; + } + if is_browser_executable(&base) && args_have_url(rest) { + return true; + } + + false +} + +fn args_have_url(args: &[String]) -> bool { + args.iter().any(|arg| looks_like_url(arg)) +} + +fn looks_like_url(token: &str) -> bool { + // Strip common PowerShell punctuation around inline URLs (quotes, parens, trailing semicolons). + // Capture the middle token after trimming leading quotes/parens/whitespace and trailing semicolons/closing parens. + static RE: Lazy> = + Lazy::new(|| Regex::new(r#"^[ "'\(\s]*([^\s"'\);]+)[\s;\)]*$"#).ok()); + // If the token embeds a URL alongside other text (e.g., Start-Process('https://...')) + // as a single shlex token, grab the substring starting at the first URL prefix. + let urlish = token + .find("https://") + .or_else(|| token.find("http://")) + .map(|idx| &token[idx..]) + .unwrap_or(token); + + let candidate = RE + .as_ref() + .and_then(|re| re.captures(urlish)) + .and_then(|caps| caps.get(1)) + .map(|m| m.as_str()) + .unwrap_or(urlish); + let Ok(url) = Url::parse(candidate) else { + return false; + }; + matches!(url.scheme(), "http" | "https") +} + +fn executable_basename(exe: &str) -> Option { + Path::new(exe) + .file_name() + .and_then(|osstr| osstr.to_str()) + .map(str::to_ascii_lowercase) +} + +fn is_powershell_executable(exe: &str) -> bool { + matches!( + executable_basename(exe).as_deref(), + Some("powershell") | Some("powershell.exe") | Some("pwsh") | Some("pwsh.exe") + ) +} + +fn is_browser_executable(name: &str) -> bool { + matches!( + name, + "chrome" + | "chrome.exe" + | "msedge" + | "msedge.exe" + | "firefox" + | "firefox.exe" + | "iexplore" + | "iexplore.exe" + ) +} + +struct ParsedPowershell { + tokens: Vec, +} + +fn parse_powershell_invocation(args: &[String]) -> Option { + if args.is_empty() { + return None; + } + + let mut idx = 0; + while idx < args.len() { + let arg = &args[idx]; + let lower = arg.to_ascii_lowercase(); + match lower.as_str() { + "-command" | "/command" | "-c" => { + let script = args.get(idx + 1)?; + if idx + 2 != args.len() { + return None; + } + let tokens = shlex_split(script)?; + return Some(ParsedPowershell { tokens }); + } + _ if lower.starts_with("-command:") || lower.starts_with("/command:") => { + if idx + 1 != args.len() { + return None; + } + let (_, script) = arg.split_once(':')?; + let tokens = shlex_split(script)?; + return Some(ParsedPowershell { tokens }); + } + "-nologo" | "-noprofile" | "-noninteractive" | "-mta" | "-sta" => { + idx += 1; + } + _ if lower.starts_with('-') => { + idx += 1; + } + _ => { + let rest = args[idx..].to_vec(); + return Some(ParsedPowershell { tokens: rest }); + } + } + } + + None +} + +#[cfg(test)] +mod tests { + use super::is_dangerous_command_windows; + + fn vec_str(items: &[&str]) -> Vec { + items.iter().map(std::string::ToString::to_string).collect() + } + + #[test] + fn powershell_start_process_url_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-NoLogo", + "-Command", + "Start-Process 'https://example.com'" + ]))); + } + + #[test] + fn powershell_start_process_url_with_trailing_semicolon_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Start-Process('https://example.com');" + ]))); + } + + #[test] + fn powershell_start_process_local_is_not_flagged() { + assert!(!is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Start-Process notepad.exe" + ]))); + } + + #[test] + fn cmd_start_with_url_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "cmd", + "/c", + "start", + "https://example.com" + ]))); + } + + #[test] + fn msedge_with_url_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "msedge.exe", + "https://example.com" + ]))); + } + + #[test] + fn explorer_with_directory_is_not_flagged() { + assert!(!is_dangerous_command_windows(&vec_str(&[ + "explorer.exe", + "." + ]))); + } +} diff --git a/docs/windows_sandbox_security.md b/docs/windows_sandbox_security.md index e8008964f..79f8f781b 100644 --- a/docs/windows_sandbox_security.md +++ b/docs/windows_sandbox_security.md @@ -8,7 +8,7 @@ When commands run via `codex sandbox windows …` (or when the CLI/TUI calls int ## Known Security Limitations -Running `python windows-sandbox-rs/sandbox_smoketests.py` with full filesystem and network access currently results in **37/42** passing cases. The list below focuses on the four high-value failures numbered #32 and higher in the smoketests (earlier tests are less security-focused). +Running `python windows-sandbox-rs/sandbox_smoketests.py` with full filesystem and network access currently results in **37/41** passing cases. The list below focuses on the four high-value failures numbered #32 and higher in the smoketests (earlier tests are less security-focused). | Test | Purpose | | --------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | @@ -19,4 +19,4 @@ Running `python windows-sandbox-rs/sandbox_smoketests.py` with full filesystem a ## Want to Help? -If you are a security-minded Windows user, help us get these tests passing! Improved implementations that make these smoke tests pass meaningfully reduce Codex's escape surface. After iterating, rerun `python windows-sandbox-rs/sandbox_smoketests.py` to validate the fixes and help us drive the suite toward 42/42. +If you are a security-minded Windows user, help us get these tests passing! Improved implementations that make these smoke tests pass meaningfully reduce Codex's escape surface. After iterating, rerun `python windows-sandbox-rs/sandbox_smoketests.py` to validate the fixes and help us drive the suite toward 41/41.