Skip to content

Commit ce6f444

Browse files
starr-openaicodex
andcommitted
codex: address remaining sandbox review feedback (#16736)
Co-authored-by: Codex <noreply@openai.com>
1 parent b61a0d0 commit ce6f444

File tree

16 files changed

+171
-212
lines changed

16 files changed

+171
-212
lines changed

codex-rs/core/src/exec.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ use codex_protocol::protocol::ExecOutputStream;
4040
use codex_protocol::protocol::SandboxPolicy;
4141
use codex_sandboxing::SandboxCommand;
4242
use codex_sandboxing::SandboxManager;
43-
use codex_sandboxing::SandboxTransformRequest;
4443
use codex_sandboxing::SandboxType;
4544
use codex_sandboxing::SandboxablePreference;
4645
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -284,20 +283,20 @@ pub fn build_exec_request(
284283
capture_policy,
285284
};
286285
let mut exec_req = manager
287-
.transform(SandboxTransformRequest {
286+
.transform(
288287
command,
289-
policy: sandbox_policy,
290-
file_system_policy: file_system_sandbox_policy,
291-
network_policy: network_sandbox_policy,
292-
sandbox: sandbox_type,
288+
sandbox_policy,
289+
file_system_sandbox_policy,
290+
network_sandbox_policy,
291+
sandbox_type,
293292
enforce_managed_network,
294-
network: network.as_ref(),
295-
sandbox_policy_cwd: sandbox_cwd,
296-
codex_linux_sandbox_exe: codex_linux_sandbox_exe.as_ref(),
293+
network.as_ref(),
294+
sandbox_cwd,
295+
codex_linux_sandbox_exe.as_ref(),
297296
use_legacy_landlock,
298297
windows_sandbox_level,
299298
windows_sandbox_private_desktop,
300-
})
299+
)
301300
.map(|request| ExecRequest::from_sandbox_exec_request(request, options))
302301
.map_err(CodexErr::from)?;
303302
exec_req.windows_restricted_token_filesystem_overlay =

codex-rs/core/src/tools/js_repl/mod.rs

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ use crate::tools::ToolRouter;
4343
use crate::tools::context::SharedTurnDiffTracker;
4444
use codex_sandboxing::SandboxCommand;
4545
use codex_sandboxing::SandboxManager;
46-
use codex_sandboxing::SandboxTransformRequest;
4746
use codex_sandboxing::SandboxablePreference;
4847
use codex_tools::ToolSpec;
4948
use codex_utils_output_truncation::TruncationPolicy;
@@ -1059,23 +1058,20 @@ impl JsReplManager {
10591058
capture_policy: ExecCapturePolicy::ShellTool,
10601059
};
10611060
let exec_env = sandbox
1062-
.transform(SandboxTransformRequest {
1061+
.transform(
10631062
command,
1064-
policy: &turn.sandbox_policy,
1065-
file_system_policy: &turn.file_system_sandbox_policy,
1066-
network_policy: turn.network_sandbox_policy,
1067-
sandbox: sandbox_type,
1068-
enforce_managed_network: has_managed_network_requirements,
1069-
network: None,
1070-
sandbox_policy_cwd: &turn.cwd,
1071-
codex_linux_sandbox_exe: turn.codex_linux_sandbox_exe.as_ref(),
1072-
use_legacy_landlock: turn.features.use_legacy_landlock(),
1073-
windows_sandbox_level: turn.windows_sandbox_level,
1074-
windows_sandbox_private_desktop: turn
1075-
.config
1076-
.permissions
1077-
.windows_sandbox_private_desktop,
1078-
})
1063+
&turn.sandbox_policy,
1064+
&turn.file_system_sandbox_policy,
1065+
turn.network_sandbox_policy,
1066+
sandbox_type,
1067+
has_managed_network_requirements,
1068+
None,
1069+
&turn.cwd,
1070+
turn.codex_linux_sandbox_exe.as_ref(),
1071+
turn.features.use_legacy_landlock(),
1072+
turn.windows_sandbox_level,
1073+
turn.config.permissions.windows_sandbox_private_desktop,
1074+
)
10791075
.map(|request| {
10801076
crate::sandboxing::ExecRequest::from_sandbox_exec_request(request, options)
10811077
})

codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ use codex_protocol::protocol::ReviewDecision;
3434
use codex_protocol::protocol::SandboxPolicy;
3535
use codex_sandboxing::SandboxCommand;
3636
use codex_sandboxing::SandboxManager;
37-
use codex_sandboxing::SandboxTransformRequest;
3837
use codex_sandboxing::SandboxType;
3938
use codex_sandboxing::SandboxablePreference;
4039
use codex_shell_command::bash::parse_shell_lc_plain_commands;
@@ -835,20 +834,20 @@ impl CoreShellCommandExecutor {
835834
expiration: ExecExpiration::DefaultTimeout,
836835
capture_policy: ExecCapturePolicy::ShellTool,
837836
};
838-
let exec_request = sandbox_manager.transform(SandboxTransformRequest {
837+
let exec_request = sandbox_manager.transform(
839838
command,
840-
policy: sandbox_policy,
841-
file_system_policy: file_system_sandbox_policy,
842-
network_policy: network_sandbox_policy,
839+
sandbox_policy,
840+
file_system_sandbox_policy,
841+
network_sandbox_policy,
843842
sandbox,
844-
enforce_managed_network: self.network.is_some(),
845-
network: self.network.as_ref(),
846-
sandbox_policy_cwd: &self.sandbox_policy_cwd,
847-
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.as_ref(),
848-
use_legacy_landlock: self.use_legacy_landlock,
849-
windows_sandbox_level: self.windows_sandbox_level,
850-
windows_sandbox_private_desktop: false,
851-
})?;
843+
self.network.is_some(),
844+
self.network.as_ref(),
845+
&self.sandbox_policy_cwd,
846+
self.codex_linux_sandbox_exe.as_ref(),
847+
self.use_legacy_landlock,
848+
self.windows_sandbox_level,
849+
false,
850+
)?;
852851
let mut exec_request =
853852
crate::sandboxing::ExecRequest::from_sandbox_exec_request(exec_request, options);
854853
if let Some(network) = exec_request.network.as_ref() {

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,12 @@ pub struct UnifiedExecRuntime<'a> {
8686
fn build_remote_exec_sandbox_config(
8787
attempt: &SandboxAttempt<'_>,
8888
additional_permissions: Option<PermissionProfile>,
89-
) -> Option<SandboxLaunchConfig> {
89+
) -> SandboxLaunchConfig {
9090
if matches!(attempt.sandbox, codex_sandboxing::SandboxType::None) {
91-
return None;
91+
return SandboxLaunchConfig::no_sandbox(attempt.sandbox_cwd.to_path_buf());
9292
}
9393

94-
Some(SandboxLaunchConfig {
94+
SandboxLaunchConfig {
9595
sandbox: attempt.sandbox,
9696
policy: attempt.policy.clone(),
9797
file_system_policy: attempt.file_system_policy.clone(),
@@ -102,7 +102,7 @@ fn build_remote_exec_sandbox_config(
102102
windows_sandbox_level: attempt.windows_sandbox_level,
103103
windows_sandbox_private_desktop: attempt.windows_sandbox_private_desktop,
104104
use_legacy_landlock: attempt.use_legacy_landlock,
105-
})
105+
}
106106
}
107107

108108
impl<'a> UnifiedExecRuntime<'a> {
@@ -248,12 +248,6 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
248248
.to_string(),
249249
));
250250
}
251-
if req.network.is_some() {
252-
return Err(ToolError::Rejected(
253-
"unified_exec managed-network is not supported when exec_server_url is configured"
254-
.to_string(),
255-
));
256-
}
257251
let exec_params = codex_exec_server::ExecParams {
258252
process_id: req.process_id.to_string().into(),
259253
argv: command,

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

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ use codex_protocol::protocol::SandboxPolicy;
2424
use codex_sandboxing::SandboxCommand;
2525
use codex_sandboxing::SandboxManager;
2626
use codex_sandboxing::SandboxTransformError;
27-
use codex_sandboxing::SandboxTransformRequest;
2827
use codex_sandboxing::SandboxType;
2928
use codex_sandboxing::SandboxablePreference;
3029
use futures::Future;
@@ -339,20 +338,20 @@ impl<'a> SandboxAttempt<'a> {
339338
network: Option<&NetworkProxy>,
340339
) -> Result<crate::sandboxing::ExecRequest, SandboxTransformError> {
341340
self.manager
342-
.transform(SandboxTransformRequest {
341+
.transform(
343342
command,
344-
policy: self.policy,
345-
file_system_policy: self.file_system_policy,
346-
network_policy: self.network_policy,
347-
sandbox: self.sandbox,
348-
enforce_managed_network: self.enforce_managed_network,
343+
self.policy,
344+
self.file_system_policy,
345+
self.network_policy,
346+
self.sandbox,
347+
self.enforce_managed_network,
349348
network,
350-
sandbox_policy_cwd: self.sandbox_cwd,
351-
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe,
352-
use_legacy_landlock: self.use_legacy_landlock,
353-
windows_sandbox_level: self.windows_sandbox_level,
354-
windows_sandbox_private_desktop: self.windows_sandbox_private_desktop,
355-
})
349+
self.sandbox_cwd,
350+
self.codex_linux_sandbox_exe,
351+
self.use_legacy_landlock,
352+
self.windows_sandbox_level,
353+
self.windows_sandbox_private_desktop,
354+
)
356355
.map(|request| {
357356
crate::sandboxing::ExecRequest::from_sandbox_exec_request(request, options)
358357
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
//! Flow at a glance (open process)
1313
//! 1) Build a small request `{ command, cwd }`.
1414
//! 2) Orchestrator: approval (bypass/cache/prompt) → select sandbox → run.
15-
//! 3) Runtime: transform `SandboxTransformRequest` -> `ExecRequest` -> spawn PTY.
15+
//! 3) Runtime: transform sandbox config -> `ExecRequest` -> spawn PTY.
1616
//! 4) If denial, orchestrator retries with `SandboxType::None`.
1717
//! 5) Process handle is returned with streaming output + metadata.
1818
//!

codex-rs/core/src/unified_exec/process_manager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ impl UnifiedExecProcessManager {
609609
env: env.env.clone(),
610610
tty,
611611
arg0: env.arg0.clone(),
612-
sandbox: None,
612+
sandbox: codex_sandboxing::SandboxLaunchConfig::no_sandbox(env.cwd.clone()),
613613
})
614614
.await
615615
.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ mod tests {
160160
use super::Environment;
161161
use super::EnvironmentManager;
162162
use crate::ProcessId;
163+
use codex_sandboxing::SandboxLaunchConfig;
163164
use pretty_assertions::assert_eq;
164165

165166
#[tokio::test]
@@ -202,7 +203,9 @@ mod tests {
202203
env: Default::default(),
203204
tty: false,
204205
arg0: None,
205-
sandbox: None,
206+
sandbox: SandboxLaunchConfig::no_sandbox(
207+
std::env::current_dir().expect("read current dir"),
208+
),
206209
})
207210
.await
208211
.expect("start process");

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

Lines changed: 4 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,9 @@ use std::time::Duration;
99

1010
use async_trait::async_trait;
1111
use codex_app_server_protocol::JSONRPCErrorError;
12-
use codex_protocol::config_types::WindowsSandboxLevel;
13-
use codex_protocol::permissions::FileSystemSandboxPolicy;
14-
use codex_protocol::permissions::NetworkSandboxPolicy;
15-
use codex_protocol::protocol::SandboxPolicy;
1612
use codex_sandboxing::SandboxCommand;
1713
use codex_sandboxing::SandboxExecRequest;
1814
use codex_sandboxing::SandboxType;
19-
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
2015
use codex_utils_pty::ExecCommandSession;
2116
use codex_utils_pty::TerminalSize;
2217
use tokio::sync::Mutex;
@@ -110,14 +105,8 @@ struct ExecServerRuntimeConfig {
110105
impl ExecServerRuntimeConfig {
111106
fn detect() -> Self {
112107
let env_path = std::env::var_os("CODEX_LINUX_SANDBOX_EXE").map(PathBuf::from);
113-
let sibling_path = std::env::current_exe().ok().and_then(|current_exe| {
114-
current_exe
115-
.parent()
116-
.map(|parent| parent.join(CODEX_LINUX_SANDBOX_ARG0))
117-
.filter(|candidate| candidate.exists())
118-
});
119108
Self {
120-
codex_linux_sandbox_exe: env_path.or(sibling_path),
109+
codex_linux_sandbox_exe: env_path,
121110
}
122111
}
123112
}
@@ -523,29 +512,14 @@ fn prepare_exec_launch(
523512
params: &ExecParams,
524513
runtime: &ExecServerRuntimeConfig,
525514
) -> Result<SandboxExecRequest, JSONRPCErrorError> {
526-
let Some(sandbox) = params.sandbox.as_ref() else {
527-
return Ok(SandboxExecRequest {
528-
command: params.argv.clone(),
529-
cwd: params.cwd.clone(),
530-
env: params.env.clone(),
531-
arg0: params.arg0.clone(),
532-
network: None,
533-
sandbox: SandboxType::None,
534-
windows_sandbox_level: WindowsSandboxLevel::Disabled,
535-
windows_sandbox_private_desktop: false,
536-
sandbox_policy: SandboxPolicy::DangerFullAccess,
537-
file_system_sandbox_policy: FileSystemSandboxPolicy::unrestricted(),
538-
network_sandbox_policy: NetworkSandboxPolicy::Enabled,
539-
});
540-
};
541-
542515
let command = build_sandbox_command(
543516
&params.argv,
544517
params.cwd.as_path(),
545518
&params.env,
546-
sandbox.additional_permissions.clone(),
519+
params.sandbox.additional_permissions.clone(),
547520
)?;
548-
sandbox
521+
params
522+
.sandbox
549523
.transform(
550524
command,
551525
// TODO: Thread managed-network proxy state across exec-server so

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ pub struct ExecParams {
6262
pub env: HashMap<String, String>,
6363
pub tty: bool,
6464
pub arg0: Option<String>,
65-
pub sandbox: Option<SandboxLaunchConfig>,
65+
pub sandbox: SandboxLaunchConfig,
6666
}
6767

6868
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]

0 commit comments

Comments
 (0)