Harden Windows PowerShell parser trust

codex-rs/shell-command/src/command_safety/is_safe_command.rs

@@ -621,12 +621,10 @@ mod tests {
             return;
         }
 
-        let Some(powershell) = crate::powershell::try_find_pwsh_executable_blocking()
-            .or_else(crate::powershell::try_find_powershell_executable_blocking)
-        else {
+        let powershell = r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe";
+        if !std::path::Path::new(powershell).is_file() {
             return;
-        };
-        let powershell = powershell.as_path().to_str().unwrap();
+        }
 
         assert!(is_known_safe_command(&vec_str(&[
             powershell,

codex-rs/shell-command/src/command_safety/powershell_parser.rs

@@ -7,6 +7,8 @@ use std::io::BufRead;
 use std::io::BufReader;
 use std::io::ErrorKind;
 use std::io::Write;
+#[cfg(any(test, windows))]
+use std::path::Path;
 use std::process::Child;
 use std::process::ChildStdin;
 use std::process::ChildStdout;
@@ -17,6 +19,10 @@ use std::sync::Mutex;
 use std::sync::PoisonError;
 
 const POWERSHELL_PARSER_SCRIPT: &str = include_str!("powershell_parser.ps1");
+#[cfg(any(test, windows))]
+const WINDOWS_POWERSHELL_EXE: &str = r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe";
+#[cfg(any(test, windows))]
+const WINDOWS_PWSH_EXE: &str = r"C:\Program Files\PowerShell\7\pwsh.exe";
 
 /// Cache one long-lived parser process per executable path so repeated safety checks reuse
 /// PowerShell startup work while still consulting the real parser every time.
@@ -24,7 +30,7 @@ const POWERSHELL_PARSER_SCRIPT: &str = include_str!("powershell_parser.ps1");
 /// We keep the cache behind one mutex because each child process speaks a simple
 /// request/response protocol over a single stdin/stdout pair, so callers targeting the same
 /// executable must serialize access anyway.
-pub(super) fn parse_with_powershell_ast(executable: &str, script: &str) -> PowershellParseOutcome {
+fn parse_with_powershell_ast(executable: &str, script: &str) -> PowershellParseOutcome {
     static PARSER_PROCESSES: LazyLock<Mutex<HashMap<String, PowershellParserProcess>>> =
         LazyLock::new(|| Mutex::new(HashMap::new()));
 
@@ -38,12 +44,49 @@ pub(crate) fn try_parse_powershell_ast_commands(
     executable: &str,
     script: &str,
 ) -> Option<Vec<Vec<String>>> {
-    match parse_with_powershell_ast(executable, script) {
+    let parser_executable = trusted_powershell_parser_executable(executable)?;
+    match parse_with_powershell_ast(parser_executable, script) {
         PowershellParseOutcome::Commands(commands) => Some(commands),
         PowershellParseOutcome::Unsupported | PowershellParseOutcome::Failed => None,
     }
 }
 
+/// Selects the host-side parser only when the command itself names that same trusted binary.
+///
+/// The parser runs before the command approval and sandbox boundaries. Bare executable names and
+/// arbitrary paths are therefore not eligible: their eventual resolution can depend on the
+/// workspace, current directory, or `PATH`. Returning a fixed path also ensures the parser spawn
+/// never receives the command's attacker-controlled `argv[0]`.
+fn trusted_powershell_parser_executable(executable: &str) -> Option<&'static str> {
+    #[cfg(windows)]
+    {
+        resolve_trusted_powershell_parser_executable(executable, Path::is_file)
+    }
+
+    #[cfg(not(windows))]
+    {
+        let _ = executable;
+        None
+    }
+}
+
+#[cfg(any(test, windows))]
+fn resolve_trusted_powershell_parser_executable(
+    executable: &str,
+    is_file: impl Fn(&Path) -> bool,
+) -> Option<&'static str> {
+    let normalized_executable = executable
+        .strip_prefix(r"\\?\")
+        .unwrap_or(executable)
+        .replace('/', "\\");
+
+    let trusted_path = [WINDOWS_POWERSHELL_EXE, WINDOWS_PWSH_EXE]
+        .into_iter()
+        .find(|candidate| normalized_executable.eq_ignore_ascii_case(candidate))?;
+
+    is_file(Path::new(trusted_path)).then_some(trusted_path)
+}
+
 #[derive(Debug, PartialEq, Eq)]
 pub(super) enum PowershellParseOutcome {
     Commands(Vec<Vec<String>>),
@@ -264,6 +307,10 @@ fn kill_child(child: &mut Child) {
     let _ = child.wait();
 }
 
+#[cfg(test)]
+#[path = "powershell_parser_trust_tests.rs"]
+mod trust_tests;
+
 #[cfg(all(test, windows))]
 mod tests {
     use super::*;

codex-rs/shell-command/src/command_safety/powershell_parser_trust_tests.rs

@@ -0,0 +1,84 @@
+use super::*;
+use pretty_assertions::assert_eq;
+
+fn resolve_existing(executable: &str) -> Option<&'static str> {
+    resolve_trusted_powershell_parser_executable(executable, |_| true)
+}
+
+#[test]
+fn accepts_only_fixed_trusted_powershell_paths() {
+    assert_eq!(
+        resolve_existing(WINDOWS_POWERSHELL_EXE),
+        Some(WINDOWS_POWERSHELL_EXE),
+    );
+    assert_eq!(
+        resolve_existing(r"c:/WINDOWS/System32/WindowsPowerShell/v1.0/POWERSHELL.EXE"),
+        Some(WINDOWS_POWERSHELL_EXE),
+    );
+    assert_eq!(
+        resolve_existing(r"\\?\C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe"),
+        Some(WINDOWS_POWERSHELL_EXE),
+    );
+    assert_eq!(resolve_existing(WINDOWS_PWSH_EXE), Some(WINDOWS_PWSH_EXE));
+    assert_eq!(
+        resolve_existing(r"c:/program files/powershell/7/PWSH.EXE"),
+        Some(WINDOWS_PWSH_EXE),
+    );
+}
+
+#[test]
+fn rejects_search_path_and_workspace_controlled_variants() {
+    for executable in [
+        "powershell",
+        "powershell.exe",
+        "pwsh",
+        "pwsh.exe",
+        r".\powershell.exe",
+        r".\pwsh.exe",
+        "./powershell.exe",
+        "./pwsh.exe",
+        r"tools\powershell.exe",
+        r"tools\pwsh.exe",
+        r"C:\workspace\powershell.exe",
+        r"C:\workspace\pwsh.exe",
+        r"\\server\share\pwsh.exe",
+    ] {
+        assert_eq!(
+            resolve_existing(executable),
+            None,
+            "{executable:?} must not be launched as a parser",
+        );
+    }
+}
+
+#[test]
+fn rejects_command_wrappers_and_path_aliases() {
+    for executable in [
+        r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.cmd",
+        r"C:\Program Files\PowerShell\7\pwsh.cmd",
+        r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell",
+        r"C:\Program Files\PowerShell\7\pwsh",
+        r"C:\Windows\System32\WindowsPowerShell\v1.0\..\powershell.exe",
+        r"\\.\C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe",
+        r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe.cmd",
+        r"C:\Program Files\PowerShell\7\pwsh.exe.cmd",
+    ] {
+        assert_eq!(
+            resolve_existing(executable),
+            None,
+            "{executable:?} must fail closed",
+        );
+    }
+}
+
+#[test]
+fn fails_closed_when_the_trusted_install_is_missing() {
+    assert_eq!(
+        resolve_trusted_powershell_parser_executable(WINDOWS_POWERSHELL_EXE, |_| false),
+        None,
+    );
+    assert_eq!(
+        resolve_trusted_powershell_parser_executable(WINDOWS_PWSH_EXE, |_| false),
+        None,
+    );
+}

codex-rs/shell-command/src/command_safety/windows_safe_commands.rs

@@ -1,6 +1,5 @@
 use crate::command_safety::is_safe_command::is_safe_git_command;
-use crate::command_safety::powershell_parser::PowershellParseOutcome;
-use crate::command_safety::powershell_parser::parse_with_powershell_ast;
+use crate::command_safety::powershell_parser::try_parse_powershell_ast_commands;
 use std::path::Path;
 
 /// On Windows, we conservatively allow only clearly read-only PowerShell invocations
@@ -94,13 +93,7 @@ fn parse_powershell_invocation(executable: &str, args: &[String]) -> Option<Vec<
 /// Tokenizes an inline PowerShell script and delegates to the command splitter.
 /// Examples of when this is called: pwsh.exe -Command '<script>' or pwsh.exe -Command:<script>
 fn parse_powershell_script(executable: &str, script: &str) -> Option<Vec<Vec<String>>> {
-    if let PowershellParseOutcome::Commands(commands) =
-        parse_with_powershell_ast(executable, script)
-    {
-        Some(commands)
-    } else {
-        None
-    }
+    try_parse_powershell_ast_commands(executable, script)
 }
 
 /// Returns true when the executable name is one of the supported PowerShell binaries.
@@ -224,10 +217,16 @@ fn is_safe_ripgrep(words: &[String]) -> bool {
 #[cfg(all(test, windows))]
 mod tests {
     use super::*;
-    use crate::powershell::try_find_pwsh_executable_blocking;
     use pretty_assertions::assert_eq;
     use std::string::ToString;
 
+    const POWERSHELL_EXE: &str = r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe";
+    const PWSH_EXE: &str = r"C:\Program Files\PowerShell\7\pwsh.exe";
+
+    fn installed_pwsh() -> Option<&'static str> {
+        Path::new(PWSH_EXE).is_file().then_some(PWSH_EXE)
+    }
+
     /// Converts a slice of string literals into owned `String`s for the tests.
     fn vec_str(args: &[&str]) -> Vec<String> {
         args.iter().map(ToString::to_string).collect()
@@ -236,29 +235,29 @@ mod tests {
     #[test]
     fn recognizes_safe_powershell_wrappers() {
         assert!(is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-NoLogo",
             "-Command",
             "Get-ChildItem -Path .",
         ])));
 
         assert!(is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-NoProfile",
             "-Command",
             "git status",
         ])));
 
         assert!(is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "Get-Content",
             "Cargo.toml",
         ])));
 
         // pwsh parity
-        if let Some(pwsh) = try_find_pwsh_executable_blocking() {
+        if let Some(pwsh) = installed_pwsh() {
             assert!(is_safe_command_windows(&[
-                pwsh.as_path().to_str().unwrap().into(),
+                pwsh.to_string(),
                 "-NoProfile".to_string(),
                 "-Command".to_string(),
                 "Get-ChildItem".to_string(),
@@ -273,9 +272,9 @@ mod tests {
             return;
         }
 
-        if let Some(pwsh) = try_find_pwsh_executable_blocking() {
+        if let Some(pwsh) = installed_pwsh() {
             assert!(is_safe_command_windows(&[
-                pwsh.as_path().to_str().unwrap().into(),
+                pwsh.to_string(),
                 "-NoProfile".to_string(),
                 "-Command".to_string(),
                 "Get-ChildItem -Path .".to_string(),
@@ -283,19 +282,52 @@ mod tests {
         }
 
         assert!(is_safe_command_windows(&vec_str(&[
-            r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Get-Content Cargo.toml",
         ])));
     }
 
+    #[test]
+    fn rejects_workspace_controlled_powershell_binaries_before_parser_spawn() {
+        if !Path::new(POWERSHELL_EXE).is_file() {
+            return;
+        }
+
+        let unique = std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap()
+            .as_nanos();
+        let test_dir = std::env::temp_dir().join(format!(
+            "codex-untrusted-powershell-parser-{}-{unique}",
+            std::process::id()
+        ));
+        std::fs::create_dir_all(&test_dir).unwrap();
+
+        let fake_powershell = test_dir.join("powershell.exe");
+        let fake_pwsh = test_dir.join("pwsh.exe");
+        std::fs::copy(POWERSHELL_EXE, &fake_powershell).unwrap();
+        std::fs::copy(POWERSHELL_EXE, &fake_pwsh).unwrap();
+
+        let results = [&fake_powershell, &fake_pwsh].map(|executable| {
+            is_safe_command_windows(&vec_str(&[
+                executable.to_str().unwrap(),
+                "-Command",
+                "Get-Location",
+            ]))
+        });
+
+        let _ = std::fs::remove_dir_all(test_dir);
+        assert_eq!([false, false], results);
+    }
+
     #[test]
     fn allows_read_only_pipelines_and_git_usage() {
-        let Some(pwsh) = try_find_pwsh_executable_blocking() else {
+        let Some(pwsh) = installed_pwsh() else {
             return;
         };
 
-        let pwsh: String = pwsh.as_path().to_str().unwrap().into();
+        let pwsh = pwsh.to_string();
         assert!(is_safe_command_windows(&[
             pwsh.clone(),
             "-NoLogo".to_string(),
@@ -334,11 +366,11 @@ mod tests {
 
     #[test]
     fn rejects_git_global_override_options() {
-        let Some(pwsh) = try_find_pwsh_executable_blocking() else {
+        let Some(pwsh) = installed_pwsh() else {
             return;
         };
 
-        let pwsh: String = pwsh.as_path().to_str().unwrap().into();
+        let pwsh = pwsh.to_string();
         for script in [
             "git -c core.pager=cat show HEAD:foo.rs",
             "git --config-env core.pager=PAGER show HEAD:foo.rs",
@@ -381,7 +413,7 @@ mod tests {
             (
                 script,
                 is_safe_command_windows(&[
-                    "powershell.exe".to_string(),
+                    POWERSHELL_EXE.to_string(),
                     "-NoProfile".to_string(),
                     "-Command".to_string(),
                     script.to_string(),
@@ -405,7 +437,7 @@ mod tests {
     #[test]
     fn rejects_stop_parsing_git_forms() {
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-NoProfile",
             "-Command",
             "git log --% HEAD --output=codex_poc.txt",
@@ -415,7 +447,7 @@ mod tests {
     #[test]
     fn rejects_powershell_param_blocks() {
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-NoProfile",
             "-Command",
             "param([string]$path = (Get-Location)) Write-Output test",
@@ -425,7 +457,7 @@ mod tests {
     #[test]
     fn rejects_powershell_named_blocks() {
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-NoProfile",
             "-Command",
             "begin { Set-Content codex_poc.txt pwned } end { Get-Content Cargo.toml }",
@@ -435,7 +467,7 @@ mod tests {
     #[test]
     fn rejects_powershell_using_statements() {
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-NoProfile",
             "-Command",
             "using module ./codex_poc.psm1\nGet-Content Cargo.toml",
@@ -445,7 +477,7 @@ mod tests {
     #[test]
     fn rejects_powershell_trap_blocks() {
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-NoProfile",
             "-Command",
             "trap { Set-Content codex_poc.txt pwned; continue } Get-Content missing -ErrorAction Stop",
@@ -455,97 +487,97 @@ mod tests {
     #[test]
     fn rejects_powershell_commands_with_side_effects() {
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-NoLogo",
             "-Command",
             "Remove-Item foo.txt",
         ])));
 
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-NoProfile",
             "-Command",
             "rg --pre cat",
         ])));
 
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Set-Content foo.txt 'hello'",
         ])));
 
         // Redirections are blocked
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "echo hi > out.txt",
         ])));
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Get-Content x | Out-File y",
         ])));
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Write-Output foo 2> err.txt",
         ])));
 
         // Call operator is blocked
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "& Remove-Item foo",
         ])));
 
         // Chained safe + unsafe must fail
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Get-ChildItem; Remove-Item foo",
         ])));
         // Nested unsafe cmdlet inside safe command must fail
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Write-Output (Set-Content foo6.txt 'abc')",
         ])));
         // Additional nested unsafe cmdlet examples must fail
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Write-Host (Remove-Item foo.txt)",
         ])));
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Get-Content (New-Item bar.txt)",
         ])));
 
         // Unsafe @ expansion.
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "ls @(calc.exe)"
         ])));
 
         // Unsupported constructs that the AST parser refuses (no fallback to manual splitting).
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "ls && pwd"
         ])));
 
         // Sub-expressions are rejected even if they contain otherwise safe commands.
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Write-Output $(Get-Content foo)"
         ])));
 
         // Empty words from the parser (e.g. '') are rejected.
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "''"
         ])));
