Skip to content

Commit c2596cc

Browse files
starr-openaicodex
andcommitted
codex: centralize sandbox spawn env preparation
Prepare sandbox env inside the sandbox transform result so core and exec-server do not have to mutate the request at their spawn boundaries. Co-authored-by: Codex <noreply@openai.com>
1 parent 3ef4c50 commit c2596cc

File tree

14 files changed

+143
-114
lines changed

14 files changed

+143
-114
lines changed

codex-rs/core/src/sandboxing/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,11 @@ impl ExecRequest {
8585
}
8686

8787
pub(crate) fn from_sandbox_exec_request(
88-
mut request: SandboxExecRequest,
88+
request: SandboxExecRequest,
8989
options: ExecOptions,
9090
) -> Self {
91-
request.prepare_env_for_spawn();
91+
// SandboxManager::transform prepares spawn env (`CODEX_SANDBOX*`).
92+
// This adapter only attaches core-owned exec options.
9293
let SandboxExecRequest {
9394
command,
9495
cwd,

codex-rs/core/src/tools/runtimes/unified_exec.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,9 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
244244
.as_ref()
245245
.filter(|environment| environment.exec_server_url().is_some())
246246
{
247+
// Let the exec-server host perform sandbox transformation for its platform.
248+
// The local fallback below still transforms in core, where the session's
249+
// linux-sandbox helper path and zsh-fork spawn lifecycle are available.
247250
if let UnifiedExecShellMode::ZshFork(_) = &self.shell_mode {
248251
return Err(ToolError::Rejected(
249252
"unified_exec zsh-fork is not supported when exec_server_url is configured"

codex-rs/core/src/tools/spec_tests.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,6 @@ fn multi_agent_v2_tools_config() -> ToolsConfig {
175175
session_source: SessionSource::Cli,
176176
sandbox_policy: &SandboxPolicy::DangerFullAccess,
177177
windows_sandbox_level: WindowsSandboxLevel::Disabled,
178-
image_generation_tool_auth_allowed: true,
179178
})
180179
}
181180

codex-rs/exec-server/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ The crate exports:
242242
- `RemoteExecServerConnectArgs`
243243
- protocol structs `InitializeParams` and `InitializeResponse`
244244
- `DEFAULT_LISTEN_URL` and `ExecServerListenUrlParseError`
245-
- `run_main_with_listen_url()`
245+
- `run_main_with_runtime()`
246246
- `run_main()` for embedding the websocket server in a binary
247247

248248
## Example session

codex-rs/exec-server/src/bin/codex-exec-server.rs

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clap::Parser;
22

3-
#[cfg(target_os = "linux")]
3+
use std::future::Future;
44
use std::path::PathBuf;
55

66
#[cfg(target_os = "linux")]
@@ -18,6 +18,26 @@ struct ExecServerArgs {
1818
}
1919

2020
fn main() -> anyhow::Result<()> {
21+
linux_sandbox_arg0_dispatch_or_else(run_main)
22+
}
23+
24+
async fn run_main(codex_linux_sandbox_exe: Option<PathBuf>) -> anyhow::Result<()> {
25+
let args = ExecServerArgs::parse();
26+
let runtime = codex_exec_server::ExecServerRuntimeConfig::new(codex_linux_sandbox_exe);
27+
codex_exec_server::run_main_with_runtime(&args.listen, runtime)
28+
.await
29+
.map_err(|err| anyhow::Error::msg(err.to_string()))?;
30+
Ok(())
31+
}
32+
33+
// Keep this standalone-server wrapper local instead of using
34+
// codex_arg0::arg0_dispatch_or_else: codex-arg0 owns CLI aliases such as
35+
// apply_patch, and depends on codex-exec-server to apply patches.
36+
fn linux_sandbox_arg0_dispatch_or_else<F, Fut>(main_fn: F) -> anyhow::Result<()>
37+
where
38+
F: FnOnce(Option<PathBuf>) -> Fut,
39+
Fut: Future<Output = anyhow::Result<()>>,
40+
{
2141
dispatch_linux_sandbox_arg0();
2242

2343
let _linux_sandbox_alias = LinuxSandboxAlias::create();
@@ -31,14 +51,7 @@ fn main() -> anyhow::Result<()> {
3151
let runtime = tokio::runtime::Builder::new_multi_thread()
3252
.enable_all()
3353
.build()?;
34-
runtime.block_on(async move {
35-
let args = ExecServerArgs::parse();
36-
let runtime = codex_exec_server::ExecServerRuntimeConfig::new(codex_linux_sandbox_exe);
37-
codex_exec_server::run_main_with_runtime(&args.listen, runtime)
38-
.await
39-
.map_err(|err| anyhow::Error::msg(err.to_string()))?;
40-
Ok(())
41-
})
54+
runtime.block_on(main_fn(codex_linux_sandbox_exe))
4255
}
4356

4457
#[cfg(target_os = "linux")]

codex-rs/exec-server/src/environment.rs

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use tokio::sync::OnceCell;
44

55
use crate::ExecServerClient;
66
use crate::ExecServerError;
7+
use crate::ExecServerRuntimeConfig;
78
use crate::RemoteExecServerConnectArgs;
89
use crate::file_system::ExecutorFileSystem;
910
use crate::local_file_system::LocalFileSystem;
@@ -22,6 +23,7 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL";
2223
pub struct EnvironmentManager {
2324
exec_server_url: Option<String>,
2425
disabled: bool,
26+
runtime: ExecServerRuntimeConfig,
2527
current_environment: OnceCell<Option<Arc<Environment>>>,
2628
}
2729

@@ -34,17 +36,33 @@ impl Default for EnvironmentManager {
3436
impl EnvironmentManager {
3537
/// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value.
3638
pub fn new(exec_server_url: Option<String>) -> Self {
39+
Self::new_with_runtime(exec_server_url, ExecServerRuntimeConfig::detect())
40+
}
41+
42+
/// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and the
43+
/// runtime resources available in this client process.
44+
pub fn new_with_runtime(
45+
exec_server_url: Option<String>,
46+
runtime: ExecServerRuntimeConfig,
47+
) -> Self {
3748
let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url);
3849
Self {
3950
exec_server_url,
4051
disabled,
52+
runtime,
4153
current_environment: OnceCell::new(),
4254
}
4355
}
4456

4557
/// Builds a manager from process environment variables.
4658
pub fn from_env() -> Self {
47-
Self::new(std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok())
59+
Self::from_env_with_runtime(ExecServerRuntimeConfig::detect())
60+
}
61+
62+
/// Builds a manager from process environment variables and explicit local
63+
/// runtime resources.
64+
pub fn from_env_with_runtime(runtime: ExecServerRuntimeConfig) -> Self {
65+
Self::new_with_runtime(std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), runtime)
4866
}
4967

5068
/// Builds a manager from the currently selected environment, or from the
@@ -54,11 +72,13 @@ impl EnvironmentManager {
5472
Some(environment) => Self {
5573
exec_server_url: environment.exec_server_url().map(str::to_owned),
5674
disabled: false,
75+
runtime: ExecServerRuntimeConfig::detect(),
5776
current_environment: OnceCell::new(),
5877
},
5978
None => Self {
6079
exec_server_url: None,
6180
disabled: true,
81+
runtime: ExecServerRuntimeConfig::detect(),
6282
current_environment: OnceCell::new(),
6383
},
6484
}
@@ -82,7 +102,11 @@ impl EnvironmentManager {
82102
Ok(None)
83103
} else {
84104
Ok(Some(Arc::new(
85-
Environment::create(self.exec_server_url.clone()).await?,
105+
Environment::create_with_runtime(
106+
self.exec_server_url.clone(),
107+
self.runtime.clone(),
108+
)
109+
.await?,
86110
)))
87111
}
88112
})
@@ -132,6 +156,15 @@ impl std::fmt::Debug for Environment {
132156
impl Environment {
133157
/// Builds an environment from the raw `CODEX_EXEC_SERVER_URL` value.
134158
pub async fn create(exec_server_url: Option<String>) -> Result<Self, ExecServerError> {
159+
Self::create_with_runtime(exec_server_url, ExecServerRuntimeConfig::detect()).await
160+
}
161+
162+
/// Builds an environment from the raw `CODEX_EXEC_SERVER_URL` value and
163+
/// runtime resources available when spawning local processes.
164+
pub async fn create_with_runtime(
165+
exec_server_url: Option<String>,
166+
runtime: ExecServerRuntimeConfig,
167+
) -> Result<Self, ExecServerError> {
135168
let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url);
136169
if disabled {
137170
return Err(ExecServerError::Protocol(
@@ -161,7 +194,7 @@ impl Environment {
161194
));
162195
}
163196
None => {
164-
let local_process = LocalProcess::default();
197+
let local_process = LocalProcess::default_with_runtime(runtime);
165198
local_process
166199
.initialize()
167200
.map_err(|err| ExecServerError::Protocol(err.message))?;

codex-rs/exec-server/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,4 @@ pub use protocol::WriteStatus;
6666
pub use server::DEFAULT_LISTEN_URL;
6767
pub use server::ExecServerListenUrlParseError;
6868
pub use server::run_main;
69-
pub use server::run_main_with_listen_url;
7069
pub use server::run_main_with_runtime;

codex-rs/exec-server/src/local_process.rs

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use std::collections::HashMap;
22
use std::collections::VecDeque;
3-
use std::path::Path;
43
use std::path::PathBuf;
54
use std::sync::Arc;
65
use std::sync::atomic::AtomicBool;
@@ -123,18 +122,20 @@ struct StartedProcess {
123122

124123
impl Default for LocalProcess {
125124
fn default() -> Self {
125+
Self::default_with_runtime(ExecServerRuntimeConfig::detect())
126+
}
127+
}
128+
129+
impl LocalProcess {
130+
pub(crate) fn default_with_runtime(runtime: ExecServerRuntimeConfig) -> Self {
126131
let (outgoing_tx, mut outgoing_rx) =
127132
mpsc::channel::<RpcServerOutboundMessage>(NOTIFICATION_CHANNEL_CAPACITY);
128133
tokio::spawn(async move { while outgoing_rx.recv().await.is_some() {} });
129-
Self::new(RpcNotificationSender::new(outgoing_tx))
134+
Self::new_with_runtime(RpcNotificationSender::new(outgoing_tx), runtime)
130135
}
131136
}
132137

133138
impl LocalProcess {
134-
pub(crate) fn new(notifications: RpcNotificationSender) -> Self {
135-
Self::new_with_runtime(notifications, ExecServerRuntimeConfig::detect())
136-
}
137-
138139
pub(crate) fn new_with_runtime(
139140
notifications: RpcNotificationSender,
140141
runtime: ExecServerRuntimeConfig,
@@ -502,36 +503,23 @@ impl ExecProcess for LocalExecProcess {
502503
}
503504
}
504505

505-
fn build_sandbox_command(
506-
argv: &[String],
507-
cwd: &Path,
508-
env: &HashMap<String, String>,
509-
additional_permissions: Option<codex_protocol::models::PermissionProfile>,
510-
) -> Result<SandboxCommand, JSONRPCErrorError> {
511-
let (program, args) = argv
506+
fn prepare_exec_launch(
507+
params: &ExecParams,
508+
runtime: &ExecServerRuntimeConfig,
509+
) -> Result<SandboxExecRequest, JSONRPCErrorError> {
510+
let (program, args) = params
511+
.argv
512512
.split_first()
513513
.ok_or_else(|| invalid_params("argv must not be empty".to_string()))?;
514-
Ok(SandboxCommand {
514+
let command = SandboxCommand {
515515
program: program.clone().into(),
516516
args: args.to_vec(),
517-
cwd: AbsolutePathBuf::try_from(cwd)
517+
cwd: AbsolutePathBuf::try_from(params.cwd.as_path())
518518
.map_err(|err| invalid_params(format!("cwd must be absolute: {err}")))?,
519-
env: env.clone(),
520-
additional_permissions,
521-
})
522-
}
523-
524-
fn prepare_exec_launch(
525-
params: &ExecParams,
526-
runtime: &ExecServerRuntimeConfig,
527-
) -> Result<SandboxExecRequest, JSONRPCErrorError> {
528-
let command = build_sandbox_command(
529-
&params.argv,
530-
params.cwd.as_path(),
531-
&params.env,
532-
params.sandbox.additional_permissions.clone(),
533-
)?;
534-
let mut launch = params
519+
env: params.env.clone(),
520+
additional_permissions: params.sandbox.additional_permissions.clone(),
521+
};
522+
params
535523
.sandbox
536524
.transform(
537525
command,
@@ -541,9 +529,7 @@ fn prepare_exec_launch(
541529
None,
542530
runtime.codex_linux_sandbox_exe.as_deref(),
543531
)
544-
.map_err(|err| internal_error(format!("failed to build sandbox launch: {err}")))?;
545-
launch.prepare_env_for_spawn();
546-
Ok(launch)
532+
.map_err(|err| internal_error(format!("failed to build sandbox launch: {err}")))
547533
}
548534

549535
impl LocalProcess {

codex-rs/exec-server/src/server.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,7 @@ pub use transport::DEFAULT_LISTEN_URL;
1010
pub use transport::ExecServerListenUrlParseError;
1111

1212
pub async fn run_main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
13-
run_main_with_listen_url(DEFAULT_LISTEN_URL).await
14-
}
15-
16-
pub async fn run_main_with_listen_url(
17-
listen_url: &str,
18-
) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
19-
transport::run_transport(listen_url, crate::ExecServerRuntimeConfig::detect()).await
13+
run_main_with_runtime(DEFAULT_LISTEN_URL, crate::ExecServerRuntimeConfig::detect()).await
2014
}
2115

2216
pub async fn run_main_with_runtime(

codex-rs/sandboxing/src/manager.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ impl SandboxManager {
291291
SandboxType::WindowsRestrictedToken => (os_argv_to_strings(argv), None),
292292
};
293293

294-
Ok(SandboxExecRequest {
294+
let mut request = SandboxExecRequest {
295295
command: argv,
296296
cwd: command.cwd,
297297
env: command.env,
@@ -303,7 +303,9 @@ impl SandboxManager {
303303
file_system_sandbox_policy: effective_file_system_policy,
304304
network_sandbox_policy: effective_network_policy,
305305
arg0: arg0_override,
306-
})
306+
};
307+
request.prepare_env_for_spawn();
308+
Ok(request)
307309
}
308310
}
309311

0 commit comments

Comments
 (0)