openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-15 12:08:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: flip the sense of the --sandbox option

Browse Source
This commit is contained in:
Michael Bolin
2025-04-29 10:09:36 -07:00
parent 3b39964f81
commit febc95ce1d
12 changed files with 183 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
codex-rs/core/src/approval_mode_cli_arg.rs
View File

@@ -23,6 +23,15 @@ pub enum ApprovalModeCliArg {
/// Execution failures are immediately returned to the model.
Never,
}
impl From<ApprovalModeCliArg> for AskForApproval {
fn from(value: ApprovalModeCliArg) -> Self {
match value {
ApprovalModeCliArg::OnFailure => AskForApproval::OnFailure,
ApprovalModeCliArg::UnlessAllowListed => AskForApproval::UnlessAllowListed,
ApprovalModeCliArg::Never => AskForApproval::Never,
}
}
}
#[derive(Clone, Copy, Debug, ValueEnum)]
#[value(rename_all = "kebab-case")]
@@ -37,25 +46,8 @@ pub enum SandboxModeCliArg {
DangerousNoRestrictions,
}
impl From<ApprovalModeCliArg> for AskForApproval {
fn from(value: ApprovalModeCliArg) -> Self {
match value {
ApprovalModeCliArg::OnFailure => AskForApproval::OnFailure,
ApprovalModeCliArg::UnlessAllowListed => AskForApproval::UnlessAllowListed,
ApprovalModeCliArg::Never => AskForApproval::Never,
}
}
}
impl From<SandboxModeCliArg> for SandboxPolicy {
fn from(value: SandboxModeCliArg) -> Self {
match value {
SandboxModeCliArg::NetworkRestricted => SandboxPolicy::NetworkRestricted,
SandboxModeCliArg::FileWriteRestricted => SandboxPolicy::FileWriteRestricted,
SandboxModeCliArg::NetworkAndFileWriteRestricted => {
SandboxPolicy::NetworkAndFileWriteRestricted
}
SandboxModeCliArg::DangerousNoRestrictions => SandboxPolicy::DangerousNoRestrictions,
}
impl From<Vec<SandboxModeCliArg>> for SandboxPolicy {
fn from(value: Vec<SandboxModeCliArg>) -> Self {
unimplemented!("need to convert {value:?}");
}
}

12
codex-rs/core/src/codex.rs
View File

@@ -861,7 +861,7 @@ async fn handle_function_call(
assess_command_safety(
&params.command,
sess.approval_policy,
sess.sandbox_policy,
&sess.sandbox_policy,
&state.approved_commands,
)
};
@@ -916,14 +916,11 @@ async fn handle_function_call(
)
.await;
let roots_snapshot = { sess.writable_roots.lock().unwrap().clone() };
let output_result = process_exec_tool_call(
params.clone(),
sandbox_type,
&roots_snapshot,
sess.ctrl_c.clone(),
sess.sandbox_policy,
&sess.sandbox_policy,
)
.await;
@@ -1006,16 +1003,13 @@ async fn handle_function_call(
)
.await;
let retry_roots = { sess.writable_roots.lock().unwrap().clone() };
// This is an escalated retry; the policy will not be
// examined and the sandbox has been set to `None`.
let retry_output_result = process_exec_tool_call(
params.clone(),
SandboxType::None,
&retry_roots,
sess.ctrl_c.clone(),
sess.sandbox_policy,
&sess.sandbox_policy,
)
.await;

94
codex-rs/core/src/exec.rs
View File

@@ -1,7 +1,6 @@
use std::io;
#[cfg(target_family = "unix")]
use std::os::unix::process::ExitStatusExt;
use std::path::PathBuf;
use std::process::ExitStatus;
use std::process::Stdio;
use std::sync::Arc;
@@ -33,7 +32,7 @@ const DEFAULT_TIMEOUT_MS: u64 = 10_000;
const SIGKILL_CODE: i32 = 9;
const TIMEOUT_CODE: i32 = 64;
const MACOS_SEATBELT_READONLY_POLICY: &str = include_str!("seatbelt_readonly_policy.sbpl");
const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl");
/// When working with `sandbox-exec`, only consider `sandbox-exec` in `/usr/bin`
/// to defend against an attacker trying to inject a malicious version on the
@@ -67,19 +66,17 @@ pub enum SandboxType {
#[cfg(target_os = "linux")]
async fn exec_linux(
params: ExecParams,
writable_roots: &[PathBuf],
ctrl_c: Arc<Notify>,
sandbox_policy: SandboxPolicy,
) -> Result<RawExecToolCallOutput> {
crate::linux::exec_linux(params, writable_roots, ctrl_c, sandbox_policy).await
crate::linux::exec_linux(params, ctrl_c, sandbox_policy).await
}
#[cfg(not(target_os = "linux"))]
async fn exec_linux(
_params: ExecParams,
_writable_roots: &[PathBuf],
_ctrl_c: Arc<Notify>,
_sandbox_policy: SandboxPolicy,
_sandbox_policy: &SandboxPolicy,
) -> Result<RawExecToolCallOutput> {
Err(CodexErr::Io(io::Error::new(
io::ErrorKind::InvalidInput,
@@ -90,9 +87,8 @@ async fn exec_linux(
pub async fn process_exec_tool_call(
params: ExecParams,
sandbox_type: SandboxType,
writable_roots: &[PathBuf],
ctrl_c: Arc<Notify>,
sandbox_policy: SandboxPolicy,
sandbox_policy: &SandboxPolicy,
) -> Result<ExecToolCallOutput> {
let start = Instant::now();
@@ -104,7 +100,7 @@ pub async fn process_exec_tool_call(
workdir,
timeout_ms,
} = params;
let seatbelt_command = create_seatbelt_command(command, sandbox_policy, writable_roots);
let seatbelt_command = create_seatbelt_command(command, sandbox_policy);
exec(
ExecParams {
command: seatbelt_command,
@@ -115,9 +111,7 @@ pub async fn process_exec_tool_call(
)
.await
}
SandboxType::LinuxSeccomp => {
exec_linux(params, writable_roots, ctrl_c, sandbox_policy).await
}
SandboxType::LinuxSeccomp => exec_linux(params, ctrl_c, sandbox_policy).await,
};
let duration = start.elapsed();
match raw_output_result {
@@ -162,41 +156,61 @@ pub async fn process_exec_tool_call(
pub fn create_seatbelt_command(
command: Vec<String>,
sandbox_policy: SandboxPolicy,
writable_roots: &[PathBuf],
sandbox_policy: &SandboxPolicy,
) -> Vec<String> {
let (policies, cli_args): (Vec<String>, Vec<String>) = writable_roots
.iter()
.enumerate()
.map(|(index, root)| {
let param_name = format!("WRITABLE_ROOT_{index}");
let policy: String = format!("(subpath (param \"{param_name}\"))");
let cli_arg = format!("-D{param_name}={}", root.to_string_lossy());
(policy, cli_arg)
})
.unzip();
// TODO(ragona): The seatbelt policy should reflect the SandboxPolicy that
// is passed, but everything is currently hardcoded to use
// MACOS_SEATBELT_READONLY_POLICY.
// TODO(mbolin): apply_patch calls must also honor the SandboxPolicy.
if !matches!(sandbox_policy, SandboxPolicy::NetworkRestricted) {
tracing::error!("specified sandbox policy {sandbox_policy:?} will not be honroed");
}
let full_policy = if policies.is_empty() {
MACOS_SEATBELT_READONLY_POLICY.to_string()
} else {
let scoped_write_policy = format!("(allow file-write*\n{}\n)", policies.join(" "));
format!("{MACOS_SEATBELT_READONLY_POLICY}\n{scoped_write_policy}")
let (file_write_policy, extra_cli_args) = {
if sandbox_policy.has_full_disk_write_access() {
// Allegedly, this is more permissive than `(allow file-write*)`.
(
r#"(allow file-write* (regex #"^/"))"#.to_string(),
Vec::<String>::new(),
)
} else {
let writable_roots = sandbox_policy.get_writable_roots();
let (writable_folder_policies, cli_args): (Vec<String>, Vec<String>) = writable_roots
.iter()
.enumerate()
.map(|(index, root)| {
let param_name = format!("WRITABLE_ROOT_{index}");
let policy: String = format!("(subpath (param \"{param_name}\"))");
let cli_arg = format!("-D{param_name}={}", root.to_string_lossy());
(policy, cli_arg)
})
.unzip();
if writable_folder_policies.is_empty() {
("".to_string(), Vec::<String>::new())
} else {
let file_write_policy = format!(
"(allow file-write*\n{}\n)",
writable_folder_policies.join(" ")
);
(file_write_policy, cli_args)
}
}
};
let file_read_policy = if sandbox_policy.has_full_disk_read_access() {
"; allow read-only file operations\n(allow file-read*)"
} else {
""
};
// TODO(mbolin): apply_patch calls must also honor the SandboxPolicy.
let network_policy = if sandbox_policy.has_full_network_access() {
"(allow network-outbound)\n(allow network-inbound)\n(allow system-socket)"
} else {
""
};
let full_policy = format!(
"{MACOS_SEATBELT_BASE_POLICY}\n{file_read_policy}\n{file_write_policy}\n{network_policy}"
);
let mut seatbelt_command: Vec<String> = vec![
MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string(),
"-p".to_string(),
full_policy.to_string(),
full_policy,
];
seatbelt_command.extend(cli_args);
seatbelt_command.extend(extra_cli_args);
seatbelt_command.push("--".to_string());
seatbelt_command.extend(command);
seatbelt_command

3
codex-rs/core/src/linux.rs
View File

@@ -32,14 +32,13 @@ use tokio::sync::Notify;
pub async fn exec_linux(
params: ExecParams,
writable_roots: &[PathBuf],
ctrl_c: Arc<Notify>,
sandbox_policy: SandboxPolicy,
) -> Result<RawExecToolCallOutput> {
// Allow READ on /
// Allow WRITE on /dev/null
let ctrl_c_copy = ctrl_c.clone();
let writable_roots_copy = writable_roots.to_vec();
let writable_roots = sandbox_policy.get_writable_roots();
// Isolate thread to run the sandbox from
let tool_call_output = std::thread::spawn(move || {

125
codex-rs/core/src/protocol.rs
View File

@@ -93,44 +93,115 @@ pub enum AskForApproval {
}
/// Determines execution restrictions for model shell commands
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)]
#[derive(Debug, Clone, Default, PartialEq, Eq, Hash, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum SandboxPolicy {
/// Network syscalls will be blocked
NetworkRestricted,
/// Filesystem writes will be restricted
FileWriteRestricted,
/// Network and filesystem writes will be restricted
#[default]
NetworkAndFileWriteRestricted,
/// No restrictions; full "unsandboxed" mode
DangerousNoRestrictions,
pub struct SandboxPolicy {
pub permissions: Vec<SandboxPermission>,
}
impl SandboxPolicy {
pub fn is_dangerous(&self) -> bool {
match self {
SandboxPolicy::NetworkRestricted => false,
SandboxPolicy::FileWriteRestricted => false,
SandboxPolicy::NetworkAndFileWriteRestricted => false,
SandboxPolicy::DangerousNoRestrictions => true,
pub fn new_read_only_policy() -> Self {
Self {
permissions: vec![SandboxPermission::DiskFullReadAccess],
}
}
pub fn is_network_restricted(&self) -> bool {
matches!(
self,
SandboxPolicy::NetworkRestricted | SandboxPolicy::NetworkAndFileWriteRestricted
)
pub fn has_full_disk_read_access(&self) -> bool {
self.permissions
.iter()
.any(|perm| matches!(perm, SandboxPermission::DiskFullReadAccess))
}
pub fn is_file_write_restricted(&self) -> bool {
matches!(
self,
SandboxPolicy::FileWriteRestricted | SandboxPolicy::NetworkAndFileWriteRestricted
)
pub fn has_full_disk_write_access(&self) -> bool {
self.permissions
.iter()
.any(|perm| matches!(perm, SandboxPermission::DiskFullWriteAccess))
}
pub fn has_full_network_access(&self) -> bool {
self.permissions
.iter()
.any(|perm| matches!(perm, SandboxPermission::NetworkFullAccess))
}
pub fn get_writable_roots(&self) -> Vec<PathBuf> {
let mut writable_roots = Vec::<PathBuf>::new();
for perm in &self.permissions {
use SandboxPermission::*;
match perm {
DiskWritePlatformUserTempFolder => {
if cfg!(target_os = "macos") {
if let Some(temp_dir) = std::env::var_os("TMPDIR") {
// Add temp_dir and the canonicalized version of temp_dir.
}
}
// For Linux, should this be XDG_RUNTIME_DIR, /run/user/<uid>, or something else?
}
DiskWritePlatformGlobalTempFolder => {
if cfg!(unix) {
writable_roots.push(PathBuf::from("/tmp"));
}
}
DiskWriteCwd => match std::env::current_dir() {
Ok(cwd) => writable_roots.push(cwd),
Err(err) => {
tracing::error!("Failed to get current working directory: {err}");
}
},
DiskWriteFolder { folder } => {
writable_roots.push(folder.clone());
}
DiskFullReadAccess | NetworkFullAccess => {}
DiskFullWriteAccess => {
// Currently, we expect callers to only invoke this method
// after verifying has_full_disk_write_access() is false.
}
}
}
writable_roots
}
pub fn is_unrestricted(&self) -> bool {
self.has_full_disk_read_access()
&& self.has_full_disk_write_access()
&& self.has_full_network_access()
}
}
/// Permissions that should be granted to the sandbox in which the agent
/// operates.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub enum SandboxPermission {
/// Is allowed to read all files on disk.
DiskFullReadAccess,
/// Is allowed to write to the operating system's temp dir that
/// is restricted to the user the agent is running as. For
/// example, on macOS, this is generally something under
/// `/var/folders` as opposed to `/tmp`.
DiskWritePlatformUserTempFolder,
/// Is allowed to write to the operating system's shared temp
/// dir. On UNIX, this is generally `/tmp`.
DiskWritePlatformGlobalTempFolder,
/// Is allowed to write to the current working directory (in practice, this
/// is the `cwd` where `codex` was spawned).
DiskWriteCwd,
/// Is allowed to the specified folder. `PathBuf` must be an
/// absolute path, though it is up to the caller to canonicalize
/// it if the path contains symlinks.
DiskWriteFolder { folder: PathBuf },
/// Is allowed to write to any file on disk.
DiskFullWriteAccess,
/// Can make arbitrary network requests.
NetworkFullAccess,
}
/// User input
#[non_exhaustive]
#[derive(Debug, Clone, Deserialize, Serialize)]

13
codex-rs/core/src/safety.rs
View File

@@ -65,7 +65,7 @@ pub fn assess_patch_safety(
pub fn assess_command_safety(
command: &[String],
approval_policy: AskForApproval,
sandbox_policy: SandboxPolicy,
sandbox_policy: &SandboxPolicy,
approved: &HashSet<Vec<String>>,
) -> SafetyCheck {
let approve_without_sandbox = || SafetyCheck::AutoApprove {
@@ -81,11 +81,10 @@ pub fn assess_command_safety(
}
// Command was not known-safe or allow-listed
match sandbox_policy {
// Only the dangerous sandbox policy will run arbitrary commands outside a sandbox
SandboxPolicy::DangerousNoRestrictions => approve_without_sandbox(),
// All other policies try to run the command in a sandbox if it is available
_ => match get_platform_sandbox() {
if sandbox_policy.is_unrestricted() {
approve_without_sandbox()
} else {
match get_platform_sandbox() {
// We have a sandbox, so we can approve the command in all modes
Some(sandbox_type) => SafetyCheck::AutoApprove { sandbox_type },
None => {
@@ -99,7 +98,7 @@ pub fn assess_command_safety(
_ => SafetyCheck::AskUser,
}
}
},
}
}
}

3
codex-rs/core/src/seatbelt_readonly_policy.sbpl → codex-rs/core/src/seatbelt_base_policy.sbpl
View File

@@ -6,9 +6,6 @@
; start with closed-by-default
(deny default)
; allow read-only file operations
(allow file-read*)
; child processes inherit the policy of their parent
(allow process-exec)
(allow process-fork)

2
codex-rs/core/tests/live_agent.rs
View File

@@ -55,7 +55,7 @@ async fn spawn_codex() -> Codex {
model: config.model,
instructions: None,
approval_policy: config.approval_policy,
sandbox_policy: SandboxPolicy::NetworkAndFileWriteRestricted,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
disable_response_storage: false,
},
})

2
codex-rs/core/tests/previous_response_id.rs
View File

@@ -95,7 +95,7 @@ async fn keeps_previous_response_id_between_tasks() {
model: config.model,
instructions: None,
approval_policy: config.approval_policy,
sandbox_policy: SandboxPolicy::NetworkAndFileWriteRestricted,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
disable_response_storage: false,
},
})

2
codex-rs/core/tests/stream_no_completed.rs
View File

@@ -78,7 +78,7 @@ async fn retries_on_early_close() {
model: config.model,
instructions: None,
approval_policy: config.approval_policy,
sandbox_policy: SandboxPolicy::NetworkAndFileWriteRestricted,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
disable_response_storage: false,
},
})

7
codex-rs/tui/src/cli.rs
View File

@@ -1,3 +1,4 @@
use clap::ArgAction;
use clap::Parser;
use codex_core::ApprovalModeCliArg;
use codex_core::SandboxModeCliArg;
@@ -21,11 +22,11 @@ pub struct Cli {
#[arg(long = "ask-for-approval", short = 'a')]
pub approval_policy: Option<ApprovalModeCliArg>,
/// Configure the process restrictions when a command is executed.
/// Configure the sandbox permissions when executed an untrusted command.
///
/// Uses OS-specific sandboxing tools; Seatbelt on OSX, landlock+seccomp on Linux.
#[arg(long = "sandbox", short = 's')]
pub sandbox_policy: Option<SandboxModeCliArg>,
#[arg(long = "sandbox", short = 's', action = ArgAction::Append)]
pub sandbox_policy: Vec<SandboxModeCliArg>,
/// Allow running Codex outside a Git repository.
#[arg(long = "skip-git-repo-check", default_value_t = false)]

3
codex-rs/tui/src/lib.rs
View File

@@ -38,7 +38,8 @@ pub fn run_main(cli: Cli) -> std::io::Result<()> {
let overrides = ConfigOverrides {
model: cli.model.clone(),
approval_policy: cli.approval_policy.map(Into::into),
sandbox_policy: cli.sandbox_policy.map(Into::into),
// FIXME(mbolin): How should this work?
sandbox_policy: None,
disable_response_storage: if cli.disable_response_storage {
Some(true)
} else {