mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Harden Windows updater command execution (#40422)
## Why Windows update commands could be influenced by the current project through relative `PATH` entries or project-local package-manager configuration. ## What changed - Resolve package-manager commands and `powershell.exe` using only absolute `PATH` entries before starting an update. - Run the resolved command from a temporary directory so the current project does not affect the update, while retaining Windows `.cmd` and `.bat` handling. - Reject a relative-only `PATH` with guidance to update manually. ## Testing Added a Windows regression test that verifies relative `PATH` entries and current-directory command decoys are ignored. GitOrigin-RevId: 15303165af42718244b99786f939f400dc6cbd07
This commit is contained in:
+83
-13
@@ -842,23 +842,30 @@ fn run_update_action(action: UpdateAction) -> anyhow::Result<()> {
|
||||
println!();
|
||||
let cmd_str = action.command_str();
|
||||
println!("Updating Codex via `{cmd_str}`...");
|
||||
|
||||
let status = {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
if action == UpdateAction::StandaloneWindows {
|
||||
let (cmd, args) = action.command_args();
|
||||
// Run the standalone PowerShell installer with PowerShell
|
||||
// itself. Routing this through `cmd.exe /C` would parse
|
||||
// PowerShell metacharacters like `|` before PowerShell sees
|
||||
// the installer command.
|
||||
std::process::Command::new(cmd).args(args).status()?
|
||||
let (cmd, args) = action.command_args();
|
||||
let cmd = if action == UpdateAction::StandaloneWindows {
|
||||
// These args contain PowerShell metacharacters, so do not let
|
||||
// PATHEXT select a batch shim for this action.
|
||||
"powershell.exe"
|
||||
} else {
|
||||
// On Windows, run via cmd.exe so .CMD/.BAT are correctly resolved (PATHEXT semantics).
|
||||
std::process::Command::new("cmd")
|
||||
.args(["/C", &cmd_str])
|
||||
.status()?
|
||||
}
|
||||
cmd
|
||||
};
|
||||
let path_env =
|
||||
std::env::var_os("PATH").ok_or_else(|| anyhow::anyhow!("PATH is not set"))?;
|
||||
let command_path = resolve_windows_update_command_from_path(cmd, &path_env)?;
|
||||
// Do not let a project-local command or package-manager config
|
||||
// influence the updater after the user accepts the update prompt.
|
||||
let update_cwd = tempfile::tempdir()?;
|
||||
// Resolve through PATH without consulting the project cwd. When
|
||||
// this returns a .cmd/.bat shim, std::process::Command routes the
|
||||
// absolute path through the system command processor.
|
||||
std::process::Command::new(command_path)
|
||||
.args(args)
|
||||
.current_dir(update_cwd.path())
|
||||
.status()?
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
@@ -880,6 +887,23 @@ fn run_update_action(action: UpdateAction) -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn resolve_windows_update_command_from_path(
|
||||
command: &str,
|
||||
path_env: &std::ffi::OsStr,
|
||||
) -> anyhow::Result<PathBuf> {
|
||||
let path_env =
|
||||
std::env::join_paths(std::env::split_paths(path_env).filter(|path| path.is_absolute()))?;
|
||||
if path_env.is_empty() {
|
||||
anyhow::bail!(
|
||||
"Could not find an absolute update command `{command}` on PATH. Please update manually: https://developers.openai.com/codex/cli/"
|
||||
);
|
||||
}
|
||||
which::which_in_global(command, Some(&path_env))?
|
||||
.next()
|
||||
.ok_or_else(|| anyhow::anyhow!("could not find update command `{command}` on PATH"))
|
||||
}
|
||||
|
||||
fn run_update_command() -> anyhow::Result<()> {
|
||||
#[cfg(debug_assertions)]
|
||||
{
|
||||
@@ -2862,6 +2886,52 @@ mod tests {
|
||||
assert!(size < 64 * 1024, "interactive TUI future is {size} bytes");
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_update_command_resolution_ignores_relative_path_entries() {
|
||||
let cwd = std::env::current_dir().expect("current directory");
|
||||
let decoy_dir = tempfile::tempdir_in(&cwd).expect("relative decoy directory");
|
||||
let trusted_dir = tempfile::tempdir().expect("trusted PATH directory");
|
||||
let relative_decoy_dir = decoy_dir
|
||||
.path()
|
||||
.strip_prefix(&cwd)
|
||||
.expect("decoy directory should be relative to cwd");
|
||||
|
||||
for command in ["npm.cmd", "pnpm.cmd", "bun.exe"] {
|
||||
std::fs::write(decoy_dir.path().join(command), "decoy")
|
||||
.expect("write cwd-relative decoy");
|
||||
std::fs::write(trusted_dir.path().join(command), "trusted")
|
||||
.expect("write trusted PATH command");
|
||||
let path_env = std::env::join_paths([relative_decoy_dir, trusted_dir.path()])
|
||||
.expect("join synthetic PATH");
|
||||
|
||||
let resolved = resolve_windows_update_command_from_path(command, &path_env)
|
||||
.expect("resolve update command");
|
||||
|
||||
assert_eq!(resolved, trusted_dir.path().join(command));
|
||||
}
|
||||
|
||||
let cwd_decoy = tempfile::Builder::new()
|
||||
.suffix(".cmd")
|
||||
.tempfile_in(&cwd)
|
||||
.expect("cwd-local decoy");
|
||||
let command = cwd_decoy
|
||||
.path()
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.expect("decoy filename");
|
||||
let relative_only_path_env = std::env::join_paths(["."]).expect("join relative-only PATH");
|
||||
let err = resolve_windows_update_command_from_path(command, &relative_only_path_env)
|
||||
.expect_err("relative-only PATH should not resolve a cwd command");
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
format!(
|
||||
"Could not find an absolute update command `{command}` on PATH. Please update manually: https://developers.openai.com/codex/cli/"
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn updater_http_client_factory_honors_respect_system_proxy() {
|
||||
let codex_home = tempfile::tempdir().expect("temporary Codex home");
|
||||
|
||||
Reference in New Issue
Block a user