mirror of
https://github.com/openai/codex.git
synced 2026-05-29 15:30:22 +00:00
codex: share turn environment selection for process tools
Move tool environment selection onto ResolvedTurnEnvironments so follow-up tool slices can share lookup behavior without keeping a handler-global helper. Update exec_command and shell_command routing to use the shared selection method, and restore focused coverage for resolving path permissions against the selected workdir. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -41,6 +41,16 @@ impl ResolvedTurnEnvironments {
|
||||
self.turn_environments.first()
|
||||
}
|
||||
|
||||
pub(crate) fn get(&self, environment_id: &str) -> Option<&TurnEnvironment> {
|
||||
self.turn_environments
|
||||
.iter()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
}
|
||||
|
||||
pub(crate) fn select(&self, environment_id: Option<&str>) -> Option<&TurnEnvironment> {
|
||||
environment_id.map_or_else(|| self.primary(), |environment_id| self.get(environment_id))
|
||||
}
|
||||
|
||||
pub(crate) fn primary_environment(&self) -> Option<Arc<codex_exec_server::Environment>> {
|
||||
self.primary()
|
||||
.map(|environment| Arc::clone(&environment.environment))
|
||||
@@ -177,4 +187,35 @@ mod tests {
|
||||
"local"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolved_environment_selections_selects_by_id_or_primary() {
|
||||
let cwd = AbsolutePathBuf::current_dir().expect("cwd");
|
||||
let manager = EnvironmentManager::default_for_tests();
|
||||
|
||||
let resolved = resolve_environment_selections(
|
||||
&manager,
|
||||
&[TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd,
|
||||
}],
|
||||
)
|
||||
.expect("environment selections should resolve");
|
||||
|
||||
assert_eq!(
|
||||
resolved
|
||||
.select(/*environment_id*/ None)
|
||||
.expect("primary environment")
|
||||
.environment_id,
|
||||
"local"
|
||||
);
|
||||
assert_eq!(
|
||||
resolved
|
||||
.select(Some("local"))
|
||||
.expect("selected environment")
|
||||
.environment_id,
|
||||
"local"
|
||||
);
|
||||
assert!(resolved.select(Some("unknown")).is_none());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ use crate::exec::execute_exec_request;
|
||||
use crate::spawn::CODEX_SANDBOX_ENV_VAR;
|
||||
use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicy;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::exec_output::ExecToolCallOutput;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
@@ -41,6 +42,43 @@ pub(crate) struct ExecServerEnvConfig {
|
||||
pub(crate) local_policy_env: HashMap<String, String>,
|
||||
}
|
||||
|
||||
impl ExecServerEnvConfig {
|
||||
pub(crate) fn from_shell_policy(
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
local_policy_env: HashMap<String, String>,
|
||||
) -> Self {
|
||||
Self {
|
||||
policy: 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(),
|
||||
include_only: policy
|
||||
.include_only
|
||||
.iter()
|
||||
.map(std::string::ToString::to_string)
|
||||
.collect(),
|
||||
},
|
||||
local_policy_env,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn env_overlay(
|
||||
&self,
|
||||
request_env: &HashMap<String, String>,
|
||||
) -> HashMap<String, String> {
|
||||
request_env
|
||||
.iter()
|
||||
.filter(|(key, value)| self.local_policy_env.get(*key) != Some(*value))
|
||||
.map(|(key, value)| (key.clone(), value.clone()))
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct ExecRequest {
|
||||
pub command: Vec<String>,
|
||||
@@ -175,3 +213,74 @@ pub async fn execute_exec_request_with_after_spawn(
|
||||
) -> codex_protocol::error::Result<ExecToolCallOutput> {
|
||||
execute_exec_request(exec_request, stdout_stream, after_spawn).await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::config_types::EnvironmentVariablePattern;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicyInherit;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn exec_server_env_config_from_shell_policy_preserves_policy_fields() {
|
||||
let policy = ShellEnvironmentPolicy {
|
||||
inherit: ShellEnvironmentPolicyInherit::Core,
|
||||
ignore_default_excludes: false,
|
||||
exclude: vec![EnvironmentVariablePattern::new_case_insensitive("SECRET_*")],
|
||||
r#set: HashMap::from([("FROM_POLICY".to_string(), "set-value".to_string())]),
|
||||
include_only: vec![EnvironmentVariablePattern::new_case_insensitive("*PATH")],
|
||||
use_profile: true,
|
||||
};
|
||||
let local_policy_env = HashMap::from([
|
||||
("PATH".to_string(), "/usr/bin".to_string()),
|
||||
("FROM_POLICY".to_string(), "set-value".to_string()),
|
||||
]);
|
||||
|
||||
let config = ExecServerEnvConfig::from_shell_policy(&policy, local_policy_env.clone());
|
||||
|
||||
assert_eq!(config.policy.inherit, ShellEnvironmentPolicyInherit::Core);
|
||||
assert!(!config.policy.ignore_default_excludes);
|
||||
assert_eq!(
|
||||
config.policy.exclude,
|
||||
vec![EnvironmentVariablePattern::new_case_insensitive("SECRET_*")]
|
||||
);
|
||||
assert_eq!(config.policy.r#set, policy.r#set);
|
||||
assert_eq!(
|
||||
config.policy.include_only,
|
||||
vec![EnvironmentVariablePattern::new_case_insensitive("*PATH")]
|
||||
);
|
||||
assert_eq!(config.local_policy_env, local_policy_env);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_server_env_config_env_overlay_keeps_only_runtime_changes() {
|
||||
let config = ExecServerEnvConfig {
|
||||
policy: codex_exec_server::ExecEnvPolicy {
|
||||
inherit: ShellEnvironmentPolicyInherit::Core,
|
||||
ignore_default_excludes: false,
|
||||
exclude: Vec::new(),
|
||||
r#set: HashMap::new(),
|
||||
include_only: Vec::new(),
|
||||
},
|
||||
local_policy_env: HashMap::from([
|
||||
("HOME".to_string(), "/client-home".to_string()),
|
||||
("PATH".to_string(), "/client-path".to_string()),
|
||||
("SHELL_SET".to_string(), "policy".to_string()),
|
||||
]),
|
||||
};
|
||||
let request_env = HashMap::from([
|
||||
("HOME".to_string(), "/client-home".to_string()),
|
||||
("PATH".to_string(), "/sandbox-path".to_string()),
|
||||
("SHELL_SET".to_string(), "policy".to_string()),
|
||||
("CODEX_THREAD_ID".to_string(), "thread-1".to_string()),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
config.env_overlay(&request_env),
|
||||
HashMap::from([
|
||||
("PATH".to_string(), "/sandbox-path".to_string()),
|
||||
("CODEX_THREAD_ID".to_string(), "thread-1".to_string()),
|
||||
])
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,10 +27,10 @@ use serde::Deserialize;
|
||||
use serde_json::Value;
|
||||
use std::path::Path;
|
||||
|
||||
use crate::environment_selection::ResolvedTurnEnvironments;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
pub(crate) use crate::tools::code_mode::CodeModeExecuteHandler;
|
||||
pub(crate) use crate::tools::code_mode::CodeModeWaitHandler;
|
||||
@@ -93,25 +93,39 @@ fn resolve_workdir_base_path(
|
||||
.map_or_else(|| default_cwd.clone(), |workdir| default_cwd.join(workdir)))
|
||||
}
|
||||
|
||||
fn resolve_tool_environment<'a>(
|
||||
turn: &'a TurnContext,
|
||||
environment_id: Option<&str>,
|
||||
) -> Result<Option<&'a TurnEnvironment>, FunctionCallError> {
|
||||
environment_id.map_or_else(
|
||||
|| Ok(turn.environments.primary()),
|
||||
|environment_id| {
|
||||
turn.environments
|
||||
.turn_environments
|
||||
.iter()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.map(Some)
|
||||
.ok_or_else(|| {
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
"unknown turn environment id `{environment_id}`"
|
||||
))
|
||||
})
|
||||
},
|
||||
)
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct EnvironmentWorkdirArgs {
|
||||
#[serde(default)]
|
||||
environment_id: Option<String>,
|
||||
// Keep this raw until after environment selection; relative paths must be
|
||||
// resolved against the selected environment cwd, not the process cwd.
|
||||
#[serde(default)]
|
||||
workdir: Option<String>,
|
||||
}
|
||||
|
||||
/// Parses optional `environment_id` and `workdir` fields from tool arguments.
|
||||
///
|
||||
/// Returns the selected turn environment plus the effective execution cwd. The
|
||||
/// returned cwd is `turn_environment.cwd.join(workdir)` when `workdir` is
|
||||
/// provided and non-empty, otherwise it is the selected `turn_environment.cwd`.
|
||||
fn resolve_environment_workdir_target(
|
||||
arguments: &str,
|
||||
environments: &ResolvedTurnEnvironments,
|
||||
) -> Result<Option<(TurnEnvironment, AbsolutePathBuf)>, FunctionCallError> {
|
||||
let target_args: EnvironmentWorkdirArgs = parse_arguments(arguments)?;
|
||||
let Some(turn_environment) = environments.select(target_args.environment_id.as_deref()) else {
|
||||
return Ok(None);
|
||||
};
|
||||
let cwd = target_args
|
||||
.workdir
|
||||
.as_deref()
|
||||
.filter(|workdir| !workdir.is_empty())
|
||||
.map_or_else(
|
||||
|| turn_environment.cwd.clone(),
|
||||
|workdir| turn_environment.cwd.join(workdir),
|
||||
);
|
||||
|
||||
Ok(Some((turn_environment.clone(), cwd)))
|
||||
}
|
||||
|
||||
/// Validates feature/policy constraints for `with_additional_permissions` and
|
||||
|
||||
@@ -26,7 +26,7 @@ use crate::tools::handlers::implicit_granted_permissions;
|
||||
use crate::tools::handlers::normalize_and_validate_additional_permissions;
|
||||
use crate::tools::handlers::parse_arguments;
|
||||
use crate::tools::handlers::parse_arguments_with_base_path;
|
||||
use crate::tools::handlers::resolve_tool_environment;
|
||||
use crate::tools::handlers::resolve_environment_workdir_target;
|
||||
use crate::tools::handlers::resolve_workdir_base_path;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::orchestrator::ToolOrchestrator;
|
||||
@@ -39,7 +39,6 @@ use crate::tools::runtimes::shell::ShellRuntime;
|
||||
use crate::tools::runtimes::shell::ShellRuntimeBackend;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicy;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
use codex_shell_command::is_safe_command::is_known_safe_command;
|
||||
@@ -94,9 +93,7 @@ struct RunExecLikeArgs {
|
||||
tool_name: String,
|
||||
exec_params: ExecParams,
|
||||
hook_command: String,
|
||||
target_environment: Arc<codex_exec_server::Environment>,
|
||||
remote_execution_enabled: bool,
|
||||
exec_server_env_config: Option<crate::sandboxing::ExecServerEnvConfig>,
|
||||
environment: Arc<codex_exec_server::Environment>,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
prefix_rule: Option<Vec<String>>,
|
||||
session: Arc<crate::session::session::Session>,
|
||||
@@ -107,16 +104,6 @@ struct RunExecLikeArgs {
|
||||
shell_runtime_backend: ShellRuntimeBackend,
|
||||
}
|
||||
|
||||
#[derive(Debug, serde::Deserialize)]
|
||||
struct ShellCommandEnvironmentArgs {
|
||||
#[serde(default)]
|
||||
environment_id: Option<String>,
|
||||
// Keep this raw until after environment selection; relative paths must be
|
||||
// resolved against the selected environment cwd, not the process cwd.
|
||||
#[serde(default)]
|
||||
workdir: Option<String>,
|
||||
}
|
||||
|
||||
impl ShellHandler {
|
||||
fn to_exec_params(
|
||||
params: &ShellToolCallParams,
|
||||
@@ -197,36 +184,6 @@ impl ShellCommandHandler {
|
||||
}
|
||||
}
|
||||
|
||||
fn exec_server_env_policy_from_shell_policy(
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
) -> codex_exec_server::ExecEnvPolicy {
|
||||
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(),
|
||||
include_only: policy
|
||||
.include_only
|
||||
.iter()
|
||||
.map(std::string::ToString::to_string)
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
fn exec_server_env_config(
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
local_policy_env: &std::collections::HashMap<String, String>,
|
||||
) -> crate::sandboxing::ExecServerEnvConfig {
|
||||
crate::sandboxing::ExecServerEnvConfig {
|
||||
policy: exec_server_env_policy_from_shell_policy(policy),
|
||||
local_policy_env: local_policy_env.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
impl From<ShellCommandBackendConfig> for ShellCommandHandler {
|
||||
fn from(config: ShellCommandBackendConfig) -> Self {
|
||||
let backend = match config {
|
||||
@@ -302,7 +259,7 @@ impl ToolHandler for ShellHandler {
|
||||
tool_name: "shell".to_string(),
|
||||
exec_params,
|
||||
hook_command: codex_shell_command::parse_command::shlex_join(¶ms.command),
|
||||
target_environment: turn
|
||||
environment: turn
|
||||
.environments
|
||||
.primary()
|
||||
.map(|environment| Arc::clone(&environment.environment))
|
||||
@@ -311,8 +268,6 @@ impl ToolHandler for ShellHandler {
|
||||
"shell is unavailable in this session".to_string(),
|
||||
)
|
||||
})?,
|
||||
remote_execution_enabled: false,
|
||||
exec_server_env_config: None,
|
||||
additional_permissions: params.additional_permissions.clone(),
|
||||
prefix_rule,
|
||||
session,
|
||||
@@ -391,7 +346,7 @@ impl ToolHandler for ContainerExecHandler {
|
||||
tool_name: "container.exec".to_string(),
|
||||
exec_params,
|
||||
hook_command: codex_shell_command::parse_command::shlex_join(¶ms.command),
|
||||
target_environment: turn
|
||||
environment: turn
|
||||
.environments
|
||||
.primary()
|
||||
.map(|environment| Arc::clone(&environment.environment))
|
||||
@@ -400,8 +355,6 @@ impl ToolHandler for ContainerExecHandler {
|
||||
"shell is unavailable in this session".to_string(),
|
||||
)
|
||||
})?,
|
||||
remote_execution_enabled: false,
|
||||
exec_server_env_config: None,
|
||||
additional_permissions: params.additional_permissions.clone(),
|
||||
prefix_rule,
|
||||
session,
|
||||
@@ -483,7 +436,7 @@ impl ToolHandler for LocalShellHandler {
|
||||
tool_name: "local_shell".to_string(),
|
||||
exec_params,
|
||||
hook_command: codex_shell_command::parse_command::shlex_join(¶ms.command),
|
||||
target_environment: turn
|
||||
environment: turn
|
||||
.environments
|
||||
.primary()
|
||||
.map(|environment| Arc::clone(&environment.environment))
|
||||
@@ -492,8 +445,6 @@ impl ToolHandler for LocalShellHandler {
|
||||
"shell is unavailable in this session".to_string(),
|
||||
)
|
||||
})?,
|
||||
remote_execution_enabled: false,
|
||||
exec_server_env_config: None,
|
||||
additional_permissions: None,
|
||||
prefix_rule: None,
|
||||
session,
|
||||
@@ -604,22 +555,13 @@ impl ToolHandler for ShellCommandHandler {
|
||||
)));
|
||||
};
|
||||
|
||||
let environment_args: ShellCommandEnvironmentArgs = parse_arguments(&arguments)?;
|
||||
let Some(turn_environment) =
|
||||
resolve_tool_environment(turn.as_ref(), environment_args.environment_id.as_deref())?
|
||||
let Some((turn_environment, cwd)) =
|
||||
resolve_environment_workdir_target(&arguments, &turn.environments)?
|
||||
else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"shell is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let cwd = environment_args
|
||||
.workdir
|
||||
.as_deref()
|
||||
.filter(|workdir| !workdir.is_empty())
|
||||
.map_or_else(
|
||||
|| turn_environment.cwd.clone(),
|
||||
|workdir| turn_environment.cwd.join(workdir),
|
||||
);
|
||||
let params: ShellCommandToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
|
||||
maybe_emit_implicit_skill_invocation(
|
||||
session.as_ref(),
|
||||
@@ -638,17 +580,11 @@ impl ToolHandler for ShellCommandHandler {
|
||||
cwd,
|
||||
turn.tools_config.allow_login_shell,
|
||||
)?;
|
||||
let exec_server_env_config = turn_environment
|
||||
.environment
|
||||
.is_remote()
|
||||
.then(|| exec_server_env_config(&turn.shell_environment_policy, &exec_params.env));
|
||||
ShellHandler::run_exec_like(RunExecLikeArgs {
|
||||
tool_name: self.tool_name().display(),
|
||||
exec_params,
|
||||
hook_command: params.command,
|
||||
target_environment: Arc::clone(&turn_environment.environment),
|
||||
remote_execution_enabled: true,
|
||||
exec_server_env_config,
|
||||
environment: Arc::clone(&turn_environment.environment),
|
||||
additional_permissions: params.additional_permissions.clone(),
|
||||
prefix_rule,
|
||||
session,
|
||||
@@ -668,9 +604,7 @@ impl ShellHandler {
|
||||
tool_name,
|
||||
exec_params,
|
||||
hook_command,
|
||||
target_environment,
|
||||
remote_execution_enabled,
|
||||
exec_server_env_config,
|
||||
environment,
|
||||
additional_permissions,
|
||||
prefix_rule,
|
||||
session,
|
||||
@@ -682,7 +616,8 @@ impl ShellHandler {
|
||||
} = args;
|
||||
|
||||
let mut exec_params = exec_params;
|
||||
let fs = target_environment.get_filesystem();
|
||||
let fs = environment.get_filesystem();
|
||||
let local_policy_env = exec_params.env.clone();
|
||||
|
||||
let dependency_env = session.dependency_env().await;
|
||||
if !dependency_env.is_empty() {
|
||||
@@ -801,11 +736,15 @@ impl ShellHandler {
|
||||
command: exec_params.command.clone(),
|
||||
hook_command,
|
||||
cwd: exec_params.cwd.clone(),
|
||||
environment: target_environment,
|
||||
remote_execution_enabled,
|
||||
exec_server_env_config: environment.is_remote().then(|| {
|
||||
crate::sandboxing::ExecServerEnvConfig::from_shell_policy(
|
||||
&turn.shell_environment_policy,
|
||||
local_policy_env,
|
||||
)
|
||||
}),
|
||||
environment,
|
||||
timeout_ms: exec_params.expiration.timeout_ms(),
|
||||
env: exec_params.env.clone(),
|
||||
exec_server_env_config,
|
||||
explicit_env_overrides,
|
||||
network: exec_params.network.clone(),
|
||||
sandbox_permissions: effective_additional_permissions.sandbox_permissions,
|
||||
|
||||
@@ -13,7 +13,7 @@ use crate::tools::handlers::implicit_granted_permissions;
|
||||
use crate::tools::handlers::normalize_and_validate_additional_permissions;
|
||||
use crate::tools::handlers::parse_arguments;
|
||||
use crate::tools::handlers::parse_arguments_with_base_path;
|
||||
use crate::tools::handlers::resolve_tool_environment;
|
||||
use crate::tools::handlers::resolve_environment_workdir_target;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::registry::PostToolUsePayload;
|
||||
use crate::tools::registry::PreToolUsePayload;
|
||||
@@ -69,16 +69,6 @@ pub(crate) struct ExecCommandArgs {
|
||||
prefix_rule: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct ExecCommandEnvironmentArgs {
|
||||
#[serde(default)]
|
||||
environment_id: Option<String>,
|
||||
// Keep this raw until after environment selection; relative paths must be
|
||||
// resolved against the selected environment cwd, not the process cwd.
|
||||
#[serde(default)]
|
||||
workdir: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct WriteStdinArgs {
|
||||
// The model is trained on `session_id`.
|
||||
@@ -191,22 +181,13 @@ impl ToolHandler for ExecCommandHandler {
|
||||
|
||||
let manager: &UnifiedExecProcessManager = &session.services.unified_exec_manager;
|
||||
let context = UnifiedExecContext::new(session.clone(), turn.clone(), call_id.clone());
|
||||
let environment_args: ExecCommandEnvironmentArgs = parse_arguments(&arguments)?;
|
||||
let Some(turn_environment) =
|
||||
resolve_tool_environment(turn.as_ref(), environment_args.environment_id.as_deref())?
|
||||
let Some((turn_environment, cwd)) =
|
||||
resolve_environment_workdir_target(&arguments, &turn.environments)?
|
||||
else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"unified exec is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let cwd = environment_args
|
||||
.workdir
|
||||
.as_deref()
|
||||
.filter(|workdir| !workdir.is_empty())
|
||||
.map_or_else(
|
||||
|| turn_environment.cwd.clone(),
|
||||
|workdir| turn_environment.cwd.join(workdir),
|
||||
);
|
||||
let environment = Arc::clone(&turn_environment.environment);
|
||||
let fs = environment.get_filesystem();
|
||||
let args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?;
|
||||
|
||||
@@ -1,10 +1,16 @@
|
||||
use super::*;
|
||||
use crate::shell::default_user_shell;
|
||||
use crate::tools::handlers::parse_arguments_with_base_path;
|
||||
use codex_protocol::models::AdditionalPermissionProfile as PermissionProfile;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_tools::UnifiedExecShellMode;
|
||||
use codex_tools::ZshForkConfig;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use core_test_support::PathExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use std::sync::Arc;
|
||||
use tempfile::tempdir;
|
||||
|
||||
use crate::session::tests::make_session_and_context;
|
||||
use crate::tools::context::ExecCommandToolOutput;
|
||||
@@ -178,6 +184,39 @@ fn test_get_command_ignores_explicit_shell_in_zsh_fork_mode() -> anyhow::Result<
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_command_args_resolve_relative_additional_permissions_against_selected_workdir()
|
||||
-> anyhow::Result<()> {
|
||||
let selected_cwd = tempdir()?;
|
||||
let workdir = selected_cwd.path().join("nested");
|
||||
fs::create_dir_all(&workdir)?;
|
||||
let expected_write = workdir.join("relative-write.txt");
|
||||
let json = r#"{
|
||||
"cmd": "echo hello",
|
||||
"workdir": "nested",
|
||||
"additional_permissions": {
|
||||
"file_system": {
|
||||
"write": ["./relative-write.txt"]
|
||||
}
|
||||
}
|
||||
}"#;
|
||||
|
||||
let target_cwd = selected_cwd.path().abs().join("nested");
|
||||
let args: ExecCommandArgs = parse_arguments_with_base_path(json, &target_cwd)?;
|
||||
|
||||
assert_eq!(
|
||||
args.additional_permissions,
|
||||
Some(PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
/*read*/ None,
|
||||
Some(vec![expected_write.abs()]),
|
||||
)),
|
||||
..Default::default()
|
||||
})
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_command_pre_tool_use_payload_uses_raw_command() {
|
||||
let payload = ToolPayload::Function {
|
||||
|
||||
@@ -69,7 +69,6 @@ pub struct ShellRequest {
|
||||
pub hook_command: String,
|
||||
pub cwd: AbsolutePathBuf,
|
||||
pub environment: Arc<Environment>,
|
||||
pub remote_execution_enabled: bool,
|
||||
pub timeout_ms: Option<u64>,
|
||||
pub env: HashMap<String, String>,
|
||||
pub exec_server_env_config: Option<crate::sandboxing::ExecServerEnvConfig>,
|
||||
@@ -270,12 +269,6 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
attempt: &SandboxAttempt<'_>,
|
||||
ctx: &ToolCtx,
|
||||
) -> Result<ExecToolCallOutput, ToolError> {
|
||||
if req.environment.is_remote() && !req.remote_execution_enabled {
|
||||
return Err(ToolError::Rejected(
|
||||
"shell execution is unavailable for remote environments".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
let session_shell = ctx.session.user_shell();
|
||||
let managed_network =
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions);
|
||||
@@ -341,14 +334,7 @@ fn exec_server_env_for_request(
|
||||
HashMap<String, String>,
|
||||
) {
|
||||
if let Some(exec_server_env_config) = &request.exec_server_env_config {
|
||||
let env = request
|
||||
.env
|
||||
.iter()
|
||||
.filter(|(key, value)| {
|
||||
exec_server_env_config.local_policy_env.get(*key) != Some(*value)
|
||||
})
|
||||
.map(|(key, value)| (key.clone(), value.clone()))
|
||||
.collect();
|
||||
let env = exec_server_env_config.env_overlay(&request.env);
|
||||
(Some(exec_server_env_config.policy.clone()), env)
|
||||
} else {
|
||||
(None, request.env.clone())
|
||||
|
||||
@@ -50,7 +50,6 @@ use crate::unified_exec::process::OutputBuffer;
|
||||
use crate::unified_exec::process::OutputHandles;
|
||||
use crate::unified_exec::process::SpawnLifecycleHandle;
|
||||
use crate::unified_exec::process::UnifiedExecProcess;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicy;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::SandboxErr;
|
||||
use codex_protocol::protocol::ExecCommandSource;
|
||||
@@ -98,37 +97,6 @@ fn apply_unified_exec_env(mut env: HashMap<String, String>) -> HashMap<String, S
|
||||
env
|
||||
}
|
||||
|
||||
fn exec_env_policy_from_shell_policy(
|
||||
policy: &ShellEnvironmentPolicy,
|
||||
) -> codex_exec_server::ExecEnvPolicy {
|
||||
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(),
|
||||
include_only: policy
|
||||
.include_only
|
||||
.iter()
|
||||
.map(std::string::ToString::to_string)
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
fn env_overlay_for_exec_server(
|
||||
request_env: &HashMap<String, String>,
|
||||
local_policy_env: &HashMap<String, String>,
|
||||
) -> HashMap<String, String> {
|
||||
request_env
|
||||
.iter()
|
||||
.filter(|(key, value)| local_policy_env.get(*key) != Some(*value))
|
||||
.map(|(key, value)| (key.clone(), value.clone()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn exec_server_env_for_request(
|
||||
request: &ExecRequest,
|
||||
) -> (
|
||||
@@ -138,7 +106,7 @@ fn exec_server_env_for_request(
|
||||
if let Some(exec_server_env_config) = &request.exec_server_env_config {
|
||||
(
|
||||
Some(exec_server_env_config.policy.clone()),
|
||||
env_overlay_for_exec_server(&request.env, &exec_server_env_config.local_policy_env),
|
||||
exec_server_env_config.env_overlay(&request.env),
|
||||
)
|
||||
} else {
|
||||
(None, request.env.clone())
|
||||
@@ -990,10 +958,10 @@ impl UnifiedExecProcessManager {
|
||||
context.session.conversation_id.to_string(),
|
||||
);
|
||||
let env = apply_unified_exec_env(env);
|
||||
let exec_server_env_config = ExecServerEnvConfig {
|
||||
policy: exec_env_policy_from_shell_policy(&context.turn.shell_environment_policy),
|
||||
let exec_server_env_config = ExecServerEnvConfig::from_shell_policy(
|
||||
&context.turn.shell_environment_policy,
|
||||
local_policy_env,
|
||||
};
|
||||
);
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = UnifiedExecRuntime::new(
|
||||
self,
|
||||
|
||||
@@ -52,8 +52,19 @@ fn env_overlay_for_exec_server_keeps_runtime_changes_only() {
|
||||
),
|
||||
]);
|
||||
|
||||
let config = ExecServerEnvConfig {
|
||||
policy: codex_exec_server::ExecEnvPolicy {
|
||||
inherit: codex_protocol::config_types::ShellEnvironmentPolicyInherit::Core,
|
||||
ignore_default_excludes: false,
|
||||
exclude: Vec::new(),
|
||||
r#set: HashMap::new(),
|
||||
include_only: Vec::new(),
|
||||
},
|
||||
local_policy_env,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
env_overlay_for_exec_server(&request_env, &local_policy_env),
|
||||
config.env_overlay(&request_env),
|
||||
HashMap::from([
|
||||
("PATH".to_string(), "/sandbox-path".to_string()),
|
||||
("CODEX_THREAD_ID".to_string(), "thread-1".to_string()),
|
||||
|
||||
Reference in New Issue
Block a user