Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::*;
use codex_protocol::shell_environment::is_non_inheritable_env_var;

#[derive(Clone)]
pub(crate) struct CommandExecRequestProcessor {
Expand Down Expand Up @@ -163,6 +164,7 @@ impl CommandExecRequestProcessor {
}
}
}
env.retain(|name, _| !is_non_inheritable_env_var(name));
let timeout_ms = match timeout_ms {
Some(timeout_ms) => match u64::try_from(timeout_ms) {
Ok(timeout_ms) => Some(timeout_ms),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use codex_core::exec::ExecExpirationOutcome;
use codex_core::exec::IO_DRAIN_TIMEOUT_MS;
use codex_exec_server::EnvironmentManager;
use codex_protocol::exec_output::bytes_to_string_smart;
use codex_protocol::shell_environment::is_non_inheritable_env_var;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP;
use codex_utils_pty::ProcessHandle;
Expand Down Expand Up @@ -107,6 +108,7 @@ impl ProcessExecRequestProcessor {
}
}
}
env.retain(|name, _| !is_non_inheritable_env_var(name));
let expiration = match timeout_ms {
Some(Some(timeout_ms)) => match u64::try_from(timeout_ms) {
Ok(timeout_ms) => timeout_ms.into(),
Expand Down
13 changes: 11 additions & 2 deletions codex-rs/app-server/tests/suite/v2/command_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SandboxPolicy;
use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR;
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY;
use codex_protocol::shell_environment::OPENAI_FEDERATION_RULE_ID_ENV_VAR;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::path::Path;
Expand Down Expand Up @@ -151,7 +152,7 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un
command: vec![
"/bin/sh".to_string(),
"-lc".to_string(),
"printf '%s|%s|%s|%s' \"$COMMAND_EXEC_BASELINE\" \"$COMMAND_EXEC_EXTRA\" \"${RUST_LOG-unset}\" \"$CODEX_HOME\"".to_string(),
"printf '%s|%s|%s|%s|%s|%s' \"$COMMAND_EXEC_BASELINE\" \"$COMMAND_EXEC_EXTRA\" \"${RUST_LOG-unset}\" \"$CODEX_HOME\" \"$OPENAI_FEDERATION_RULE_ID\" \"$openai_identity_token_file\"".to_string(),
],
process_id: None,
tty: false,
Expand All @@ -169,6 +170,14 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un
),
("COMMAND_EXEC_EXTRA".to_string(), Some("added".to_string())),
("RUST_LOG".to_string(), None),
(
OPENAI_FEDERATION_RULE_ID_ENV_VAR.to_string(),
Some("rule".to_string()),
),
(
"openai_identity_token_file".to_string(),
Some("/run/identity-token".to_string()),
),
])),
size: None,
sandbox_policy: None,
Expand All @@ -181,7 +190,7 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un
response,
CommandExecResponse {
exit_code: 0,
stdout: format!("request|added|unset|{}", codex_home.path().display()),
stdout: format!("request|added|unset|{}||", codex_home.path().display()),
stderr: String::new(),
}
);
Expand Down
14 changes: 11 additions & 3 deletions codex-rs/app-server/tests/suite/v2/process_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu
"while (!(Test-Path -LiteralPath $env:CODEX_PROCESS_EXEC_RELEASE_FILE)) { ",
"Start-Sleep -Milliseconds 20 ",
"}; ",
"[Console]::Out.Write('process-out'); ",
"[Console]::Out.Write(('process-out|{0}|{1}' -f $env:OpenAI_Federation_Rule_Id, $env:OPENAI_IDENTITY_TOKEN_FILE)); ",
"[Console]::Error.Write('process-err')",
)
.to_string(),
Expand All @@ -54,7 +54,7 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu
concat!(
"printf process > \"$CODEX_PROCESS_EXEC_PROBE_FILE\"; ",
"while [ ! -e \"$CODEX_PROCESS_EXEC_RELEASE_FILE\" ]; do sleep 0.05; done; ",
"printf process-out; ",
"printf 'process-out|%s|%s' \"$OpenAI_Federation_Rule_Id\" \"$OPENAI_IDENTITY_TOKEN_FILE\"; ",
"printf process-err >&2",
)
.to_string(),
Expand All @@ -69,6 +69,14 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu
"CODEX_PROCESS_EXEC_RELEASE_FILE".to_string(),
Some(release_file.display().to_string()),
),
(
"OpenAI_Federation_Rule_Id".to_string(),
Some("rule".to_string()),
),
(
"OPENAI_IDENTITY_TOKEN_FILE".to_string(),
Some("/run/identity-token".to_string()),
),
]);
let spawn_request_id = mcp
.send_process_spawn_request(ProcessSpawnParams {
Expand All @@ -94,7 +102,7 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu
ProcessExitedNotification {
process_handle,
exit_code: 0,
stdout: "process-out".to_string(),
stdout: "process-out||".to_string(),
stdout_cap_reached: false,
stderr: "process-err".to_string(),
stderr_cap_reached: false,
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/code-mode/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ workspace = true
codex-code-mode-protocol = { workspace = true }
codex-http-client = { workspace = true }
codex-install-context = { workspace = true }
codex-protocol = { workspace = true }
codex-websocket-client = { workspace = true }
futures = { workspace = true }
tokio = { workspace = true, features = ["io-util", "macros", "net", "process", "rt", "sync", "time"] }
Expand All @@ -24,6 +25,5 @@ tokio-util = { workspace = true, features = ["rt"] }
tracing = { workspace = true }

[dev-dependencies]
codex-protocol = { workspace = true }
pretty_assertions = { workspace = true }
tokio = { workspace = true, features = ["test-util"] }
15 changes: 8 additions & 7 deletions codex-rs/code-mode/src/remote_session/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use codex_code_mode_protocol::host::SESSION_RESOURCE_LIMITS_CAPABILITY;
use codex_code_mode_protocol::host::SupportedProtocolVersions;
use codex_code_mode_protocol::host::TransportLane;
use codex_http_client::HttpClientFactory;
use codex_protocol::shell_environment::scrub_non_inheritable_env_vars;
use codex_websocket_client::WebSocketConnector;
use futures::StreamExt;
use tokio::io::AsyncBufReadExt;
Expand Down Expand Up @@ -215,16 +216,16 @@ impl Connection {
let mut command = Command::new(host_program);
#[cfg(unix)]
command.process_group(0);
let mut child = command
command
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.kill_on_drop(true)
.spawn()
.map_err(|error| ConnectionError::Spawn {
host_program: host_program.to_path_buf(),
error,
})?;
.kill_on_drop(true);
scrub_non_inheritable_env_vars(command.as_std_mut());
let mut child = command.spawn().map_err(|error| ConnectionError::Spawn {
host_program: host_program.to_path_buf(),
error,
})?;

if let Some(stderr) = child.stderr.take() {
tokio::spawn(async move {
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/core/src/spawn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use tokio::process::Command;
use tracing::trace;

use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::shell_environment::is_non_inheritable_env_var;

/// Experimental environment variable that will be set to some non-empty value
/// if both of the following are true:
Expand Down Expand Up @@ -60,6 +61,8 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io
mut env,
} = request;

env.retain(|name, _| !is_non_inheritable_env_var(name));

trace!(
"spawn_child_async: {program:?} {args:?} {arg0:?} {cwd:?} {network_sandbox_policy:?} {stdio_policy:?} {env:?}"
);
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/core/src/tools/runtimes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub(crate) use codex_network_proxy::is_managed_proxy_env_var;
pub(crate) use codex_network_proxy::strip_managed_proxy_env;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::shell_environment::is_non_inheritable_env_var;
use codex_sandboxing::SandboxCommand;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
Expand Down Expand Up @@ -304,6 +305,7 @@ fn build_override_exports(
.keys()
.map(String::as_str)
.chain(restore_even_when_absent.iter().copied())
.filter(|key| !is_non_inheritable_env_var(key))
.filter(|key| is_valid_shell_variable_name(key))
.collect::<Vec<_>>();
keys.sort_unstable();
Expand Down
15 changes: 11 additions & 4 deletions codex-rs/core/src/tools/runtimes/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1082,10 +1082,16 @@ fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() {
"-lc".to_string(),
"printf '%s' \"$OPENAI_API_KEY\"".to_string(),
];
let explicit_env_overrides = HashMap::from([(
"OPENAI_API_KEY".to_string(),
"super-secret-value".to_string(),
)]);
let explicit_env_overrides = HashMap::from([
(
"OPENAI_API_KEY".to_string(),
"super-secret-value".to_string(),
),
(
"openai_identity_token_file".to_string(),
"/run/identity-token".to_string(),
),
]);
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
Expand All @@ -1099,6 +1105,7 @@ fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() {
);

assert!(!rewritten[2].contains("super-secret-value"));
assert!(!rewritten[2].contains("openai_identity_token_file"));
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env("OPENAI_API_KEY", "super-secret-value")
Expand Down
11 changes: 8 additions & 3 deletions codex-rs/core/src/unified_exec/process_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ use codex_protocol::error::SandboxErr;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecCommandSource;
use codex_protocol::protocol::TerminalInteractionEvent;
use codex_protocol::shell_environment::is_non_inheritable_env_var;
use codex_sandboxing::SandboxCommand;
use codex_tools::ToolName;
use codex_utils_output_truncation::approx_tokens_from_byte_count;
Expand Down Expand Up @@ -123,7 +124,10 @@ fn exec_env_policy_from_shell_policy(
.collect::<Vec<_>>();
exclude.push(CODEX_PERMISSION_PROFILE_ENV_VAR.to_string());
let mut r#set = policy.r#set.clone();
r#set.retain(|key, _| !key.eq_ignore_ascii_case(CODEX_PERMISSION_PROFILE_ENV_VAR));
r#set.retain(|key, _| {
!key.eq_ignore_ascii_case(CODEX_PERMISSION_PROFILE_ENV_VAR)
&& !is_non_inheritable_env_var(key)
});
codex_exec_server::ExecEnvPolicy {
inherit: policy.inherit.clone(),
ignore_default_excludes: policy.ignore_default_excludes,
Expand All @@ -144,8 +148,9 @@ fn env_overlay_for_exec_server(
request_env
.iter()
.filter(|(key, value)| {
key.as_str() == CODEX_PERMISSION_PROFILE_ENV_VAR
|| local_policy_env.get(*key) != Some(*value)
!is_non_inheritable_env_var(key)
&& (key.as_str() == CODEX_PERMISSION_PROFILE_ENV_VAR
|| local_policy_env.get(*key) != Some(*value))
})
.map(|(key, value)| (key.clone(), value.clone()))
.collect()
Expand Down
7 changes: 6 additions & 1 deletion codex-rs/core/src/unified_exec/process_manager_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ fn env_overlay_for_exec_server_keeps_runtime_changes_only() {
let request_env = HashMap::from([
("HOME".to_string(), "/client-home".to_string()),
("PATH".to_string(), "/sandbox-path".to_string()),
("OpenAI_Federation_Rule_Id".to_string(), "rule".to_string()),
("SHELL_SET".to_string(), "policy".to_string()),
("CODEX_THREAD_ID".to_string(), "thread-1".to_string()),
(
Expand Down Expand Up @@ -81,13 +82,17 @@ fn env_overlay_for_exec_server_keeps_runtime_changes_only() {
}

#[test]
fn exec_env_policy_excludes_runtime_permission_profile() {
fn exec_env_policy_excludes_non_inheritable_and_runtime_variables() {
let policy = ShellEnvironmentPolicy {
r#set: HashMap::from([
(
"codex_permission_profile".to_string(),
"stale-profile".to_string(),
),
(
"openai_identity_token_file".to_string(),
"/run/identity-token".to_string(),
),
("KEEP".to_string(), "value".to_string()),
]),
..Default::default()
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/exec-server/src/client_transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use tracing::debug;
use tracing::warn;

use codex_http_client::HttpClientFactory;
use codex_protocol::shell_environment::scrub_non_inheritable_env_vars;
use codex_utils_rustls_provider::ensure_rustls_crypto_provider;
use codex_websocket_client::WebSocketConnector;
use codex_websocket_client::WebSocketTlsMode;
Expand Down Expand Up @@ -441,6 +442,7 @@ fn stdio_command_process(stdio_command: &StdioExecServerCommand) -> Command {
let mut command = Command::new(&stdio_command.program);
command.args(&stdio_command.args);
command.envs(&stdio_command.env);
scrub_non_inheritable_env_vars(command.as_std_mut());
if let Some(cwd) = &stdio_command.cwd {
command.current_dir(cwd);
}
Expand Down
3 changes: 2 additions & 1 deletion codex-rs/exec-server/src/fs_sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ fn spawn_command(
SandboxExecRequest {
command: argv,
cwd,
env,
mut env,
arg0,
..
}: SandboxExecRequest,
Expand All @@ -346,6 +346,7 @@ fn spawn_command(
// TODO(anp): Keep PathUri through the filesystem helper launch boundary.
let cwd = cwd.to_abs_path().map_err(io_error)?;
command.current_dir(cwd.as_path());
env.retain(|name, _| !codex_protocol::shell_environment::is_non_inheritable_env_var(name));
command.env_clear();
command.envs(env);
command.stdin(std::process::Stdio::piped());
Expand Down
10 changes: 9 additions & 1 deletion codex-rs/exec-server/src/local_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,7 @@ fn child_env(params: &ExecParams) -> HashMap<String, String> {
None => params.env.clone(),
};
env.remove(crate::CODEX_EXEC_SERVER_EXIT_ON_STDIN_CLOSE_ENV_VAR);
env.retain(|name, _| !shell_environment::is_non_inheritable_env_var(name));
env
}

Expand Down Expand Up @@ -1268,12 +1269,19 @@ mod tests {
let mut params = test_exec_params(HashMap::from([
("OVERLAY".to_string(), "overlay".to_string()),
("POLICY_SET".to_string(), "overlay-wins".to_string()),
(
"openai_identity_token_file".to_string(),
"/run/identity-token".to_string(),
),
]));
params.env_policy = Some(ExecEnvPolicy {
inherit: ShellEnvironmentPolicyInherit::None,
ignore_default_excludes: true,
exclude: Vec::new(),
r#set: HashMap::from([("POLICY_SET".to_string(), "policy".to_string())]),
r#set: HashMap::from([
("POLICY_SET".to_string(), "policy".to_string()),
("OpenAI_Federation_Rule_Id".to_string(), "rule".to_string()),
]),
include_only: Vec::new(),
});

Expand Down
10 changes: 7 additions & 3 deletions codex-rs/git-utils/src/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,14 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
}

fn resolve_git_root(cwd: &Path) -> io::Result<PathBuf> {
let out = std::process::Command::new("git")
let mut command = std::process::Command::new("git");
command
.args(["-c", crate::SAFE_BARE_REPOSITORY_CONFIG])
.arg("rev-parse")
.arg("--show-toplevel")
.current_dir(cwd)
.output()?;
.current_dir(cwd);
crate::scrub_non_inheritable_environment(&mut command);
let out = command.output()?;
let code = out.status.code().unwrap_or(-1);
if code != 0 {
return Err(io::Error::other(format!(
Expand Down Expand Up @@ -158,6 +160,7 @@ fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32,
for a in args {
cmd.arg(a);
}
crate::scrub_non_inheritable_environment(&mut cmd);
let out = cmd.current_dir(cwd).output()?;
let code = out.status.code().unwrap_or(-1);
let stdout = String::from_utf8_lossy(&out.stdout).into_owned();
Expand Down Expand Up @@ -338,6 +341,7 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> {
for p in &existing {
cmd.arg(OsStr::new(p));
}
crate::scrub_non_inheritable_environment(&mut cmd);
let out = cmd.current_dir(git_root).output()?;
let _code = out.status.code().unwrap_or(-1);
// We do not hard fail staging; best-effort is OK. Return Ok even on non-zero.
Expand Down
1 change: 1 addition & 0 deletions codex-rs/git-utils/src/git_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ impl Drop for KillGitProcessTreeOnDrop {
}

fn spawn_git_command(command: &mut Command) -> Option<(Child, KillGitProcessTreeOnDrop)> {
crate::scrub_non_inheritable_environment(command.as_std_mut());
#[cfg(unix)]
command.process_group(0);
command.kill_on_drop(true);
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/git-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,7 @@ pub use info::recent_commits;
pub use info::resolve_root_git_project_for_trust;
pub use platform::create_symlink;
pub use status::get_has_changes_in_repo;

pub(crate) fn scrub_non_inheritable_environment(command: &mut std::process::Command) {
codex_protocol::shell_environment::scrub_non_inheritable_env_vars(command);
}
Loading
Loading