github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-19 16:17:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Avoid executing PATH helpers before workspace trust (#42324)

Browse Source 
## Why

Automatic startup work and `codex doctor` can run before a workspace is
trusted. A repository-controlled `PATH` must not be able to make those flows
execute workspace-provided helpers.

## What changed

- Resolve helpers used by automatic startup from trusted system installation
  directories, and give plugin-sync Git subprocesses a sanitized environment.
- Make terminal detection environment-only and have doctor inspect executable
  locations without running them. Fetch update metadata with the HTTP client
  instead of `curl`.
- Fall back conservatively when trusted terminal helpers are unavailable,
  including for tmux keyboard enhancement flags.

## Testing

Add black-box coverage with hostile workspace `PATH` entries for startup,
interactive tmux startup, support log collection, doctor, and curated plugin
sync. Add unit coverage for trusted executable resolution and bounded update
HTTP responses.

GitOrigin-RevId: 3b8995eb422b60ed53b0386951de59e8f9bfc542
This commit is contained in:
Felipe Coury
2026-09-02 18:34:27 +00:00
committed by copyberry
parent 5e26f7621c
commit 637c3227b3
19 changed files with 975 additions and 906 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/Cargo.lock
Generated
+2 -1
View File
@@ -2468,6 +2468,7 @@ dependencies = [
"codex-utils-cargo-bin",
"codex-utils-cli",
"codex-utils-path",
"codex-utils-pty",
"codex-windows-sandbox",
"crossterm",
"flate2",
@@ -2997,7 +2998,6 @@ dependencies = [
"tracing-test",
"url",
"uuid",
"which 8.0.0",
"wiremock",
"zip",
]
@@ -4769,6 +4769,7 @@ dependencies = [
"dunce",
"pretty_assertions",
"tempfile",
"windows-sys 0.52.0",
]
[[package]]
codex-rs/cli/Cargo.toml
+1
View File
@@ -115,6 +115,7 @@ app_test_support = { workspace = true }
assert_cmd = { workspace = true }
assert_matches = { workspace = true }
codex-utils-cargo-bin = { workspace = true }
codex-utils-pty = { workspace = true }
flate2 = { workspace = true }
futures = { workspace = true }
insta = { workspace = true }
codex-rs/cli/src/doctor.rs
+25 -208
View File
@@ -4,7 +4,8 @@
//! configuration, authentication, terminal, state paths, and bounded reachability
//! probes without attempting repair or starting long-lived services. Each check
//! returns a redacted, serializable row so the same data can back the human
//! summary and `--json` support report.
//! summary and `--json` support report. PATH entries are untrusted data: checks
//! may inspect them, but must not execute the programs they select.
//!
//! A failing check should describe the problem and remediation, but it should not
//! mutate user state. That keeps the command safe to run before filing a support
@@ -21,7 +22,6 @@ use std::io::Read;
use std::net::IpAddr;
use std::path::Path;
use std::path::PathBuf;
use std::process::Command;
use std::sync::Arc;
use std::time::Duration;
use std::time::Instant;
@@ -133,10 +133,6 @@ const COLOR_ENV_VARS: &[&str] = &[
const TERMINAL_DIMENSION_ENV_VARS: &[&str] = &["COLUMNS", "LINES"];
const TERMINFO_ENV_VARS: &[&str] = &["TERMINFO", "TERMINFO_DIRS"];
const LOCALE_ENV_VARS: &[&str] = &["LC_ALL", "LC_CTYPE", "LANG"];
#[cfg(windows)]
const NPM_COMMAND: &str = "npm.cmd";
#[cfg(not(windows))]
const NPM_COMMAND: &str = "npm";
const REMOTE_TERMINAL_ENV_VARS: &[&str] = &[
"SSH_TTY",
"SSH_CONNECTION",
@@ -150,13 +146,6 @@ const REMOTE_TERMINAL_ENV_VARS: &[&str] = &[
"DISPLAY",
"WT_SESSION",
];
const TMUX_OPTION_NAMES: &[&str] = &[
"extended-keys",
"xterm-keys",
"allow-passthrough",
"set-clipboard",
"focus-events",
];
const NARROW_TERMINAL_COLUMNS: u16 = 80;
const NARROW_TERMINAL_ROWS: u16 = 24;
@@ -430,7 +419,7 @@ async fn build_report(
),
})
},
async { run_sync_check("updates", progress.clone(), || updates_check(config)) },
run_async_check("updates", progress.clone(), updates_check(config)),
async {
run_sync_check("network", progress.clone(), || network::check(Some(config)))
},
@@ -450,7 +439,9 @@ async fn build_report(
terminal_check(command.no_color)
})
},
run_async_check("git", progress.clone(), git_check(config.cwd.as_path())),
async {
run_sync_check("git", progress.clone(), || git_check(config.cwd.as_path()))
},
async {
run_sync_check("terminal title", progress.clone(), || {
terminal_title_check(config)
@@ -528,7 +519,7 @@ async fn build_report(
terminal_check(command.no_color)
})
},
run_async_check("git", progress.clone(), git_check(&cwd)),
async { run_sync_check("git", progress.clone(), || git_check(&cwd)) },
async { run_sync_check("state", progress.clone(), fallback_state_check) },
run_async_check(
"provider reachability",
@@ -907,41 +898,15 @@ fn installation_check(show_details: bool) -> DoctorCheck {
}
if doctor_managed_by_npm(current_exe.as_deref()) {
match npm_global_root_check() {
NpmRootCheck::Match { package_root } => {
details.push(format!("npm update target: {}", package_root.display()));
}
NpmRootCheck::Mismatch {
running_package_root,
npm_package_root,
} => {
status = CheckStatus::Fail;
summary =
"npm install -g @openai/codex would update a different install".to_string();
remediation = Some(format!(
"Fix PATH or npm prefix so the running package root ({}) matches the npm global package root ({}).",
running_package_root.display(),
npm_package_root.display()
));
details.push(format!(
"running package root: {}",
running_package_root.display()
));
details.push(format!("npm package root: {}", npm_package_root.display()));
}
NpmRootCheck::MissingPackageRoot => {
status = status.max(CheckStatus::Warning);
summary = "npm-managed launch is missing package-root provenance".to_string();
remediation = Some(
"Reinstall or update Codex so the JS shim provides CODEX_MANAGED_PACKAGE_ROOT."
.to_string(),
);
}
NpmRootCheck::NpmUnavailable(error) => {
status = status.max(CheckStatus::Warning);
summary = "npm-managed launch could not inspect npm global root".to_string();
details.push(format!("npm root -g failed: {error}"));
}
details
.push("npm update target: not inspected (PATH helpers are not executed)".to_string());
if env::var_os("CODEX_MANAGED_PACKAGE_ROOT").is_none() {
status = status.max(CheckStatus::Warning);
summary = "npm-managed launch is missing package-root provenance".to_string();
remediation = Some(
"Reinstall or update Codex so the JS shim provides CODEX_MANAGED_PACKAGE_ROOT."
.to_string(),
);
}
}
@@ -1063,62 +1028,6 @@ fn display_optional_path(path: Option<&Path>) -> String {
.unwrap_or_else(|| "none".to_string())
}
#[derive(Debug, PartialEq, Eq)]
enum NpmRootCheck {
Match {
package_root: PathBuf,
},
Mismatch {
running_package_root: PathBuf,
npm_package_root: PathBuf,
},
MissingPackageRoot,
NpmUnavailable(String),
}
fn npm_global_root_check() -> NpmRootCheck {
let Some(running_package_root) = env::var_os("CODEX_MANAGED_PACKAGE_ROOT").map(PathBuf::from)
else {
return NpmRootCheck::MissingPackageRoot;
};
let output = match run_command(NPM_COMMAND, ["root", "-g"]) {
Ok(output) => output,
Err(err) => return NpmRootCheck::NpmUnavailable(err),
};
let Some(npm_root) = output.lines().map(str::trim).find(|line| !line.is_empty()) else {
return NpmRootCheck::NpmUnavailable("empty output from npm root -g".to_string());
};
compare_npm_package_roots(&running_package_root, &PathBuf::from(npm_root))
}
fn compare_npm_package_roots(running_package_root: &Path, npm_root: &Path) -> NpmRootCheck {
let npm_package_root = npm_root.join("@openai").join("codex");
let running = normalize_path_for_compare(running_package_root);
let target = normalize_path_for_compare(&npm_package_root);
if running == target {
NpmRootCheck::Match {
package_root: npm_package_root,
}
} else {
NpmRootCheck::Mismatch {
running_package_root: running_package_root.to_path_buf(),
npm_package_root,
}
}
}
fn normalize_path_for_compare(path: &Path) -> String {
let canonical = path.canonicalize().unwrap_or_else(|_| path.to_path_buf());
let raw = canonical.to_string_lossy().replace('\\', "/");
if cfg!(windows) {
raw.to_ascii_lowercase()
} else {
raw
}
}
fn display_list<T: AsRef<str>>(items: &[T]) -> String {
if items.is_empty() {
"none".to_string()
@@ -1132,39 +1041,16 @@ fn display_list<T: AsRef<str>>(items: &[T]) -> String {
}
fn codex_path_entries() -> Vec<String> {
#[cfg(windows)]
let result = run_command("where", ["codex"]);
#[cfg(not(windows))]
let result = run_command("which", ["-a", "codex"]);
result
.unwrap_or_default()
.lines()
.map(str::trim)
.filter(|line| !line.is_empty())
.map(str::to_string)
let Ok(candidates) = which::which_all("codex") else {
return Vec::new();
};
let mut seen = BTreeSet::new();
candidates
.filter(|candidate| seen.insert(candidate.clone()))
.map(|path| path.display().to_string())
.collect()
}
fn run_command<I, S>(program: &str, args: I) -> Result<String, String>
where
I: IntoIterator<Item = S>,
S: AsRef<OsStr>,
{
let output = Command::new(program)
.args(args)
.output()
.map_err(|err| err.to_string())?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
if stderr.is_empty() {
return Err(format!("exited with status {}", output.status));
}
return Err(stderr);
}
Ok(String::from_utf8_lossy(&output.stdout).to_string())
}
fn config_check(config: &Config) -> DoctorCheck {
let mut details = Vec::new();
details.push(format!("CODEX_HOME: {}", config.codex_home.display()));
@@ -1748,7 +1634,7 @@ impl TerminalCheckInputs {
let terminal_size = crossterm::terminal::size().map_err(|err| err.to_string());
let info = terminal_info();
let tmux_details = if matches!(info.multiplexer, Some(Multiplexer::Tmux { .. })) {
tmux_diagnostic_details()
vec!["tmux options: not inspected (PATH helpers are not executed)".to_string()]
} else {
Vec::new()
};
@@ -2147,50 +2033,6 @@ fn terminal_size_issues(inputs: &TerminalCheckInputs) -> Vec<DoctorIssue> {
issues
}
fn tmux_diagnostic_details() -> Vec<String> {
let mut details = Vec::new();
push_tmux_display_detail(&mut details, "tmux client termtype", "#{client_termtype}");
push_tmux_display_detail(&mut details, "tmux client termname", "#{client_termname}");
for option in TMUX_OPTION_NAMES {
let value = tmux_option_value(option).unwrap_or_else(|| "unavailable".to_string());
details.push(format!("tmux {option}: {value}"));
}
details
}
fn push_tmux_display_detail(details: &mut Vec<String>, label: &str, format: &str) {
if let Some(value) = tmux_display_message(format) {
details.push(format!("{label}: {value}"));
}
}
fn tmux_option_value(option: &str) -> Option<String> {
let output = Command::new("tmux")
.args(["show-options", "-gqv", option])
.output()
.ok()?;
if !output.status.success() {
return None;
}
non_empty_trimmed(String::from_utf8(output.stdout).ok()?)
}
fn tmux_display_message(format: &str) -> Option<String> {
let output = Command::new("tmux")
.args(["display-message", "-p", format])
.output()
.ok()?;
if !output.status.success() {
return None;
}
non_empty_trimmed(String::from_utf8(output.stdout).ok()?)
}
fn non_empty_trimmed(value: String) -> Option<String> {
let value = value.trim().to_string();
if value.is_empty() { None } else { Some(value) }
}
async fn state_check(config: &Config, command: &DoctorCommand) -> DoctorCheck {
let mut details = Vec::new();
path_readiness(&mut details, "CODEX_HOME", &config.codex_home);
@@ -3250,31 +3092,6 @@ mod tests {
);
}
#[test]
fn compare_npm_package_roots_detects_match() {
let running = PathBuf::from("/prefix/lib/node_modules/@openai/codex");
let npm_root = PathBuf::from("/prefix/lib/node_modules");
assert_eq!(
compare_npm_package_roots(&running, &npm_root),
NpmRootCheck::Match {
package_root: npm_root.join("@openai").join("codex")
}
);
}
#[test]
fn compare_npm_package_roots_detects_mismatch() {
let running = PathBuf::from("/old/lib/node_modules/@openai/codex");
let npm_root = PathBuf::from("/new/lib/node_modules");
assert_eq!(
compare_npm_package_roots(&running, &npm_root),
NpmRootCheck::Mismatch {
running_package_root: running,
npm_package_root: npm_root.join("@openai").join("codex"),
}
);
}
#[test]
fn startup_warning_counts_group_known_sources() {
let warnings = vec![
@@ -4366,7 +4183,7 @@ mod tests {
}
#[test]
fn terminal_check_keeps_tmux_probe_failures_non_fatal() {
fn terminal_check_allows_missing_tmux_details() {
let mut inputs = terminal_inputs();
inputs.info.multiplexer = Some(Multiplexer::Tmux { version: None });
codex-rs/cli/src/doctor/git.rs
+29 -250
View File
@@ -1,62 +1,33 @@
//! Inspects Git executable locations and repository metadata without running Git.
use std::collections::BTreeSet;
use std::path::Path;
use std::path::PathBuf;
use std::process::Output;
use std::process::Stdio;
use std::time::Duration;
use codex_git_utils::get_git_repo_root;
use tokio::process::Command;
use tokio::time::timeout;
use super::CheckStatus;
use super::DoctorCheck;
use super::DoctorIssue;
const GIT_COMMAND_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 2);
#[derive(Clone, Debug, Default, Eq, PartialEq)]
struct GitCheckInputs {
selected_git: Option<PathBuf>,
git_candidates: Vec<PathBuf>,
git_version: Option<String>,
git_exec_path: Option<String>,
git_build_options: Option<String>,
repo_root: Option<PathBuf>,
git_entry: Option<String>,
branch: Option<String>,
core_fsmonitor: Option<String>,
}
pub(super) async fn git_check(cwd: &Path) -> DoctorCheck {
pub(super) fn git_check(cwd: &Path) -> DoctorCheck {
let selected_git = which::which("git").ok();
let git_candidates = git_candidates();
let repo_root = get_git_repo_root(cwd);
let (git_version, git_exec_path, git_build_options, branch, core_fsmonitor) =
if let Some(git_path) = selected_git.as_deref() {
let (version, exec_path, build_options, branch, fsmonitor) = tokio::join!(
git_output(git_path, cwd, &["--version"]),
git_output(git_path, cwd, &["--exec-path"]),
git_output(git_path, cwd, &["version", "--build-options"]),
git_output(git_path, cwd, &["rev-parse", "--abbrev-ref", "HEAD"]),
git_output(git_path, cwd, &["config", "--get", "core.fsmonitor"]),
);
(version, exec_path, build_options, branch, fsmonitor)
} else {
(None, None, None, None, None)
};
git_check_from_inputs(GitCheckInputs {
selected_git,
git_candidates,
git_version,
git_exec_path,
git_build_options,
git_entry: repo_root.as_deref().map(git_entry_summary),
repo_root,
branch,
core_fsmonitor,
})
}
@@ -70,17 +41,6 @@ fn git_check_from_inputs(inputs: GitCheckInputs) -> DoctorCheck {
for (index, path) in inputs.git_candidates.iter().enumerate() {
details.push(format!("PATH git #{}: {}", index + 1, path.display()));
}
push_optional_detail(&mut details, "git version", inputs.git_version.as_deref());
push_optional_detail(
&mut details,
"git exec path",
inputs.git_exec_path.as_deref(),
);
push_optional_detail(
&mut details,
"git build options",
inputs.git_build_options.as_deref(),
);
match inputs.repo_root.as_deref() {
Some(root) => {
details.push("repo detected: true".to_string());
@@ -88,43 +48,24 @@ fn git_check_from_inputs(inputs: GitCheckInputs) -> DoctorCheck {
}
None => details.push("repo detected: false".to_string()),
}
push_optional_detail(&mut details, ".git entry", inputs.git_entry.as_deref());
push_optional_detail(
&mut details,
"git branch",
normalized_branch(inputs.branch.as_deref()),
);
push_optional_detail(
&mut details,
"core.fsmonitor",
inputs
.core_fsmonitor
.as_deref()
.filter(|value| !value.is_empty()),
);
if let Some(entry) = inputs.git_entry.as_deref() {
details.push(format!(".git entry: {entry}"));
}
details.push("git execution: not inspected (PATH helpers are not executed)".to_string());
let mut check = DoctorCheck::new(
"git.environment",
"git",
CheckStatus::Ok,
git_summary(&inputs),
if inputs.selected_git.is_some() {
"git executable found; execution not verified"
} else {
"git executable not found"
},
)
.details(details);
if inputs.selected_git.is_some() && inputs.git_version.is_none() {
check.status = CheckStatus::Warning;
check.summary = "Git executable found but could not be run".to_string();
check = check.issue(
DoctorIssue::new(
CheckStatus::Warning,
"Git executable was found on PATH but did not return a version",
)
.expected("git --version succeeds")
.remedy("Fix the selected Git executable or PATH so Codex can inspect Git metadata.")
.field("git version")
.field("selected git"),
);
} else if inputs.selected_git.is_none() && inputs.repo_root.is_some() {
if inputs.selected_git.is_none() && inputs.repo_root.is_some() {
check.status = CheckStatus::Warning;
check.summary = "Git repository detected but git executable was not found".to_string();
check = check.issue(
@@ -136,50 +77,11 @@ fn git_check_from_inputs(inputs: GitCheckInputs) -> DoctorCheck {
.remedy("Install Git or fix PATH so Codex can inspect repository metadata.")
.field("selected git"),
);
} else if let Some(cause) =
old_windows_git_warning(inputs.git_version.as_deref(), cfg!(windows))
{
check.status = CheckStatus::Warning;
check.summary = cause.clone();
check = check.issue(
DoctorIssue::new(CheckStatus::Warning, cause)
.measured(inputs.git_version.unwrap_or_else(|| "unknown".to_string()))
.expected("current Git for Windows")
.remedy(
"Update Git for Windows or the bundled Git executable Codex resolves first.",
)
.field("git version")
.field("selected git"),
);
}
check
}
fn git_summary(inputs: &GitCheckInputs) -> String {
match inputs.git_version.as_deref() {
Some(version) => version.to_string(),
None if inputs.selected_git.is_some() => {
"git executable found; version unavailable".to_string()
}
None => "git executable not found".to_string(),
}
}
fn push_optional_detail(details: &mut Vec<String>, label: &str, value: Option<&str>) {
if let Some(value) = value {
details.push(format!("{label}: {value}"));
}
}
fn normalized_branch(branch: Option<&str>) -> Option<&str> {
match branch {
Some("HEAD") => Some("detached HEAD"),
Some(value) if !value.is_empty() => Some(value),
_ => None,
}
}
fn git_candidates() -> Vec<PathBuf> {
let Ok(candidates) = which::which_all("git") else {
return Vec::new();
@@ -190,40 +92,6 @@ fn git_candidates() -> Vec<PathBuf> {
.collect()
}
async fn git_output(git_path: &Path, cwd: &Path, args: &[&str]) -> Option<String> {
let mut command = Command::new(git_path);
command
.env("GIT_OPTIONAL_LOCKS", "0")
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
.args(args)
.current_dir(cwd)
.stdin(Stdio::null())
.kill_on_drop(true);
let output = timeout(GIT_COMMAND_TIMEOUT, command.output())
.await
.ok()?
.ok()?;
command_output_text(output)
}
fn command_output_text(output: Output) -> Option<String> {
if !output.status.success() {
return None;
}
let stdout = String::from_utf8_lossy(&output.stdout);
let normalized = stdout
.lines()
.map(str::trim)
.filter(|line| !line.is_empty())
.collect::<Vec<_>>()
.join("; ");
if normalized.is_empty() {
None
} else {
Some(normalized)
}
}
fn git_entry_summary(repo_root: &Path) -> String {
let entry = repo_root.join(".git");
match std::fs::metadata(&entry) {
@@ -243,96 +111,12 @@ fn git_entry_summary(repo_root: &Path) -> String {
}
}
fn old_windows_git_warning(version: Option<&str>, is_windows: bool) -> Option<String> {
if !is_windows {
return None;
}
let version = version?;
if version.to_ascii_lowercase().contains("msysgit") {
return Some("old msysgit installation may corrupt Windows TUI rendering".to_string());
}
let parsed = parse_git_version(version)?;
if parsed.major < 2 || (parsed.major == 2 && parsed.minor <= 34) {
return Some("old Git for Windows may corrupt Windows TUI rendering".to_string());
}
None
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
struct ParsedGitVersion {
major: u32,
minor: u32,
patch: u32,
}
fn parse_git_version(version: &str) -> Option<ParsedGitVersion> {
let version = version.strip_prefix("git version ")?;
let numeric = version
.split_whitespace()
.next()?
.split(".windows.")
.next()
.unwrap_or(version);
let mut parts = numeric.split('.');
Some(ParsedGitVersion {
major: parts.next()?.parse().ok()?,
minor: parts.next()?.parse().ok()?,
patch: parts.next().unwrap_or("0").parse().ok()?,
})
}
#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;
use super::*;
#[test]
fn parses_git_for_windows_version() {
assert_eq!(
parse_git_version("git version 2.34.1.windows.1"),
Some(ParsedGitVersion {
major: 2,
minor: 34,
patch: 1,
})
);
assert_eq!(
parse_git_version("git version 2.54.0.windows.1"),
Some(ParsedGitVersion {
major: 2,
minor: 54,
patch: 0,
})
);
}
#[test]
fn classifies_old_windows_git() {
assert_eq!(
old_windows_git_warning(
Some("git version 2.34.1.windows.1"),
/*is_windows*/ true
)
.as_deref(),
Some("old Git for Windows may corrupt Windows TUI rendering")
);
assert_eq!(
old_windows_git_warning(
Some("git version 2.54.0.windows.1"),
/*is_windows*/ true
),
None
);
assert_eq!(
old_windows_git_warning(
Some("git version 2.34.1.windows.1"),
/*is_windows*/ false
),
None
);
}
#[test]
fn warns_when_git_repo_has_no_git_executable() {
let check = git_check_from_inputs(GitCheckInputs {
@@ -347,35 +131,30 @@ mod tests {
);
}
#[test]
fn warns_when_selected_git_cannot_report_version() {
let check = git_check_from_inputs(GitCheckInputs {
selected_git: Some(PathBuf::from("/usr/bin/git")),
repo_root: Some(PathBuf::from("/repo")),
..GitCheckInputs::default()
});
assert_eq!(check.status, CheckStatus::Warning);
assert_eq!(check.summary, "Git executable found but could not be run");
}
#[test]
fn reports_git_candidates_and_repo_metadata() {
let check = git_check_from_inputs(GitCheckInputs {
selected_git: Some(PathBuf::from("/usr/bin/git")),
git_candidates: vec![PathBuf::from("/usr/bin/git"), PathBuf::from("/opt/bin/git")],
git_version: Some("git version 2.54.0".to_string()),
git_exec_path: Some("/usr/libexec/git-core".to_string()),
repo_root: Some(PathBuf::from("/repo")),
git_entry: Some("directory".to_string()),
branch: Some("main".to_string()),
core_fsmonitor: Some("false".to_string()),
..GitCheckInputs::default()
});
assert_eq!(check.status, CheckStatus::Ok);
assert!(check.details.contains(&"PATH git entries: 2".to_string()));
assert!(check.details.contains(&"git branch: main".to_string()));
assert!(check.details.contains(&"core.fsmonitor: false".to_string()));
let report = super::super::DoctorReport {
schema_version: 1,
generated_at: "2026-01-01T00:00:00Z".to_string(),
overall_status: check.status,
codex_version: "test".to_string(),
checks: vec![check],
};
insta::assert_snapshot!(super::super::render_human_report(
&report,
super::super::HumanOutputOptions {
show_details: true,
show_all: false,
ascii: true,
color_enabled: false,
},
));
}
}
codex-rs/cli/src/doctor/output.rs
-7
View File
@@ -642,7 +642,6 @@ fn display_summary(check: &DoctorCheck, options: HumanOutputOptions) -> String {
"runtime" => runtime_summary(check),
"install" if check.status == CheckStatus::Ok => "consistent".to_string(),
"search" => search_summary(check),
"git" => git_summary(check),
"terminal" => terminal_summary(check),
"title" => title_summary(check, options),
"state" => state_summary(check),
@@ -681,12 +680,6 @@ fn search_summary(check: &DoctorCheck) -> String {
}
}
fn git_summary(check: &DoctorCheck) -> String {
detail::detail_value(check, "git version")
.or_else(|| detail::detail_value(check, "selected git"))
.unwrap_or_else(|| check.summary.clone())
}
fn terminal_summary(check: &DoctorCheck) -> String {
let mut parts = Vec::new();
if let Some(terminal) = detail::detail_value(check, "terminal") {
codex-rs/cli/src/doctor/runtime.rs
+6 -18
View File
@@ -6,7 +6,6 @@
//! package files or from PATH.
use std::env;
use std::process::Command;
use codex_install_context::InstallContext;
use codex_install_context::InstallMethod;
@@ -48,7 +47,7 @@ pub(super) fn runtime_check() -> DoctorCheck {
.details(details)
}
/// Verifies that the search command selected by the install context is usable.
/// Inspects the search command selected by the install context without executing it.
///
/// Package-layout installs should point at a bundled ripgrep binary, while local
/// installs without that layout usually resolve rg from PATH. A warning here
@@ -80,23 +79,12 @@ pub(super) fn search_check() -> DoctorCheck {
}
}
} else {
match Command::new(&rg_command).arg("--version").output() {
Ok(output) if output.status.success() => {
let version = String::from_utf8_lossy(&output.stdout)
.lines()
.next()
.unwrap_or("rg version unknown")
.to_string();
details.push(format!("search command readiness: {version}"));
match which::which(&rg_command) {
Ok(path) => {
details.push(format!("search command path: {}", path.display()));
details.push("search command readiness: found; execution not verified".to_string());
CheckStatus::Ok
}
Ok(output) => {
details.push(format!(
"search command readiness: exited with status {}",
output.status
));
CheckStatus::Warning
}
Err(err) => {
details.push(format!("search command readiness: {err}"));
CheckStatus::Warning
@@ -105,7 +93,7 @@ pub(super) fn search_check() -> DoctorCheck {
};
let summary = match status {
CheckStatus::Ok => format!("search is OK ({provider})"),
CheckStatus::Ok => format!("search command found ({provider}); execution not verified"),
CheckStatus::Warning => "search command could not be verified".to_string(),
CheckStatus::Fail => unreachable!(),
};
codex-rs/cli/src/doctor/snapshots/codex__doctor__git__tests__reports_git_candidates_and_repo_metadata.snap
+22
View File
@@ -0,0 +1,22 @@
---
source: cli/src/doctor/git.rs
assertion_line: 150
expression: "super::super::render_human_report(&report, super::super::HumanOutputOptions\n{ show_details: true, show_all: false, ascii: true, color_enabled: false, },)"
---
Codex Doctor vtest
Environment
[ok] git git executable found; execution not verified
selected git /usr/bin/git
repo detected true
repo root /repo
.git entry directory
PATH entries (2) /usr/bin/git
/opt/bin/git
git execution not inspected (PATH helpers are not executed)
-------------------------------------------------------------
1 ok | 0 warn | 0 fail ok
--summary compact output --all expand truncated lists
--json redacted report
codex-rs/cli/src/doctor/updates.rs
+121 -65
View File
@@ -1,25 +1,19 @@
//! Diagnoses whether Codex update paths target the running installation.
//!
//! Update diagnostics combine cached version metadata, install-channel hints,
//! and bounded latest-version probes. For npm-managed launches, this module also
//! verifies that npm install -g would update the package root that launched the
//! current process, which catches PATH and prefix mismatches before the user runs
//! an update command.
//! and bounded latest-version HTTP probes. It never executes package managers or
//! other helpers selected by PATH; npm update targets are not verified.
use std::path::Path;
#[cfg(target_os = "macos")]
use std::path::PathBuf;
#[cfg(any(target_os = "macos", target_os = "windows"))]
use std::time::Duration;
use codex_core::config::Config;
#[cfg(any(target_os = "macos", target_os = "windows"))]
use codex_http_client::ClientRouteClass;
#[cfg(any(target_os = "macos", target_os = "windows"))]
use codex_http_client::RouteAwareClientPool;
use codex_install_context::InstallContext;
use codex_install_context::InstallMethod;
#[cfg(any(target_os = "macos", target_os = "windows"))]
use http::Method;
use serde::Deserialize;
#[cfg(target_os = "macos")]
@@ -29,15 +23,14 @@ use super::CheckStatus;
use super::DoctorCheck;
#[cfg(any(target_os = "macos", target_os = "windows"))]
use super::DoctorIssue;
use super::NpmRootCheck;
#[cfg(any(target_os = "macos", target_os = "windows"))]
use super::desktop::platform::InstalledApp;
use super::doctor_install_context;
use super::doctor_managed_by_npm;
#[cfg(any(target_os = "macos", target_os = "windows"))]
use super::network;
use super::npm_global_root_check;
use super::run_command;
const MAX_VERSION_RESPONSE_BYTES: usize = 1024 * 1024;
const VERSION_FILE_NAME: &str = "version.json";
const GITHUB_LATEST_RELEASE_URL: &str = "https://api.github.com/repos/openai/codex/releases/latest";
@@ -57,7 +50,7 @@ const DESKTOP_UPDATE_URL: &str =
/// Network failures while fetching latest-version metadata degrade the row to a
/// warning instead of failing doctor outright; update freshness is useful
/// support context but should not mask more direct install/config failures.
pub(super) fn updates_check(config: &Config) -> DoctorCheck {
pub(super) async fn updates_check(config: &Config) -> DoctorCheck {
let current_exe = std::env::current_exe().ok();
let install_context = doctor_install_context(current_exe.as_deref());
let mut details = vec![
@@ -71,48 +64,18 @@ pub(super) fn updates_check(config: &Config) -> DoctorCheck {
push_cached_version_details(&mut details, &version_file);
let mut status = CheckStatus::Ok;
let mut summary = "update configuration is locally consistent".to_string();
let mut remediation = None;
let summary = "update configuration is locally consistent".to_string();
if doctor_managed_by_npm(current_exe.as_deref()) {
match npm_global_root_check() {
NpmRootCheck::Match { package_root } => {
details.push(format!("npm update target: {}", package_root.display()));
}
NpmRootCheck::Mismatch {
running_package_root,
npm_package_root,
} => {
status = CheckStatus::Fail;
summary = "update would target a different npm install".to_string();
details.push(format!(
"running package root: {}",
running_package_root.display()
));
details.push(format!("npm package root: {}", npm_package_root.display()));
remediation = Some(format!(
"Fix PATH or npm prefix so the running package root ({}) matches the npm global package root ({}).",
running_package_root.display(),
npm_package_root.display()
));
}
NpmRootCheck::MissingPackageRoot => {
status = status.max(CheckStatus::Warning);
summary = "npm update target could not be proven".to_string();
remediation = Some(
"Reinstall or update Codex so the JS shim provides CODEX_MANAGED_PACKAGE_ROOT."
.to_string(),
);
}
NpmRootCheck::NpmUnavailable(error) => {
status = status.max(CheckStatus::Warning);
summary = "npm update target could not be inspected".to_string();
details.push(format!("npm root -g failed: {error}"));
}
}
details
.push("npm update target: not inspected (PATH helpers are not executed)".to_string());
}
let client = RouteAwareClientPool::new_without_request_logging(
config.http_client_factory(),
ClientRouteClass::Other,
);
match fetch_latest_version(&install_context) {
match fetch_latest_version(&client, &install_context).await {
Ok(latest_version) => {
details.push(format!("latest version: {latest_version}"));
if is_newer(&latest_version, env!("CARGO_PKG_VERSION")) == Some(true) {
@@ -127,11 +90,7 @@ pub(super) fn updates_check(config: &Config) -> DoctorCheck {
}
}
let mut check = DoctorCheck::new("updates.status", "updates", status, summary).details(details);
if let Some(remediation) = remediation {
check = check.remediation(remediation);
}
check
DoctorCheck::new("updates.status", "updates", status, summary).details(details)
}
#[cfg(any(target_os = "macos", target_os = "windows"))]
@@ -440,46 +399,75 @@ fn update_action_label(context: &InstallContext) -> &'static str {
}
}
fn fetch_latest_version(context: &InstallContext) -> Result<String, String> {
async fn fetch_latest_version(
client: &RouteAwareClientPool,
context: &InstallContext,
) -> Result<String, String> {
match &context.method {
InstallMethod::Brew => fetch_homebrew_cask_version(),
InstallMethod::Brew => fetch_homebrew_cask_version(client).await,
InstallMethod::Npm
| InstallMethod::Bun
| InstallMethod::VitePlus
| InstallMethod::Pnpm
| InstallMethod::Standalone { .. }
| InstallMethod::Other => fetch_latest_github_release_version(),
| InstallMethod::Other => fetch_latest_github_release_version(client).await,
}
}
fn fetch_latest_github_release_version() -> Result<String, String> {
async fn fetch_latest_github_release_version(
client: &RouteAwareClientPool,
) -> Result<String, String> {
#[derive(Deserialize)]
struct ReleaseInfo {
tag_name: String,
}
let info = http_get_json::<ReleaseInfo>(GITHUB_LATEST_RELEASE_URL)?;
let info = http_get_json::<ReleaseInfo>(client, GITHUB_LATEST_RELEASE_URL).await?;
info.tag_name
.strip_prefix("rust-v")
.map(str::to_string)
.ok_or_else(|| format!("failed to parse latest tag {}", info.tag_name))
}
fn fetch_homebrew_cask_version() -> Result<String, String> {
async fn fetch_homebrew_cask_version(client: &RouteAwareClientPool) -> Result<String, String> {
#[derive(Deserialize)]
struct HomebrewCaskInfo {
version: String,
}
http_get_json::<HomebrewCaskInfo>(HOMEBREW_CASK_API_URL).map(|info| info.version)
http_get_json::<HomebrewCaskInfo>(client, HOMEBREW_CASK_API_URL)
.await
.map(|info| info.version)
}
fn http_get_json<T>(url: &str) -> Result<T, String>
async fn http_get_json<T>(client: &RouteAwareClientPool, url: &str) -> Result<T, String>
where
T: for<'de> Deserialize<'de>,
{
let body = run_command("curl", ["-fsSL", "--max-time", "5", url])?;
serde_json::from_str::<T>(&body).map_err(|err| err.to_string())
tokio::time::timeout(Duration::from_secs(/*secs*/ 5), async {
let mut response = client
.request(Method::GET, url)
.header(
http::header::USER_AGENT,
concat!("codex-doctor/", env!("CARGO_PKG_VERSION")),
)
.send()
.await
.map_err(|err| err.to_string())?;
if !response.status().is_success() {
return Err(format!("HTTP {}", response.status()));
}
let mut body = Vec::new();
while let Some(chunk) = response.chunk().await.map_err(|err| err.to_string())? {
if chunk.len() > MAX_VERSION_RESPONSE_BYTES.saturating_sub(body.len()) {
return Err("version response exceeds size limit".to_string());
}
body.extend_from_slice(&chunk);
}
serde_json::from_slice(&body).map_err(|err| err.to_string())
})
.await
.map_err(|_| "version request timed out".to_string())?
}
fn is_newer(latest: &str, current: &str) -> Option<bool> {
@@ -509,6 +497,74 @@ struct VersionInfo {
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[tokio::test]
async fn version_http_probe_decodes_json_and_rejects_invalid_responses() {
use codex_http_client::HttpClientFactory;
use codex_http_client::OutboundProxyPolicy;
use wiremock::Mock;
use wiremock::MockServer;
use wiremock::ResponseTemplate;
use wiremock::matchers::method;
use wiremock::matchers::path;
let server = MockServer::start().await;
let client = RouteAwareClientPool::new_without_request_logging(
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
ClientRouteClass::Other,
);
for (endpoint, response) in [
(
"valid",
ResponseTemplate::new(/*s*/ 200)
.set_body_json(serde_json::json!({"version": "1.2.3"})),
),
(
"invalid",
ResponseTemplate::new(/*s*/ 200).set_body_string("not JSON"),
),
("unavailable", ResponseTemplate::new(/*s*/ 503)),
("proxy_auth_required", ResponseTemplate::new(/*s*/ 407)),
(
"redirect",
ResponseTemplate::new(/*s*/ 302)
.insert_header("Location", format!("{}/valid", server.uri())),
),
(
"timeout",
ResponseTemplate::new(/*s*/ 200).set_delay(Duration::from_secs(/*secs*/ 6)),
),
(
"oversized",
ResponseTemplate::new(/*s*/ 200)
.set_body_bytes(vec![b' '; MAX_VERSION_RESPONSE_BYTES + 1]),
),
] {
Mock::given(method("GET"))
.and(path(format!("/{endpoint}")))
.respond_with(response)
.mount(&server)
.await;
let result = http_get_json::<serde_json::Value>(
&client,
&format!("{}/{endpoint}", server.uri()),
)
.await;
if matches!(endpoint, "valid" | "redirect") {
assert_eq!(result, Ok(serde_json::json!({"version": "1.2.3"})));
} else if endpoint == "timeout" {
assert_eq!(result, Err("version request timed out".to_string()));
} else if endpoint == "proxy_auth_required" {
assert_eq!(
result,
Err("HTTP 407 Proxy Authentication Required".to_string())
);
} else {
assert!(result.is_err(), "{endpoint} must not be accepted");
}
}
}
#[cfg(target_os = "macos")]
#[test]
codex-rs/cli/tests/doctor_path_safety.rs
+334
View File
@@ -0,0 +1,334 @@
//! Black-box coverage for diagnostics under a repository-controlled PATH.
use std::ffi::OsString;
use std::path::PathBuf;
use std::time::Duration;
use anyhow::Result;
use app_test_support::TestAppServer;
use codex_app_server_protocol::RequestId;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
use tempfile::TempDir;
use tokio::io::AsyncBufReadExt;
use tokio::io::AsyncWriteExt;
use tokio::io::BufReader;
use tokio::net::TcpListener;
use tokio::time::timeout;
struct Fixture {
root: TempDir,
program: PathBuf,
workspace: PathBuf,
home: PathBuf,
marker: PathBuf,
path: OsString,
}
impl Fixture {
fn new() -> Result<Self> {
let root = TempDir::new()?;
// Cargo-built paths deliberately ignore npm provenance. Launch outside
// target/ so this fixture also exercises the packaged-install checks.
let program = root
.path()
.join(format!("codex{}", std::env::consts::EXE_SUFFIX));
let source = codex_utils_cargo_bin::cargo_bin("codex")?;
if std::fs::hard_link(&source, &program).is_err() {
std::fs::copy(&source, &program)?;
}
let workspace = root.path().join("workspace");
let bin = workspace.join("node_modules/.bin");
let home = root.path().join("home");
let marker = root.path().join("helper-ran");
std::fs::create_dir_all(&bin)?;
std::fs::create_dir_all(workspace.join(".git"))?;
std::fs::create_dir(&home)?;
std::fs::write(
home.join("config.toml"),
r#"
cli_auth_credentials_store = "file"
check_for_update_on_startup = false
model_provider = "local"
[analytics]
enabled = false
[model_providers.local]
name = "local test"
base_url = "http://127.0.0.1:9/v1"
wire_api = "responses"
"#,
)?;
for name in [
"zellij", "tmux", "which", "where", "npm", "rg", "git", "curl", "cmd.exe",
] {
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let executable = bin.join(name);
std::fs::write(
&executable,
"#!/bin/sh\nprintf 'helper ran\\n' >> \"$CODEX_TEST_HELPER_MARKER\"\nexit 0\n",
)?;
std::fs::set_permissions(
executable,
std::fs::Permissions::from_mode(/*mode*/ 0o755),
)?;
}
#[cfg(windows)]
std::fs::write(
bin.join(format!("{name}.cmd")),
"@echo helper ran>>\"%CODEX_TEST_HELPER_MARKER%\"\r\n@exit /b 0\r\n",
)?;
}
// Windows selects rg.exe explicitly, so rg.cmd cannot satisfy discovery.
// An invalid image still counts as found: doctor must not execute it.
#[cfg(windows)]
std::fs::write(bin.join("rg.exe"), "not an executable image")?;
let path = std::env::join_paths(std::iter::once(bin).chain(std::env::split_paths(
&std::env::var_os("PATH").unwrap_or_default(),
)))?;
Ok(Self {
root,
program,
workspace,
home,
marker,
path,
})
}
fn command(&self) -> Result<assert_cmd::Command> {
let mut command = assert_cmd::Command::new(&self.program);
command
.current_dir(&self.workspace)
.env("CODEX_HOME", &self.home)
.env("HOME", self.root.path())
.env("PATH", &self.path)
.env("CODEX_TEST_HELPER_MARKER", &self.marker)
.env("CODEX_MANAGED_BY_NPM", "1")
.env(
"CODEX_MANAGED_PACKAGE_ROOT",
self.workspace.join("node_modules/@openai/codex"),
)
.env(
"CODEX_APP_SERVER_MANAGED_CONFIG_PATH",
self.home.join("managed_config.toml"),
)
.env("HTTPS_PROXY", "http://127.0.0.1:9")
.env("HTTP_PROXY", "http://127.0.0.1:9")
.env("ALL_PROXY", "http://127.0.0.1:9")
.env("https_proxy", "http://127.0.0.1:9")
.env("http_proxy", "http://127.0.0.1:9")
.env("all_proxy", "http://127.0.0.1:9")
.env("NO_PROXY", "127.0.0.1,localhost")
.env("no_proxy", "127.0.0.1,localhost")
.env_remove("TMUX")
.env_remove("TMUX_PANE")
.env_remove("ZELLIJ_VERSION")
.env_remove("ZELLIJ_SESSION_NAME")
.env_remove("TERM_PROGRAM")
.env("TERM", "dumb")
.env("ZELLIJ", "0")
.timeout(Duration::from_secs(/*secs*/ 45));
Ok(command)
}
}
#[test]
fn startup_and_doctor_do_not_execute_path_helpers() -> Result<()> {
let fixture = Fixture::new()?;
// Non-TTY dumb-terminal startup exits immediately after terminal detection.
fixture.command()?.assert().failure();
assert!(!fixture.marker.exists(), "startup executed a PATH helper");
for args in [
vec!["doctor", "--json"],
vec!["doctor", "--json", "--feedback"],
] {
let mut command = fixture.command()?;
command
.args(args)
.env("TMUX", "test-tmux")
.env("TERM_PROGRAM", "tmux");
let output = command.output()?;
let report: Value = serde_json::from_slice(&output.stdout)?;
assert_eq!(
report["checks"]["installation"]["details"]["managed by npm"],
"true"
);
assert_eq!(report["checks"]["runtime.search"]["status"], "ok");
assert_eq!(
report["checks"]["runtime.search"]["details"]["search command path"],
fixture
.workspace
.join("node_modules/.bin")
.join(format!("rg{}", std::env::consts::EXE_SUFFIX))
.display()
.to_string()
);
assert_eq!(
report["checks"]["git.environment"]["summary"],
"git executable found; execution not verified"
);
assert!(!fixture.marker.exists(), "doctor executed a PATH helper");
}
Ok(())
}
#[cfg(unix)]
#[tokio::test]
async fn interactive_tmux_startup_does_not_execute_workspace_helpers() -> Result<()> {
let fixture = Fixture::new()?;
let command = fixture.command()?;
let mut env: std::collections::HashMap<String, String> = std::env::vars().collect();
for (key, value) in command.get_envs() {
let key = key.to_string_lossy().into_owned();
if let Some(value) = value {
env.insert(key, value.to_string_lossy().into_owned());
} else {
env.remove(&key);
}
}
env.insert("TERM".to_string(), "xterm-256color".to_string());
env.insert("TERM_PROGRAM".to_string(), "tmux".to_string());
env.insert("TMUX".to_string(), "test-tmux".to_string());
env.insert(
"CODEX_TUI_DISABLE_KEYBOARD_ENHANCEMENT".to_string(),
"0".to_string(),
);
env.insert(
"GHOSTTY_RESOURCES_DIR".to_string(),
"/test/ghostty".to_string(),
);
let spawned = codex_utils_pty::spawn_pty_process(
fixture.program.to_str().unwrap(),
&[],
&fixture.workspace,
&env,
/*arg0*/ &None,
codex_utils_pty::TerminalSize {
rows: 40,
cols: 120,
},
&[],
)
.await?;
let session = spawned.session;
let mut output_rx = spawned.stdout_rx;
let writer = session.writer_sender();
let mut output = String::new();
let ansi = regex_lite::Regex::new(r"\x1b\[[0-?]*[ -/]*[@-~]")?;
let ready = timeout(Duration::from_secs(/*secs*/ 45), async {
while let Some(bytes) = output_rx.recv().await {
let chunk = String::from_utf8_lossy(&bytes);
output.push_str(&chunk);
if chunk.contains("\x1b[6n") {
writer.send(b"\x1b[1;1R".to_vec()).await?;
}
if chunk.contains("\x1b[c") {
writer.send(b"\x1b[?1;2c".to_vec()).await?;
}
// Ratatui can position over spaces instead of writing them.
let text: String = ansi
.replace_all(&output, "")
.chars()
.filter(|character| !character.is_whitespace())
.collect();
if text.contains("Doyoutrustthecontentsofthisdirectory?") {
return Ok::<_, anyhow::Error>(());
}
}
anyhow::bail!("TUI exited before trust prompt")
})
.await;
session.terminate();
let _ = timeout(Duration::from_secs(/*secs*/ 10), spawned.exit_rx).await?;
ready.map_err(|_| {
anyhow::anyhow!(
"trust prompt timed out: {}",
output.chars().take(/*n*/ 4000).collect::<String>()
)
})??;
assert!(
output.contains("\x1b[>5u"),
"expected safe Ghostty/tmux keyboard flags: {output}"
);
assert!(
!fixture.marker.exists(),
"interactive startup executed a workspace helper"
);
Ok(())
}
#[tokio::test]
async fn feedback_with_logs_does_not_execute_path_helpers() -> Result<()> {
let fixture = Fixture::new()?;
let proxy = TcpListener::bind("127.0.0.1:0").await?;
let proxy_uri = format!("http://{}", proxy.local_addr()?);
// Reject all outbound traffic locally, including the final feedback upload.
let proxy_task = tokio::spawn(async move {
while let Ok((stream, _)) = proxy.accept().await {
let mut stream = BufReader::new(stream);
let mut request = String::new();
if stream.read_line(&mut request).await.is_ok() {
let _ = stream.get_mut().write_all(b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\nConnection: close\r\n\r\n").await;
}
}
});
let path = fixture.path.to_string_lossy();
let marker = fixture.marker.to_string_lossy();
let home = fixture.root.path().to_string_lossy();
let package_root = fixture.workspace.join("node_modules/@openai/codex");
let package_root = package_root.to_string_lossy();
let mut app_server = TestAppServer::builder()
.with_program(&fixture.program)
.with_codex_home(&fixture.home)
// The CLI does not accept the standalone app-server's test-only flag.
// Disable plugins through real config; their safety test joins a full sync.
.with_plugin_startup_tasks()
.with_args(&["-c", "features.plugins=false", "app-server"])
.with_env_overrides(&[
("PATH", Some(path.as_ref())),
("HOME", Some(home.as_ref())),
("CODEX_TEST_HELPER_MARKER", Some(marker.as_ref())),
("CODEX_MANAGED_BY_NPM", Some("1")),
("CODEX_MANAGED_PACKAGE_ROOT", Some(package_root.as_ref())),
("ZELLIJ", Some("0")),
("ZELLIJ_VERSION", None),
("TMUX", None),
("TMUX_PANE", None),
("HTTP_PROXY", Some(&proxy_uri)),
("HTTPS_PROXY", Some(&proxy_uri)),
("ALL_PROXY", Some(&proxy_uri)),
("http_proxy", Some(&proxy_uri)),
("https_proxy", Some(&proxy_uri)),
("all_proxy", Some(&proxy_uri)),
("NO_PROXY", Some("127.0.0.1,localhost")),
("no_proxy", Some("127.0.0.1,localhost")),
])
.build_initialized_with_timeout(Duration::from_secs(/*secs*/ 30))
.await?;
let request_id = app_server
.send_raw_request(
"feedback/upload",
Some(json!({
"classification": "bug", "includeLogs": true
})),
)
.await?;
let error = timeout(
Duration::from_secs(/*secs*/ 45),
app_server.read_stream_until_error_message(RequestId::Integer(request_id)),
)
.await??;
assert!(error.error.message.contains("failed to upload feedback"));
assert!(!fixture.marker.exists(), "feedback executed a PATH helper");
timeout(
Duration::from_secs(/*secs*/ 10),
app_server.shutdown_gracefully(),
)
.await??;
proxy_task.abort();
Ok(())
}
codex-rs/core-plugins/Cargo.toml
-3
View File
@@ -57,9 +57,6 @@ url = { workspace = true }
uuid = { workspace = true, features = ["v4"] }
zip = { workspace = true }
[target.'cfg(target_os = "macos")'.dependencies]
which = { workspace = true }
[dev-dependencies]
codex-exec-server-test-support = { workspace = true }
libc = { workspace = true }
codex-rs/core-plugins/src/startup_sync.rs
+34 -42
View File
@@ -78,14 +78,12 @@ pub fn sync_openai_plugins_repo(
codex_home: &Path,
http_client_factory: HttpClientFactory,
) -> Result<String, String> {
// Keep Git-only egress working without trusting workspace PATH entries.
let git_binary = codex_utils_path::system_executable("git");
// Apple's /usr/bin/git is an installer shim when developer tools are absent.
// The resolver prefers the real CLT/Xcode executable when installed.
#[cfg(target_os = "macos")]
let git_binary = match which::which("git") {
Ok(git_path) => macos_git_binary_from_path(git_path, apple_developer_tools_available()),
Err(_) => None,
};
#[cfg(not(target_os = "macos"))]
let git_binary = Some(PathBuf::from("git"));
let git_binary = git_binary.filter(|path| path != Path::new("/usr/bin/git"));
sync_openai_plugins_repo_with_transport_overrides(
codex_home,
git_binary.as_deref(),
@@ -106,7 +104,7 @@ fn sync_openai_plugins_repo_with_transport_overrides(
let git_sync_result = match git_binary {
Some(git_binary) => sync_openai_plugins_repo_via_git(codex_home, git_binary),
None => Err("git executable is unavailable".to_string()),
None => Err("no Git executable found in trusted installation directories".to_string()),
};
match git_sync_result {
@@ -116,11 +114,13 @@ fn sync_openai_plugins_repo_with_transport_overrides(
Ok(remote_sha)
}
Err(err) => {
emit_curated_plugins_startup_sync_metric("git", "failure");
warn!(
error = %err,
"git sync failed for curated plugin sync; falling back to GitHub HTTP"
);
if git_binary.is_some() {
emit_curated_plugins_startup_sync_metric("git", "failure");
warn!(
error = %err,
"git sync failed for curated plugin sync; falling back to GitHub HTTP"
);
}
match sync_openai_plugins_repo_via_http(codex_home, api_base_url, http_client_factory) {
Ok(remote_sha) => {
emit_curated_plugins_startup_sync_metric("http", "success");
@@ -266,7 +266,7 @@ fn fetch_curated_plugins_commit_from(
context: &str,
) -> Result<(), String> {
let fetch_refspec = format!("+{source_revision}:{CURATED_PLUGINS_FETCH_REF}");
let mut command = git_command(git_binary);
let mut command = git_command(git_binary)?;
command
.arg("-C")
.arg(repo_path)
@@ -298,7 +298,7 @@ fn run_git_in_repo(
args: &[&str],
context: &str,
) -> Result<(), String> {
let mut command = git_command(git_binary);
let mut command = git_command(git_binary)?;
command.arg("-C").arg(repo_path).args(args);
let output = run_git_command_with_timeout(&mut command, context, CURATED_PLUGINS_GIT_TIMEOUT)?;
ensure_git_success(&output, context)
@@ -608,9 +608,10 @@ fn read_local_git_or_sha_file(
}
fn git_ls_remote_head_sha(codex_home: &Path, git_binary: &Path) -> Result<String, String> {
let mut command = git_command(git_binary);
let mut command = git_command(git_binary)?;
let _trusted_repository = crate::configure_trusted_git_repository(&mut command, codex_home)?;
command
.current_dir(codex_home)
.arg("ls-remote")
.arg(OPENAI_PLUGINS_GIT_URL)
.arg("HEAD");
@@ -637,7 +638,7 @@ fn git_ls_remote_head_sha(codex_home: &Path, git_binary: &Path) -> Result<String
}
fn git_head_sha(repo_path: &Path, git_binary: &Path) -> Result<String, String> {
let output = git_command(git_binary)
let output = git_command(git_binary)?
.arg("-C")
.arg(repo_path)
.arg("rev-parse")
@@ -661,31 +662,22 @@ fn git_head_sha(repo_path: &Path, git_binary: &Path) -> Result<String, String> {
Ok(sha)
}
fn git_command(git_binary: &Path) -> Command {
crate::PluginGitMode::Automatic.command(git_binary)
}
#[cfg(any(target_os = "macos", test))]
fn macos_git_binary_from_path(
git_path: PathBuf,
apple_developer_tools_available: bool,
) -> Option<PathBuf> {
if git_path == Path::new("/usr/bin/git") && !apple_developer_tools_available {
None
} else {
Some(git_path)
}
}
#[cfg(target_os = "macos")]
fn apple_developer_tools_available() -> bool {
Command::new("/usr/bin/xcode-select")
.arg("-p")
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
.is_ok_and(|status| status.success())
fn git_command(git_binary: &Path) -> Result<Command, String> {
let mut command = crate::PluginGitMode::Automatic.command(git_binary);
// Git launches transports and credential helpers too. Selecting a trusted
// main executable alone does not prevent a workspace PATH helper from running.
command
.env(
"PATH",
codex_utils_path::system_path()
.map_err(|err| format!("failed to construct trusted Git PATH: {err}"))?,
)
.env_remove("GIT_EXEC_PATH")
.env_remove("GIT_TEMPLATE_DIR")
.env_remove("DEVELOPER_DIR")
.env("GIT_TERMINAL_PROMPT", "0")
.env("NoDefaultCurrentDirectoryInExePath", "1");
Ok(command)
}
fn run_git_command_with_timeout(
codex-rs/core-plugins/src/startup_sync_tests.rs
+103 -26
View File
@@ -21,6 +21,106 @@ use zip::write::SimpleFileOptions;
const TEST_CURATED_PLUGIN_SHA: &str = "0123456789abcdef0123456789abcdef01234567";
#[cfg(unix)]
#[test]
fn pretrust_startup_sync_uses_installed_git_with_hostile_path() {
const CHILD_HOME: &str = "CODEX_TEST_CURATED_SYNC_HOME";
if let Some(home) = std::env::var_os(CHILD_HOME) {
// Call the public entry point, synchronously. The parent joins this
// process before checking the marker, so no detached work can escape it.
sync_openai_plugins_repo(
Path::new(&home),
crate::test_support::test_http_client_factory(),
)
.expect("Git-only startup sync should succeed");
return;
}
let fixture = tempdir().expect("fixture");
let source = fixture.path().join("remote");
let home = fixture.path().join("home");
let workspace = fixture.path().join("workspace");
let bin = workspace.join("node_modules/.bin");
let marker = fixture.path().join("helper-ran");
std::fs::create_dir_all(&source).unwrap();
std::fs::create_dir_all(&bin).unwrap();
std::fs::create_dir(&home).unwrap();
run_git(&source, &["init", "--quiet"]);
for name in ["git", "git-upload-pack", "git-remote-https"] {
write_executable_script(
&bin.join(name),
&format!("#!/bin/sh\nprintf ran > '{}'\nexit 1\n", marker.display()),
);
}
let config = fixture.path().join("gitconfig");
std::fs::write(
&config,
format!(
"[url \"file://{}\"]\n insteadOf = {OPENAI_PLUGINS_GIT_URL}\n",
source.display()
),
)
.unwrap();
for plugins in [vec!["gmail"], vec!["gmail", "linear"]] {
write_openai_curated_marketplace(&source, &plugins);
run_git(&source, &["add", "."]);
run_git(
&source,
&[
"-c",
"user.name=Test",
"-c",
"user.email=test@example.com",
"commit",
"-qm",
"update",
],
);
let expected_sha = run_git(&source, &["rev-parse", "HEAD"]);
let output = Command::new(std::env::current_exe().unwrap())
.args([
"--exact",
"startup_sync::tests::pretrust_startup_sync_uses_installed_git_with_hostile_path",
"--nocapture",
])
.current_dir(&workspace)
.env(CHILD_HOME, &home)
.env("PATH", &bin)
.env("GIT_EXEC_PATH", &bin)
.env("GIT_CONFIG_GLOBAL", &config)
.env("GIT_CONFIG_SYSTEM", "/dev/null")
.env("HTTP_PROXY", "http://127.0.0.1:9")
.env("HTTPS_PROXY", "http://127.0.0.1:9")
.env("http_proxy", "http://127.0.0.1:9")
.env("https_proxy", "http://127.0.0.1:9")
.output()
.expect("join sync subprocess");
assert!(
output.status.success(),
"{}\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert_eq!(
read_curated_plugins_sha(&home),
Some(
String::from_utf8(expected_sha.stdout)
.unwrap()
.trim()
.to_string()
)
);
assert!(!marker.exists(), "sync executed a workspace helper");
for plugin in plugins {
assert!(
curated_plugins_repo_path(&home)
.join(format!("plugins/{plugin}/.codex-plugin/plugin.json"))
.is_file()
);
}
}
}
#[tokio::test]
async fn github_http_routes_repository_ref_and_zipball_urls() {
let server = MockServer::start().await;
@@ -93,7 +193,7 @@ async fn backup_archive_routes_metadata_and_backend_supplied_download_urls() {
#[test]
fn git_command_sanitizes_ambient_repository_environment() {
let command = git_command(Path::new("git"));
let command = git_command(Path::new("git")).expect("trusted Git command");
assert_eq!(
command.get_args().collect::<Vec<_>>(),
@@ -260,6 +360,7 @@ async fn ordinary_clone_rejects_tracked_embedded_bare_repository() {
std::fs::remove_file(&marker).expect("remove unguarded execution marker");
let guarded = git_command(Path::new("git"))
.expect("trusted Git command")
.args(["status", "--short"])
.current_dir(&nested)
.output()
@@ -289,6 +390,7 @@ async fn ordinary_clone_rejects_tracked_embedded_bare_repository() {
);
let explicit_git_dir = git_command(Path::new("git"))
.expect("trusted Git command")
.arg("--git-dir")
.arg(&nested)
.args(["rev-parse", "--git-dir"])
@@ -936,31 +1038,6 @@ async fn sync_openai_plugins_repo_falls_back_to_http_when_git_is_unavailable() {
assert_eq!(read_curated_plugins_sha(tmp.path()).as_deref(), Some(sha));
}
#[test]
fn apple_git_without_developer_tools_is_unavailable() {
assert_eq!(
macos_git_binary_from_path(
PathBuf::from("/usr/bin/git"),
/*apple_developer_tools_available*/ false,
),
None
);
assert_eq!(
macos_git_binary_from_path(
PathBuf::from("/usr/bin/git"),
/*apple_developer_tools_available*/ true,
),
Some(PathBuf::from("/usr/bin/git"))
);
assert_eq!(
macos_git_binary_from_path(
PathBuf::from("/opt/homebrew/bin/git"),
/*apple_developer_tools_available*/ false,
),
Some(PathBuf::from("/opt/homebrew/bin/git"))
);
}
#[tokio::test]
async fn sync_openai_plugins_repo_uses_http_without_git_transport() {
let tmp = tempdir().expect("tempdir");
codex-rs/terminal-detection/src/lib.rs
+12 -141
View File
@@ -1,7 +1,8 @@
//! Terminal detection utilities.
//!
//! This module feeds terminal metadata into OpenTelemetry user-agent logging and into
//! terminal-specific configuration choices in the TUI.
//! terminal-specific configuration choices in the TUI. Detection only reads the
//! environment; it must not execute helpers selected by an untrusted PATH.
use std::sync::OnceLock;
@@ -70,22 +71,6 @@ pub enum Multiplexer {
},
}
/// tmux client terminal identification captured via `tmux display-message`.
///
/// `termtype` corresponds to `#{client_termtype}` and typically reflects the
/// underlying terminal program (for example, `ghostty` or `wezterm`) with an
/// optional version suffix. `termname` comes from `#{client_termname}` and
/// preserves the TERM capability string exposed by the client (for example,
/// `xterm-256color`).
///
/// This information is only available when running under tmux and lets us
/// attribute the session to the underlying terminal rather than to tmux itself.
#[derive(Clone, Debug, Default, Eq, PartialEq)]
struct TmuxClientInfo {
termtype: Option<String>,
termname: Option<String>,
}
impl TerminalInfo {
/// Creates terminal metadata from detected fields.
fn new(
@@ -120,17 +105,6 @@ impl TerminalInfo {
)
}
/// Creates terminal metadata from a `TERM_PROGRAM` match plus a `TERM` value.
fn from_term_program_and_term(
name: TerminalName,
term_program: String,
version: Option<String>,
term: Option<String>,
multiplexer: Option<Multiplexer>,
) -> Self {
Self::new(name, Some(term_program), version, term, multiplexer)
}
/// Creates terminal metadata from a known terminal name and optional version.
fn from_name(
name: TerminalName,
@@ -238,14 +212,6 @@ trait Environment {
fn has_non_empty(&self, name: &str) -> bool {
self.var_non_empty(name).is_some()
}
/// Returns tmux client details when available.
fn tmux_client_info(&self) -> TmuxClientInfo;
/// Returns Zellij version details when available.
fn zellij_version(&self) -> Option<String> {
self.var_non_empty("ZELLIJ_VERSION")
}
}
/// Reads environment variables from the running process.
@@ -262,15 +228,6 @@ impl Environment for ProcessEnvironment {
}
}
}
fn tmux_client_info(&self) -> TmuxClientInfo {
tmux_client_info()
}
fn zellij_version(&self) -> Option<String> {
self.var_non_empty("ZELLIJ_VERSION")
.or_else(zellij_version_from_command)
}
}
/// Returns a sanitized terminal identifier for User-Agent strings.
@@ -288,34 +245,26 @@ pub fn terminal_info() -> TerminalInfo {
/// Detects structured terminal metadata from an injectable environment.
///
/// Detection order favors explicit identifiers before falling back to capability strings:
/// - If `TERM_PROGRAM=tmux`, the tmux client term type/name are used instead. The client term
/// type is split on whitespace to extract a program name plus optional version (for example,
/// `ghostty 1.2.3`), while the client term name becomes the `TERM` capability string.
/// - Otherwise, `TERM_PROGRAM` (plus `TERM_PROGRAM_VERSION`) drives the detected terminal name.
/// - `TERM_PROGRAM` (plus `TERM_PROGRAM_VERSION`) drives the detected terminal name,
/// except when it identifies tmux rather than the underlying terminal.
/// This means `TERM_PROGRAM` can mask later probes (for example `WT_SESSION`).
/// - Next, terminal-specific variables (WEZTERM, iTerm2, Apple Terminal, kitty, etc.) are checked.
/// - Finally, `TERM` is used as the capability fallback.
///
/// tmux client term info is only consulted when a tmux multiplexer is detected, and it is
/// derived from `tmux display-message` to surface the underlying terminal program instead of
/// reporting tmux itself.
fn detect_terminal_info_from_env(env: &dyn Environment) -> TerminalInfo {
let multiplexer = detect_multiplexer(env);
if let Some(term_program) = env.var_non_empty("TERM_PROGRAM") {
if is_tmux_term_program(&term_program)
&& matches!(multiplexer, Some(Multiplexer::Tmux { .. }))
&& let Some(terminal) =
terminal_from_tmux_client_info(env.tmux_client_info(), multiplexer.clone())
{
return terminal;
}
if let Some(term_program) = env.var_non_empty("TERM_PROGRAM")
&& !is_tmux_term_program(&term_program)
{
let version = env.var_non_empty("TERM_PROGRAM_VERSION");
let name = terminal_name_from_term_program(&term_program).unwrap_or(TerminalName::Unknown);
return TerminalInfo::from_term_program(name, term_program, version, multiplexer);
}
if env.has_non_empty("GHOSTTY_RESOURCES_DIR") {
return TerminalInfo::from_name(TerminalName::Ghostty, /*version*/ None, multiplexer);
}
if env.has("WEZTERM_VERSION") {
let version = env.var_non_empty("WEZTERM_VERSION");
return TerminalInfo::from_name(TerminalName::WezTerm, version, multiplexer);
@@ -400,7 +349,7 @@ fn detect_multiplexer(env: &dyn Environment) -> Option<Multiplexer> {
|| env.has_non_empty("ZELLIJ_VERSION")
{
return Some(Multiplexer::Zellij {
version: env.zellij_version(),
version: env.var_non_empty("ZELLIJ_VERSION"),
});
}
@@ -411,30 +360,6 @@ fn is_tmux_term_program(value: &str) -> bool {
value.eq_ignore_ascii_case("tmux")
}
fn terminal_from_tmux_client_info(
client_info: TmuxClientInfo,
multiplexer: Option<Multiplexer>,
) -> Option<TerminalInfo> {
let termtype = client_info.termtype.and_then(none_if_whitespace);
let termname = client_info.termname.and_then(none_if_whitespace);
if let Some(termtype) = termtype.as_ref() {
let (program, version) = split_term_program_and_version(termtype);
let name = terminal_name_from_term_program(&program).unwrap_or(TerminalName::Unknown);
return Some(TerminalInfo::from_term_program_and_term(
name,
program,
version,
termname,
multiplexer,
));
}
termname
.as_ref()
.map(|termname| TerminalInfo::from_term(termname.to_string(), multiplexer))
}
fn tmux_version_from_env(env: &dyn Environment) -> Option<String> {
let term_program = env.var("TERM_PROGRAM")?;
if !is_tmux_term_program(&term_program) {
@@ -444,60 +369,6 @@ fn tmux_version_from_env(env: &dyn Environment) -> Option<String> {
env.var_non_empty("TERM_PROGRAM_VERSION")
}
fn split_term_program_and_version(value: &str) -> (String, Option<String>) {
let mut parts = value.split_whitespace();
let program = parts.next().unwrap_or_default().to_string();
let version = parts.next().map(ToString::to_string);
(program, version)
}
fn tmux_client_info() -> TmuxClientInfo {
let termtype = tmux_display_message("#{client_termtype}");
let termname = tmux_display_message("#{client_termname}");
TmuxClientInfo { termtype, termname }
}
fn tmux_display_message(format: &str) -> Option<String> {
let output = std::process::Command::new("tmux")
.args(["display-message", "-p", format])
.output()
.ok()?;
if !output.status.success() {
return None;
}
let value = String::from_utf8(output.stdout).ok()?;
none_if_whitespace(value.trim().to_string())
}
fn zellij_version_from_command() -> Option<String> {
// Best-effort fallback: missing or broken zellij binaries should not affect
// terminal detection.
let output = std::process::Command::new("zellij")
.arg("--version")
.output()
.ok()?;
if !output.status.success() {
return None;
}
let stdout = String::from_utf8(output.stdout).ok()?;
parse_zellij_version(stdout.trim())
}
fn parse_zellij_version(value: &str) -> Option<String> {
let value = none_if_whitespace(value.to_string())?;
let mut parts = value.split_whitespace();
match (parts.next(), parts.next()) {
(Some(command), Some(version)) if command.eq_ignore_ascii_case("zellij") => {
Some(version.to_string())
}
_ => Some(value),
}
}
/// Sanitizes a terminal token for use in User-Agent headers.
///
/// Invalid header characters are replaced with underscores.
codex-rs/terminal-detection/src/terminal_tests.rs
+54 -141
View File
@@ -4,16 +4,12 @@ use std::collections::HashMap;
struct FakeEnvironment {
vars: HashMap<String, String>,
tmux_client_info: TmuxClientInfo,
zellij_version: Option<String>,
}
impl FakeEnvironment {
fn new() -> Self {
Self {
vars: HashMap::new(),
tmux_client_info: TmuxClientInfo::default(),
zellij_version: None,
}
}
@@ -21,35 +17,12 @@ impl FakeEnvironment {
self.vars.insert(key.to_string(), value.to_string());
self
}
fn with_tmux_client_info(mut self, termtype: Option<&str>, termname: Option<&str>) -> Self {
self.tmux_client_info = TmuxClientInfo {
termtype: termtype.map(ToString::to_string),
termname: termname.map(ToString::to_string),
};
self
}
fn with_zellij_version(mut self, version: &str) -> Self {
self.zellij_version = Some(version.to_string());
self
}
}
impl Environment for FakeEnvironment {
fn var(&self, name: &str) -> Option<String> {
self.vars.get(name).cloned()
}
fn tmux_client_info(&self) -> TmuxClientInfo {
self.tmux_client_info.clone()
}
fn zellij_version(&self) -> Option<String> {
self.zellij_version
.clone()
.or_else(|| self.var_non_empty("ZELLIJ_VERSION"))
}
}
fn terminal_info(
@@ -313,22 +286,24 @@ fn detects_tmux_multiplexer() {
let env = FakeEnvironment::new()
.with_var("TMUX", "/tmp/tmux-1000/default,123,0")
.with_var("TERM_PROGRAM", "tmux")
.with_tmux_client_info(Some("xterm-256color"), Some("screen-256color"));
.with_var("TERM_PROGRAM_VERSION", "3.5a");
let terminal = detect_terminal_info_from_env(&env);
assert_eq!(
terminal,
terminal_info(
TerminalName::Unknown,
Some("xterm-256color"),
/*term_program*/ None,
/*version*/ None,
Some("screen-256color"),
Some(Multiplexer::Tmux { version: None }),
/*term*/ None,
Some(Multiplexer::Tmux {
version: Some("3.5a".to_string())
}),
),
"tmux_multiplexer_info"
);
assert_eq!(
terminal.user_agent_token(),
"xterm-256color",
"unknown",
"tmux_multiplexer_user_agent"
);
}
@@ -369,115 +344,6 @@ fn detects_zellij_multiplexer_version() {
);
}
#[test]
fn detects_zellij_multiplexer_command_version() {
let env = FakeEnvironment::new()
.with_var("ZELLIJ", "1")
.with_zellij_version("0.44.1");
let terminal = detect_terminal_info_from_env(&env);
assert_eq!(
terminal,
terminal_info(
TerminalName::Unknown,
/*term_program*/ None,
/*version*/ None,
/*term*/ None,
Some(Multiplexer::Zellij {
version: Some("0.44.1".to_string()),
}),
),
"zellij_multiplexer_command_version"
);
}
#[test]
fn parses_zellij_version_output() {
assert_eq!(
parse_zellij_version("zellij 0.44.1"),
Some("0.44.1".to_string())
);
assert_eq!(parse_zellij_version("0.44.1"), Some("0.44.1".to_string()));
assert_eq!(parse_zellij_version(""), None);
}
#[test]
fn detects_tmux_client_termtype() {
let env = FakeEnvironment::new()
.with_var("TMUX", "/tmp/tmux-1000/default,123,0")
.with_var("TERM_PROGRAM", "tmux")
.with_tmux_client_info(Some("WezTerm"), /*termname*/ None);
let terminal = detect_terminal_info_from_env(&env);
assert_eq!(
terminal,
terminal_info(
TerminalName::WezTerm,
Some("WezTerm"),
/*version*/ None,
/*term*/ None,
Some(Multiplexer::Tmux { version: None }),
),
"tmux_client_termtype_info"
);
assert_eq!(
terminal.user_agent_token(),
"WezTerm",
"tmux_client_termtype_user_agent"
);
}
#[test]
fn detects_tmux_client_termname() {
let env = FakeEnvironment::new()
.with_var("TMUX", "/tmp/tmux-1000/default,123,0")
.with_var("TERM_PROGRAM", "tmux")
.with_tmux_client_info(/*termtype*/ None, Some("xterm-256color"));
let terminal = detect_terminal_info_from_env(&env);
assert_eq!(
terminal,
terminal_info(
TerminalName::Unknown,
/*term_program*/ None,
/*version*/ None,
Some("xterm-256color"),
Some(Multiplexer::Tmux { version: None })
),
"tmux_client_termname_info"
);
assert_eq!(
terminal.user_agent_token(),
"xterm-256color",
"tmux_client_termname_user_agent"
);
}
#[test]
fn detects_tmux_term_program_uses_client_termtype() {
let env = FakeEnvironment::new()
.with_var("TMUX", "/tmp/tmux-1000/default,123,0")
.with_var("TERM_PROGRAM", "tmux")
.with_var("TERM_PROGRAM_VERSION", "3.6a")
.with_tmux_client_info(Some("ghostty 1.2.3"), Some("xterm-ghostty"));
let terminal = detect_terminal_info_from_env(&env);
assert_eq!(
terminal,
terminal_info(
TerminalName::Ghostty,
Some("ghostty"),
Some("1.2.3"),
Some("xterm-ghostty"),
Some(Multiplexer::Tmux {
version: Some("3.6a".to_string()),
}),
),
"tmux_term_program_client_termtype_info"
);
assert_eq!(
terminal.user_agent_token(),
"ghostty/1.2.3",
"tmux_term_program_client_termtype_user_agent"
);
}
#[test]
fn detects_wezterm() {
let env = FakeEnvironment::new().with_var("WEZTERM_VERSION", "2024.2");
@@ -961,3 +827,50 @@ fn detects_term_fallbacks() {
);
assert_eq!(terminal.user_agent_token(), "unknown", "unknown_user_agent");
}
#[test]
fn tmux_preserves_safe_underlying_terminal_identifiers() {
for (variable, value, name, version) in [
(
"WEZTERM_VERSION",
"2024.2",
TerminalName::WezTerm,
Some("2024.2"),
),
("ITERM_SESSION_ID", "w0t1p0", TerminalName::Iterm2, None),
("KITTY_WINDOW_ID", "1", TerminalName::Kitty, None),
("WT_SESSION", "session", TerminalName::WindowsTerminal, None),
(
"ALACRITTY_SOCKET",
"/tmp/alacritty",
TerminalName::Alacritty,
None,
),
(
"GHOSTTY_RESOURCES_DIR",
"/Applications/Ghostty.app/Contents/Resources/ghostty",
TerminalName::Ghostty,
None,
),
] {
let env = FakeEnvironment::new()
.with_var("TMUX", "/tmp/tmux-1000/default,123,0")
.with_var("TERM_PROGRAM", "tmux")
.with_var("TERM_PROGRAM_VERSION", "3.5a")
.with_var("TERM", "screen-256color")
.with_var(variable, value);
assert_eq!(
detect_terminal_info_from_env(&env),
terminal_info(
name,
/*term_program*/ None,
version,
/*term*/ None,
Some(Multiplexer::Tmux {
version: Some("3.5a".to_string())
}),
),
"{variable}"
);
}
}
codex-rs/tui/src/tui/keyboard_modes.rs
+35 -4
View File
@@ -64,6 +64,9 @@ fn running_in_wsl() -> bool {
}
pub(super) fn running_in_vscode_terminal() -> bool {
if term_program_is_vscode(std::env::var("TERM_PROGRAM").ok().as_deref()) {
return true;
}
vscode_terminal_detected(
std::env::var("TERM_PROGRAM").ok().as_deref(),
windows_term_program().as_deref(),
@@ -99,7 +102,11 @@ fn windows_term_program() -> Option<String> {
#[cfg(target_os = "linux")]
fn read_windows_term_program() -> Option<String> {
let output = std::process::Command::new("cmd.exe")
if !running_in_wsl() {
return None;
}
let executable = codex_utils_path::system_executable("cmd.exe")?;
let output = std::process::Command::new(executable)
.args(["/d", "/s", "/c", "set TERM_PROGRAM"])
.stdin(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
@@ -137,6 +144,7 @@ pub(super) fn enable_keyboard_enhancement() {
DisableModifyOtherKeys,
PushKeyboardEnhancementFlags(keyboard_enhancement_flags(
terminal_info().name,
running_in_tmux_session,
tmux_extended_keys_format.as_deref()
))
);
@@ -151,6 +159,7 @@ pub(super) fn enable_keyboard_enhancement() {
fn keyboard_enhancement_flags(
terminal_name: TerminalName,
running_in_tmux_session: bool,
tmux_extended_keys_format: Option<&str>,
) -> KeyboardEnhancementFlags {
let flags = KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES
@@ -158,9 +167,10 @@ fn keyboard_enhancement_flags(
// iTerm and Ghostty can leak shortcut release events that the terminal consumes.
// tmux's xterm key format also loses Shift-Enter when event types are
// reported. Preserve repeat classification on transports that support it.
// reported. An unavailable/unrecognized tmux probe must take the same safe
// fallback. Preserve repeat classification on confirmed csi-u transports.
if matches!(terminal_name, TerminalName::Ghostty | TerminalName::Iterm2)
|| matches!(tmux_extended_keys_format, Some("xterm"))
|| (running_in_tmux_session && !matches!(tmux_extended_keys_format, Some("csi-u")))
{
flags
} else {
@@ -190,11 +200,14 @@ fn tmux_should_enable_modify_other_keys_for(
}
fn read_tmux_extended_keys_format() -> Option<String> {
let executable = codex_utils_path::system_executable("tmux")?;
let path = codex_utils_path::system_path().ok()?;
for args in [
["display-message", "-p", "#{extended-keys-format}"],
["show-options", "-gqv", "extended-keys-format"],
] {
let output = std::process::Command::new("tmux")
let output = std::process::Command::new(&executable)
.env("PATH", &path)
.args(args)
.stdin(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
@@ -327,6 +340,7 @@ mod tests {
assert_eq!(
ansi_for(PushKeyboardEnhancementFlags(keyboard_enhancement_flags(
TerminalName::Iterm2,
/*running_in_tmux_session*/ false,
/*tmux_extended_keys_format*/ None
))),
"\x1b[>5u"
@@ -338,6 +352,7 @@ mod tests {
assert_eq!(
ansi_for(PushKeyboardEnhancementFlags(keyboard_enhancement_flags(
TerminalName::Ghostty,
/*running_in_tmux_session*/ false,
/*tmux_extended_keys_format*/ None
))),
"\x1b[>5u"
@@ -349,6 +364,7 @@ mod tests {
assert_eq!(
ansi_for(PushKeyboardEnhancementFlags(keyboard_enhancement_flags(
TerminalName::Kitty,
/*running_in_tmux_session*/ false,
/*tmux_extended_keys_format*/ None
))),
"\x1b[>7u"
@@ -360,6 +376,7 @@ mod tests {
assert_eq!(
ansi_for(PushKeyboardEnhancementFlags(keyboard_enhancement_flags(
TerminalName::Kitty,
/*running_in_tmux_session*/ true,
Some("csi-u")
))),
"\x1b[>7u"
@@ -371,6 +388,7 @@ mod tests {
assert_eq!(
ansi_for(PushKeyboardEnhancementFlags(keyboard_enhancement_flags(
TerminalName::Kitty,
/*running_in_tmux_session*/ true,
Some("xterm")
))),
"\x1b[>5u"
@@ -382,12 +400,25 @@ mod tests {
assert_eq!(
ansi_for(PushKeyboardEnhancementFlags(keyboard_enhancement_flags(
TerminalName::Unknown,
/*running_in_tmux_session*/ false,
/*tmux_extended_keys_format*/ None
))),
"\x1b[>7u"
);
}
#[test]
fn keyboard_enhancement_uses_conservative_flags_when_tmux_format_is_unknown() {
assert_eq!(
ansi_for(PushKeyboardEnhancementFlags(keyboard_enhancement_flags(
TerminalName::Unknown,
/*running_in_tmux_session*/ true,
/*tmux_extended_keys_format*/ None,
))),
"\x1b[>5u"
);
}
#[test]
fn keyboard_enhancement_env_flag_parses_common_values() {
assert_eq!(parse_bool_env(Some("1")), Some(true));
codex-rs/utils/path-utils/Cargo.toml
+7
View File
@@ -12,6 +12,13 @@ codex-utils-absolute-path = { workspace = true }
dunce = { workspace = true }
tempfile = { workspace = true }
[target.'cfg(windows)'.dependencies]
windows-sys = { version = "0.52", features = [
"Win32_Foundation",
"Win32_System_Com",
"Win32_UI_Shell",
] }
[dev-dependencies]
pretty_assertions = { workspace = true }
tempfile = { workspace = true }
codex-rs/utils/path-utils/src/lib.rs
+3
View File
@@ -2,6 +2,9 @@
pub(crate) mod env;
pub use env::is_wsl;
mod system_commands;
pub use system_commands::system_executable;
pub use system_commands::system_path;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::HashSet;
codex-rs/utils/path-utils/src/system_commands.rs
+156
View File
@@ -0,0 +1,156 @@
//! Installed helper discovery for automatic work before workspace trust.
//!
//! Only OS and conventional package-manager locations are trusted, never ambient
//! PATH or the working directory. Custom installations fall back to no helper.
//! This assumes the installed software directories themselves are trusted; it
//! does not defend against an attacker who can modify system installations.
use std::ffi::OsString;
use std::path::Path;
use std::path::PathBuf;
/// Finds an installed helper without consulting PATH, PATHEXT, or the working directory.
pub fn system_executable(name: &str) -> Option<PathBuf> {
executable_in_directories(name, &system_directories())
}
/// Child-process PATH for automatic helpers, not for user-requested tools.
pub fn system_path() -> std::io::Result<OsString> {
let directories = system_directories();
if directories.is_empty() {
return Err(std::io::Error::other("no trusted executable directories"));
}
std::env::join_paths(directories).map_err(std::io::Error::other)
}
fn executable_in_directories(name: &str, directories: &[PathBuf]) -> Option<PathBuf> {
// Do not allow callers to accidentally escape the installation directories.
if Path::new(name).file_name() != Some(std::ffi::OsStr::new(name)) {
return None;
}
let name = format!("{name}{}", std::env::consts::EXE_SUFFIX);
directories.iter().find_map(|directory| {
let executable = dunce::canonicalize(directory.join(&name)).ok()?;
// A symlink into a workspace is not an installed executable. Package
// manager links may target sibling libexec/Cellar directories.
let installed = directories.iter().any(|root| executable.starts_with(root))
|| installation_roots()
.iter()
.any(|root| executable.starts_with(root));
if !installed || !executable.is_file() {
return None;
}
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
if executable.metadata().ok()?.permissions().mode() & 0o111 == 0 {
return None;
}
}
Some(executable)
})
}
#[cfg(unix)]
fn installation_roots() -> Vec<PathBuf> {
[
"/usr/bin",
"/bin",
"/usr/sbin",
"/sbin",
"/usr/local",
"/opt/homebrew",
"/opt/local",
"/Library/Developer/CommandLineTools",
"/Applications/Xcode.app/Contents/Developer",
"/nix/store",
"/mnt/c/Windows/System32",
]
.into_iter()
.map(PathBuf::from)
.collect()
}
fn system_directories() -> Vec<PathBuf> {
#[cfg(unix)]
let directories = [
"/opt/homebrew/bin",
"/usr/local/bin",
"/opt/local/bin",
"/Library/Developer/CommandLineTools/usr/bin",
"/Applications/Xcode.app/Contents/Developer/usr/bin",
"/usr/bin",
"/bin",
"/usr/sbin",
"/sbin",
]
.into_iter()
.map(PathBuf::from);
#[cfg(target_os = "linux")]
let directories =
directories.chain(crate::is_wsl().then(|| PathBuf::from("/mnt/c/Windows/System32")));
#[cfg(windows)]
let directories = installation_roots().into_iter().flat_map(|root| {
vec![
root.join("Git/cmd"),
root.join("Git/mingw64/bin"),
root.join("Git/usr/bin"),
root,
]
});
let roots = installation_roots();
directories
.filter_map(|directory| dunce::canonicalize(directory).ok())
.filter(|directory| roots.iter().any(|root| directory.starts_with(root)))
.collect()
}
#[cfg(windows)]
fn installation_roots() -> Vec<PathBuf> {
use std::os::windows::ffi::OsStringExt;
use windows_sys::Win32::System::Com::CoTaskMemFree;
use windows_sys::Win32::UI::Shell::FOLDERID_ProgramFiles;
use windows_sys::Win32::UI::Shell::FOLDERID_ProgramFilesX86;
use windows_sys::Win32::UI::Shell::FOLDERID_System;
use windows_sys::Win32::UI::Shell::KF_FLAG_DEFAULT_PATH;
use windows_sys::Win32::UI::Shell::SHGetKnownFolderPath;
[
FOLDERID_ProgramFiles,
FOLDERID_ProgramFilesX86,
FOLDERID_System,
]
.iter()
.filter_map(|id| {
let mut path = std::ptr::null_mut();
// SAFETY: the API writes a CoTaskMem-allocated, NUL-terminated UTF-16 path.
let result = unsafe {
SHGetKnownFolderPath(
id,
KF_FLAG_DEFAULT_PATH as u32,
/*htoken*/ 0,
&mut path,
)
};
if result != 0 || path.is_null() {
return None;
}
// SAFETY: success guarantees the string; free it after copying.
let directory = unsafe {
let mut len = 0;
while *path.add(len) != 0 {
len += 1;
}
let directory =
PathBuf::from(OsString::from_wide(std::slice::from_raw_parts(path, len)));
CoTaskMemFree(path.cast());
directory
};
dunce::canonicalize(directory).ok()
})
.collect()
}
#[cfg(test)]
#[path = "system_commands_tests.rs"]
mod tests;
codex-rs/utils/path-utils/src/system_commands_tests.rs
+31
View File
@@ -0,0 +1,31 @@
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn resolves_only_executables_inside_installation_directories() {
let root = tempfile::tempdir().unwrap();
let bin = root.path().join("bin");
std::fs::create_dir(&bin).unwrap();
let executable = bin.join(format!("helper{}", std::env::consts::EXE_SUFFIX));
std::fs::write(&executable, "installed helper").unwrap();
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(/*mode*/ 0o755))
.unwrap();
}
let directories = vec![dunce::canonicalize(bin).unwrap()];
assert_eq!(
executable_in_directories("helper", &directories),
Some(dunce::canonicalize(&executable).unwrap())
);
assert_eq!(executable_in_directories("../helper", &directories), None);
assert_eq!(executable_in_directories("missing", &directories), None);
#[cfg(unix)]
{
let outside = root.path().join("outside");
std::fs::rename(&executable, &outside).unwrap();
std::os::unix::fs::symlink(&outside, &executable).unwrap();
assert_eq!(executable_in_directories("helper", &directories), None);
}
}