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
26 changes: 26 additions & 0 deletions codex-rs/core/src/exec_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@ use codex_protocol::ThreadId;
#[cfg(test)]
use codex_protocol::config_types::EnvironmentVariablePattern;
use codex_protocol::config_types::ShellEnvironmentPolicy;
use codex_protocol::models::ActivePermissionProfile;
use codex_protocol::shell_environment;
use std::collections::HashMap;

pub use codex_protocol::shell_environment::CODEX_THREAD_ID_ENV_VAR;

/// Informational name of the active permission profile. Child processes can
/// overwrite this value, so it must not be treated as proof of enforcement.
pub const CODEX_PERMISSION_PROFILE_ENV_VAR: &str = "CODEX_PERMISSION_PROFILE";

/// Construct an environment map based on the rules in the specified policy. The
/// resulting map can be passed directly to `Command::envs()` after calling
/// `env_clear()` to ensure no unintended variables are leaked to the spawned
Expand All @@ -25,6 +30,27 @@ pub fn create_env(
shell_environment::create_env(policy, thread_id.as_deref())
}

/// Injects the selected named permission profile into a shell tool's environment.
///
/// This is applied after the shell environment policy so the runtime-selected
/// profile wins over inherited or configured values.
pub(crate) fn inject_permission_profile_env(
env: &mut HashMap<String, String>,
active_permission_profile: Option<&ActivePermissionProfile>,
) {
if cfg!(windows) {
env.retain(|key, _| !key.eq_ignore_ascii_case(CODEX_PERMISSION_PROFILE_ENV_VAR));
} else {
env.remove(CODEX_PERMISSION_PROFILE_ENV_VAR);
}
if let Some(active_permission_profile) = active_permission_profile {
env.insert(
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
active_permission_profile.id.clone(),
Comment thread
bolinfest marked this conversation as resolved.
);
}
}