@@ -554,13 +586,13 @@ mod tests {
     #[test]
     fn accepts_constant_expression_arguments() {
         assert!(is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Get-Content 'foo bar'"
         ])));
 
         assert!(is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Get-Content \"foo bar\""
         ])));
@@ -569,13 +601,13 @@ mod tests {
     #[test]
     fn rejects_dynamic_arguments() {
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Get-Content $foo"
         ])));
 
         assert!(!is_safe_command_windows(&vec_str(&[
-            "powershell.exe",
+            POWERSHELL_EXE,
             "-Command",
             "Write-Output \"foo $bar\""
         ])));
@@ -589,19 +621,16 @@ mod tests {
 
         let chain = "pwd && ls";
         assert!(
-            !is_safe_command_windows(&vec_str(&[
-                "powershell.exe",
-                "-NoProfile",
-                "-Command",
-                chain,
-            ])),
+            !is_safe_command_windows(&vec_str(
+                &[POWERSHELL_EXE, "-NoProfile", "-Command", chain,]
+            )),
             "`{chain}` is not recognized by powershell.exe"
         );
 
-        if let Some(pwsh) = try_find_pwsh_executable_blocking() {
+        if let Some(pwsh) = installed_pwsh() {
             assert!(
                 is_safe_command_windows(&[
-                    pwsh.as_path().to_str().unwrap().into(),
+                    pwsh.to_string(),
                     "-NoProfile".to_string(),
                     "-Command".to_string(),
                     chain.to_string(),

codex-rs/shell-command/src/powershell.rs

@@ -158,6 +158,10 @@ mod tests {
     use super::parse_powershell_command_into_plain_commands;
     use super::prefix_powershell_script_with_utf8;
 
+    #[cfg(windows)]
+    const WINDOWS_POWERSHELL_EXE: &str =
+        r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe";
+
     #[test]
     fn extracts_basic_powershell_command() {
         let cmd = vec![
@@ -240,7 +244,7 @@ mod tests {
     #[test]
     fn parses_plain_powershell_commands() {
         let commands = parse_powershell_command_into_plain_commands(&[
-            "powershell.exe".to_string(),
+            WINDOWS_POWERSHELL_EXE.to_string(),
             "-NoProfile".to_string(),
             "-Command".to_string(),
             "echo hi".to_string(),
@@ -254,7 +258,7 @@ mod tests {
     #[test]
     fn parses_multiple_plain_powershell_commands() {
         let commands = parse_powershell_command_into_plain_commands(&[
-            "powershell.exe".to_string(),
+            WINDOWS_POWERSHELL_EXE.to_string(),
             "-NoProfile".to_string(),
             "-Command".to_string(),
             "Write-Output foo | Measure-Object".to_string(),