mirror of
https://github.com/openai/codex.git
synced 2026-05-16 17:23:57 +00:00
[codex] Tighten unified exec sandbox setup (#22207)
## Summary - tighten unified exec sandbox initialization - preserve the requested process workdir independently from sandbox setup - add regression coverage for the updated invariant ## Validation - Ran `/tmp/cargo-tools/bin/just fmt`. - Ran the targeted `codex-core` regression test successfully. - Ran `cargo test -p codex-core`; it did not complete cleanly because unrelated existing agent/config-loader tests failed and the run later aborted on a stack overflow in `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed`. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -324,6 +324,7 @@ impl ToolHandler for ExecCommandHandler {
|
||||
yield_time_ms,
|
||||
max_output_tokens: Some(max_output_tokens),
|
||||
cwd,
|
||||
sandbox_cwd: turn_environment.cwd.clone(),
|
||||
environment,
|
||||
network: context.turn.network.clone(),
|
||||
tty,
|
||||
|
||||
@@ -59,6 +59,7 @@ pub struct UnifiedExecRequest {
|
||||
pub hook_command: String,
|
||||
pub process_id: i32,
|
||||
pub cwd: AbsolutePathBuf,
|
||||
pub sandbox_cwd: AbsolutePathBuf,
|
||||
pub environment: Arc<Environment>,
|
||||
pub env: HashMap<String, String>,
|
||||
pub exec_server_env_config: Option<ExecServerEnvConfig>,
|
||||
@@ -217,7 +218,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
|
||||
impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRuntime<'a> {
|
||||
fn sandbox_cwd<'b>(&self, req: &'b UnifiedExecRequest) -> Option<&'b AbsolutePathBuf> {
|
||||
Some(&req.cwd)
|
||||
Some(&req.sandbox_cwd)
|
||||
}
|
||||
|
||||
fn network_approval_spec(
|
||||
@@ -361,7 +362,10 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use codex_exec_server::Environment;
|
||||
use std::time::Duration;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn unified_exec_options_combines_default_timeout_with_network_denial_cancellation() {
|
||||
@@ -384,4 +388,40 @@ mod tests {
|
||||
other => panic!("expected timeout-or-cancellation expiration, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unified_exec_uses_the_trusted_sandbox_cwd() {
|
||||
let cwd_dir = tempdir().expect("create process temp dir");
|
||||
let sandbox_dir = tempdir().expect("create sandbox temp dir");
|
||||
let cwd =
|
||||
AbsolutePathBuf::try_from(cwd_dir.path().to_path_buf()).expect("absolute temp dir");
|
||||
let sandbox_cwd = AbsolutePathBuf::try_from(sandbox_dir.path().to_path_buf())
|
||||
.expect("absolute sandbox temp dir");
|
||||
let manager = UnifiedExecProcessManager::default();
|
||||
let runtime = UnifiedExecRuntime::new(&manager, UnifiedExecShellMode::Direct);
|
||||
let request = UnifiedExecRequest {
|
||||
command: vec!["pwd".to_string()],
|
||||
hook_command: "pwd".to_string(),
|
||||
process_id: 1000,
|
||||
cwd,
|
||||
sandbox_cwd: sandbox_cwd.clone(),
|
||||
environment: Arc::new(Environment::default_for_tests()),
|
||||
env: HashMap::new(),
|
||||
exec_server_env_config: None,
|
||||
explicit_env_overrides: HashMap::new(),
|
||||
network: None,
|
||||
tty: false,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
additional_permissions: None,
|
||||
#[cfg(unix)]
|
||||
additional_permissions_preapproved: false,
|
||||
justification: None,
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
};
|
||||
|
||||
assert_eq!(runtime.sandbox_cwd(&request), Some(&sandbox_cwd));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -92,6 +92,7 @@ pub(crate) struct ExecCommandRequest {
|
||||
pub yield_time_ms: u64,
|
||||
pub max_output_tokens: Option<usize>,
|
||||
pub cwd: AbsolutePathBuf,
|
||||
pub sandbox_cwd: AbsolutePathBuf,
|
||||
pub environment: Arc<Environment>,
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub tty: bool,
|
||||
|
||||
@@ -1028,7 +1028,9 @@ impl UnifiedExecProcessManager {
|
||||
approval_policy: context.turn.approval_policy.value(),
|
||||
permission_profile: context.turn.permission_profile(),
|
||||
file_system_sandbox_policy: &file_system_sandbox_policy,
|
||||
sandbox_cwd: cwd.as_path(),
|
||||
// The process cwd may be model-controlled. Policy resolution
|
||||
// stays anchored to the selected turn environment cwd instead.
|
||||
sandbox_cwd: request.sandbox_cwd.as_path(),
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
@@ -1042,6 +1044,7 @@ impl UnifiedExecProcessManager {
|
||||
hook_command: request.hook_command.clone(),
|
||||
process_id: request.process_id,
|
||||
cwd,
|
||||
sandbox_cwd: request.sandbox_cwd.clone(),
|
||||
environment: Arc::clone(&request.environment),
|
||||
env,
|
||||
exec_server_env_config: Some(exec_server_env_config),
|
||||
|
||||
@@ -176,6 +176,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() {
|
||||
yield_time_ms: 1000,
|
||||
max_output_tokens: None,
|
||||
cwd: turn.cwd.clone(),
|
||||
sandbox_cwd: turn.cwd.clone(),
|
||||
environment: turn
|
||||
.environments
|
||||
.primary_environment()
|
||||
|
||||
Reference in New Issue
Block a user