#[cfg(all(test, target_os = "windows"))]
fn create_env_from_vars<I>(
vars: I,
Expand Down
53 changes: 53 additions & 0 deletions codex-rs/core/src/exec_env_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,59 @@ fn make_vars(pairs: &[(&str, &str)]) -> Vec<(String, String)> {
.collect()
}

#[test]
fn inject_permission_profile_env_overrides_policy_value() {
let mut env = HashMap::from([(
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
"stale-profile".to_string(),
)]);

inject_permission_profile_env(
&mut env,
Some(&ActivePermissionProfile::new("current-profile")),
);

assert_eq!(
env.get(CODEX_PERMISSION_PROFILE_ENV_VAR)
.map(String::as_str),
Some("current-profile")
);
}

#[test]
fn inject_permission_profile_env_removes_stale_value_without_active_profile() {
let mut env = HashMap::from([(
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
"stale-profile".to_string(),
)]);

inject_permission_profile_env(&mut env, /*active_permission_profile*/ None);

assert_eq!(env.get(CODEX_PERMISSION_PROFILE_ENV_VAR), None);
}

#[cfg(target_os = "windows")]
#[test]
fn inject_permission_profile_env_replaces_differently_cased_windows_key() {
let mut env = HashMap::from([(
"codex_permission_profile".to_string(),
"stale-profile".to_string(),
)]);

inject_permission_profile_env(
&mut env,
Some(&ActivePermissionProfile::new("current-profile")),
);

assert_eq!(
env,
HashMap::from([(
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
"current-profile".to_string(),
)])
);
}

#[test]
fn test_core_inherit_defaults_keep_sensitive_vars() {
let vars = make_vars(&[
Expand Down
13 changes: 9 additions & 4 deletions codex-rs/core/src/tools/handlers/shell/shell_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use crate::exec::ExecCapturePolicy;
use crate::exec::ExecParams;
use crate::exec_env::create_env;
use crate::exec_env::inject_permission_profile_env;
use crate::function_tool::FunctionCallError;
use crate::maybe_emit_implicit_skill_invocation;
use crate::session::turn_context::TurnContext;
Expand Down Expand Up @@ -99,15 +100,19 @@ impl ShellCommandHandler {
let use_login_shell = Self::resolve_use_login_shell(params.login, allow_login_shell)?;
let command = Self::base_command(shell, &params.command, use_login_shell);

let mut env = create_env(
&turn_context.config.permissions.shell_environment_policy,
Some(session.thread_id),
);
let active_permission_profile = turn_context.config.permissions.active_permission_profile();
inject_permission_profile_env(&mut env, active_permission_profile.as_ref());

Ok(ExecParams {
command,
cwd,
expiration: params.timeout_ms.into(),
capture_policy: ExecCapturePolicy::ShellTool,
env: create_env(
&turn_context.config.permissions.shell_environment_policy,
Some(session.thread_id),
),
env,
network: turn_context.network.clone(),
network_environment_id: Some(turn_environment.environment_id.clone()),
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
Expand Down
24 changes: 22 additions & 2 deletions codex-rs/core/src/tools/handlers/shell_tests.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
use std::path::PathBuf;
use std::sync::Arc;

use codex_protocol::models::ActivePermissionProfile;
use codex_protocol::models::ShellCommandToolCallParams;
use pretty_assertions::assert_eq;

use crate::config::PermissionProfileSnapshot;
use crate::exec_env::CODEX_PERMISSION_PROFILE_ENV_VAR;
use crate::exec_env::create_env;
use crate::exec_env::inject_permission_profile_env;
use crate::sandboxing::SandboxPermissions;
use crate::session::step_context::StepContext;
use crate::session::tests::make_session_and_context;
Expand Down Expand Up @@ -71,7 +75,15 @@ fn assert_safe(shell: &Shell, command: &str) {

#[tokio::test]
async fn shell_command_handler_to_exec_params_uses_selected_environment() {
let (session, turn_context) = make_session_and_context().await;
let (session, mut turn_context) = make_session_and_context().await;
let permission_profile = turn_context.config.permissions.permission_profile().clone();
Arc::make_mut(&mut turn_context.config)
.permissions
.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active(
permission_profile,
ActivePermissionProfile::new("test-profile"),
))
.expect("set active permission profile");

let command = "echo hello".to_string();
let workdir = Some("subdir".to_string());
Expand Down Expand Up @@ -99,10 +111,12 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
PathUri::from_abs_path(&selected_cwd),
Some(selected_shell),
);
let expected_env = create_env(
let mut expected_env = create_env(
&turn_context.config.permissions.shell_environment_policy,
Some(session.thread_id),
);
let active_permission_profile = turn_context.config.permissions.active_permission_profile();
inject_permission_profile_env(&mut expected_env, active_permission_profile.as_ref());

let params = ShellCommandToolCallParams {
command,
Expand All @@ -129,6 +143,12 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
assert_eq!(exec_params.command, expected_command);
assert_eq!(exec_params.cwd, expected_cwd);
assert_eq!(exec_params.env, expected_env);
assert_eq!(
exec_params.env.get(CODEX_PERMISSION_PROFILE_ENV_VAR),
active_permission_profile
.as_ref()
.map(|profile| &profile.id)
);
assert_eq!(exec_params.network, turn_context.network);
assert_eq!(
exec_params.network_environment_id.as_deref(),
Expand Down
18 changes: 14 additions & 4 deletions codex-rs/core/src/tools/runtimes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Module: runtimes
Concrete ToolRuntime implementations for specific tools. Each runtime stays
small and focused and reuses the orchestrator for approvals + sandbox + retry.
*/
use crate::exec_env::CODEX_PERMISSION_PROFILE_ENV_VAR;
use crate::exec_env::CODEX_THREAD_ID_ENV_VAR;
use crate::sandboxing::SandboxPermissions;
use crate::shell::Shell;
Expand Down Expand Up @@ -287,10 +288,14 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot(
.map(|arg| format!(" '{}'", shell_single_quote(arg)))
.collect::<String>();
let mut override_env = explicit_env_overrides.clone();
if let Some(thread_id) = env.get(CODEX_THREAD_ID_ENV_VAR) {
override_env.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.clone());
for key in [CODEX_THREAD_ID_ENV_VAR, CODEX_PERMISSION_PROFILE_ENV_VAR] {
if let Some(value) = env.get(key) {
override_env.insert(key.to_string(), value.clone());
}
}
Comment thread
bolinfest marked this conversation as resolved.
let (override_captures, override_exports) = build_override_exports(&override_env);
// Do not let a snapshot resurrect a stale profile when no named profile is active.
let (override_captures, override_exports) =
build_override_exports(&override_env, &[CODEX_PERMISSION_PROFILE_ENV_VAR]);
let (proxy_captures, proxy_exports) = build_proxy_env_exports();
let runtime_path_prepend_exports =
runtime_path_prepends.shell_exports_after_snapshot(explicit_env_overrides);
Expand All @@ -313,13 +318,18 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot(
vec![shell_path.to_string(), "-c".to_string(), rewritten_script]
}

fn build_override_exports(explicit_env_overrides: &HashMap<String, String>) -> (String, String) {
fn build_override_exports(
explicit_env_overrides: &HashMap<String, String>,
restore_even_when_absent: &[&str],
) -> (String, String) {
let mut keys = explicit_env_overrides
.keys()
.map(String::as_str)
.chain(restore_even_when_absent.iter().copied())
.filter(|key| is_valid_shell_variable_name(key))
.collect::<Vec<_>>();
keys.sort_unstable();
keys.dedup();

build_override_exports_for_keys("__CODEX_SNAPSHOT_OVERRIDE", &keys)
}
Expand Down
72 changes: 72 additions & 0 deletions codex-rs/core/src/tools/runtimes/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,78 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_codex_thread_id_from_env() {
assert_eq!(String::from_utf8_lossy(&output.stdout), "nested-thread");
}

#[test]
fn maybe_wrap_shell_lc_with_snapshot_restores_permission_profile_from_env() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
std::fs::write(
&snapshot_path,
"# Snapshot file\nexport CODEX_PERMISSION_PROFILE='parent-profile'\n",
)
.expect("write snapshot");
let (session_shell, shell_snapshot) =
shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs());
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"printenv CODEX_PERMISSION_PROFILE".to_string(),
];
let env = HashMap::from([(
CODEX_PERMISSION_PROFILE_ENV_VAR.to_string(),
"current-profile".to_string(),
)]);
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
Some(&shell_snapshot),
&HashMap::new(),
&env,
&RuntimePathPrepends::default(),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env(CODEX_PERMISSION_PROFILE_ENV_VAR, "current-profile")
.output()
.expect("run rewritten command");

assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(String::from_utf8_lossy(&output.stdout), "current-profile\n");
}

#[test]
fn maybe_wrap_shell_lc_with_snapshot_unsets_absent_permission_profile() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
std::fs::write(
&snapshot_path,
"# Snapshot file\nexport CODEX_PERMISSION_PROFILE='stale-profile'\n",
)
.expect("write snapshot");
let (session_shell, shell_snapshot) =
shell_with_snapshot(ShellType::Bash, "/bin/bash", snapshot_path.abs());
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"printenv CODEX_PERMISSION_PROFILE".to_string(),
];
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
Some(&shell_snapshot),
&HashMap::new(),
&HashMap::new(),
&RuntimePathPrepends::default(),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env_remove(CODEX_PERMISSION_PROFILE_ENV_VAR)
.output()
.expect("run rewritten command");

assert_eq!(output.status.code(), Some(1));
assert_eq!(output.stdout, b"");
}

#[test]
fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() {
let dir = tempdir().expect("create temp dir");
Expand Down
25 changes: 18 additions & 7 deletions codex-rs/core/src/unified_exec/process_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ use tokio_util::sync::CancellationToken;
use uuid::Uuid;

use crate::codex_thread::BackgroundTerminalInfo;
use crate::exec_env::CODEX_PERMISSION_PROFILE_ENV_VAR;
use crate::exec_env::CODEX_THREAD_ID_ENV_VAR;
use crate::exec_env::create_env;
use crate::exec_env::inject_permission_profile_env;
use crate::exec_policy::ExecApprovalRequest;
use crate::sandboxing::ExecOptions;
use crate::sandboxing::ExecRequest;
Expand Down Expand Up @@ -109,15 +111,19 @@ fn apply_unified_exec_env(mut env: HashMap<String, String>) -> HashMap<String, S
fn exec_env_policy_from_shell_policy(
policy: &ShellEnvironmentPolicy,
) -> codex_exec_server::ExecEnvPolicy {
let mut exclude = policy
.exclude
.iter()
.map(std::string::ToString::to_string)
.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));
codex_exec_server::ExecEnvPolicy {
inherit: policy.inherit.clone(),
ignore_default_excludes: policy.ignore_default_excludes,
exclude: policy
.exclude
.iter()
.map(std::string::ToString::to_string)
.collect(),
r#set: policy.r#set.clone(),
exclude,
r#set,
include_only: policy
.include_only
.iter()
Expand All @@ -132,7 +138,10 @@ fn env_overlay_for_exec_server(
) -> HashMap<String, String> {
request_env
.iter()
.filter(|(key, value)| local_policy_env.get(*key) != Some(*value))
.filter(|(key, value)| {
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 Expand Up @@ -1110,6 +1119,8 @@ impl UnifiedExecProcessManager {
CODEX_THREAD_ID_ENV_VAR.to_string(),
context.session.thread_id.to_string(),
);
let active_permission_profile = context.turn.config.permissions.active_permission_profile();
inject_permission_profile_env(&mut env, active_permission_profile.as_ref());
Comment thread
bolinfest marked this conversation as resolved.
Comment thread
bolinfest marked this conversation as resolved.
let env = apply_unified_exec_env(env);
let exec_server_env_config = ExecServerEnvConfig {
policy: exec_env_policy_from_shell_policy(
Expand Down
Loading
Loading