mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
codex: address remaining sandbox review feedback (#16736)
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -160,6 +160,7 @@ mod tests {
|
||||
use super::Environment;
|
||||
use super::EnvironmentManager;
|
||||
use crate::ProcessId;
|
||||
use codex_sandboxing::SandboxLaunchConfig;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
@@ -202,7 +203,9 @@ mod tests {
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
sandbox: None,
|
||||
sandbox: SandboxLaunchConfig::no_sandbox(
|
||||
std::env::current_dir().expect("read current dir"),
|
||||
),
|
||||
})
|
||||
.await
|
||||
.expect("start process");
|
||||
|
||||
@@ -9,14 +9,9 @@ use std::time::Duration;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_sandboxing::SandboxExecRequest;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
|
||||
use codex_utils_pty::ExecCommandSession;
|
||||
use codex_utils_pty::TerminalSize;
|
||||
use tokio::sync::Mutex;
|
||||
@@ -110,14 +105,8 @@ struct ExecServerRuntimeConfig {
|
||||
impl ExecServerRuntimeConfig {
|
||||
fn detect() -> Self {
|
||||
let env_path = std::env::var_os("CODEX_LINUX_SANDBOX_EXE").map(PathBuf::from);
|
||||
let sibling_path = std::env::current_exe().ok().and_then(|current_exe| {
|
||||
current_exe
|
||||
.parent()
|
||||
.map(|parent| parent.join(CODEX_LINUX_SANDBOX_ARG0))
|
||||
.filter(|candidate| candidate.exists())
|
||||
});
|
||||
Self {
|
||||
codex_linux_sandbox_exe: env_path.or(sibling_path),
|
||||
codex_linux_sandbox_exe: env_path,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -523,29 +512,14 @@ fn prepare_exec_launch(
|
||||
params: &ExecParams,
|
||||
runtime: &ExecServerRuntimeConfig,
|
||||
) -> Result<SandboxExecRequest, JSONRPCErrorError> {
|
||||
let Some(sandbox) = params.sandbox.as_ref() else {
|
||||
return Ok(SandboxExecRequest {
|
||||
command: params.argv.clone(),
|
||||
cwd: params.cwd.clone(),
|
||||
env: params.env.clone(),
|
||||
arg0: params.arg0.clone(),
|
||||
network: None,
|
||||
sandbox: SandboxType::None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
file_system_sandbox_policy: FileSystemSandboxPolicy::unrestricted(),
|
||||
network_sandbox_policy: NetworkSandboxPolicy::Enabled,
|
||||
});
|
||||
};
|
||||
|
||||
let command = build_sandbox_command(
|
||||
¶ms.argv,
|
||||
params.cwd.as_path(),
|
||||
¶ms.env,
|
||||
sandbox.additional_permissions.clone(),
|
||||
params.sandbox.additional_permissions.clone(),
|
||||
)?;
|
||||
sandbox
|
||||
params
|
||||
.sandbox
|
||||
.transform(
|
||||
command,
|
||||
// TODO: Thread managed-network proxy state across exec-server so
|
||||
|
||||
@@ -62,7 +62,7 @@ pub struct ExecParams {
|
||||
pub env: HashMap<String, String>,
|
||||
pub tty: bool,
|
||||
pub arg0: Option<String>,
|
||||
pub sandbox: Option<SandboxLaunchConfig>,
|
||||
pub sandbox: SandboxLaunchConfig,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use tokio::sync::watch;
|
||||
use tracing::trace;
|
||||
|
||||
@@ -35,10 +34,7 @@ impl RemoteProcess {
|
||||
impl ExecBackend for RemoteProcess {
|
||||
async fn start(&self, params: ExecParams) -> Result<StartedExecProcess, ExecServerError> {
|
||||
let process_id = params.process_id.clone();
|
||||
let sandbox_type = params
|
||||
.sandbox
|
||||
.as_ref()
|
||||
.map_or(SandboxType::None, |sandbox| sandbox.sandbox);
|
||||
let sandbox_type = params.sandbox.sandbox;
|
||||
let session = self.client.register_session(&process_id).await?;
|
||||
match self.client.exec(params).await {
|
||||
Ok(_) => {}
|
||||
|
||||
@@ -12,6 +12,7 @@ use crate::protocol::InitializeResponse;
|
||||
use crate::protocol::TerminateParams;
|
||||
use crate::protocol::TerminateResponse;
|
||||
use crate::rpc::RpcNotificationSender;
|
||||
use codex_sandboxing::SandboxLaunchConfig;
|
||||
|
||||
fn exec_params(process_id: &str) -> ExecParams {
|
||||
let mut env = HashMap::new();
|
||||
@@ -29,7 +30,7 @@ fn exec_params(process_id: &str) -> ExecParams {
|
||||
env,
|
||||
tty: false,
|
||||
arg0: None,
|
||||
sandbox: None,
|
||||
sandbox: SandboxLaunchConfig::no_sandbox(std::env::current_dir().expect("cwd")),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -61,7 +61,9 @@ async fn assert_exec_process_starts_and_exits(use_remote: bool) -> Result<()> {
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
sandbox: None,
|
||||
sandbox: SandboxLaunchConfig::no_sandbox(
|
||||
std::env::current_dir().expect("read current dir"),
|
||||
),
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(session.process.process_id().as_str(), "proc-1");
|
||||
@@ -138,7 +140,9 @@ async fn assert_exec_process_streams_output(use_remote: bool) -> Result<()> {
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
sandbox: None,
|
||||
sandbox: SandboxLaunchConfig::no_sandbox(
|
||||
std::env::current_dir().expect("read current dir"),
|
||||
),
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(session.process.process_id().as_str(), process_id);
|
||||
@@ -168,7 +172,7 @@ async fn assert_exec_process_write_then_read(use_remote: bool) -> Result<()> {
|
||||
env: Default::default(),
|
||||
tty: true,
|
||||
arg0: None,
|
||||
sandbox: None,
|
||||
sandbox: SandboxLaunchConfig::no_sandbox(std::env::current_dir().expect("read current dir")),
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(session.process.process_id().as_str(), process_id);
|
||||
@@ -205,7 +209,9 @@ async fn assert_exec_process_preserves_queued_events_before_subscribe(
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
sandbox: None,
|
||||
sandbox: SandboxLaunchConfig::no_sandbox(
|
||||
std::env::current_dir().expect("read current dir"),
|
||||
),
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -306,7 +312,9 @@ async fn remote_exec_process_reports_transport_disconnect() -> Result<()> {
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
arg0: None,
|
||||
sandbox: None,
|
||||
sandbox: SandboxLaunchConfig::no_sandbox(
|
||||
std::env::current_dir().expect("read current dir"),
|
||||
),
|
||||
})
|
||||
.await?;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user