diff --git a/codex-rs/cli/src/landlock.rs b/codex-rs/cli/src/landlock.rs index be2ba1e3..b57591bf 100644 --- a/codex-rs/cli/src/landlock.rs +++ b/codex-rs/cli/src/landlock.rs @@ -5,7 +5,6 @@ use codex_core::protocol::SandboxPolicy; use std::os::unix::process::ExitStatusExt; -use std::path::PathBuf; use std::process; use std::process::Command; use std::process::ExitStatus; @@ -15,7 +14,6 @@ use std::process::ExitStatus; pub(crate) fn run_landlock( command: Vec, sandbox_policy: SandboxPolicy, - writable_roots: Vec, ) -> anyhow::Result<()> { if command.is_empty() { anyhow::bail!("command args are empty"); @@ -23,16 +21,7 @@ pub(crate) fn run_landlock( // Spawn a new thread and apply the sandbox policies there. let handle = std::thread::spawn(move || -> anyhow::Result { - // Apply sandbox policies inside this thread so only the child inherits - // them, not the entire CLI process. - if sandbox_policy.is_network_restricted() { - codex_core::linux::install_network_seccomp_filter_on_current_thread()?; - } - - if sandbox_policy.is_file_write_restricted() { - codex_core::linux::install_filesystem_landlock_rules_on_current_thread(writable_roots)?; - } - + codex_core::linux::apply_sandbox_policy_to_current_thread(sandbox_policy)?; let status = Command::new(&command[0]).args(&command[1..]).status()?; Ok(status) }); diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index d8a58de8..fa0a14e6 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -7,7 +7,7 @@ use std::path::PathBuf; use clap::ArgAction; use clap::Parser; -use codex_core::SandboxModeCliArg; +use codex_core::protocol::SandboxPolicy; use codex_exec::Cli as ExecCli; use codex_repl::Cli as ReplCli; use codex_tui::Cli as TuiCli; @@ -71,9 +71,9 @@ struct SeatbeltCommand { #[arg(long = "writable-root", short = 'w', value_name = "DIR", action = ArgAction::Append, use_value_delimiter = false)] writable_roots: Vec, - /// Configure the process restrictions for the command. - #[arg(long = "sandbox", short = 's')] - sandbox_policy: SandboxModeCliArg, + /// Convenience alias for low-friction sandboxed automatic execution (network-disabled sandbox that can write to cwd and TMPDIR) + #[arg(long = "full-auto", default_value_t = false)] + full_auto: bool, /// Full command args to run under seatbelt. #[arg(trailing_var_arg = true)] @@ -86,9 +86,9 @@ struct LandlockCommand { #[arg(long = "writable-root", short = 'w', value_name = "DIR", action = ArgAction::Append, use_value_delimiter = false)] writable_roots: Vec, - /// Configure the process restrictions for the command. - #[arg(long = "sandbox", short = 's')] - sandbox_policy: SandboxModeCliArg, + /// Convenience alias for low-friction sandboxed automatic execution (network-disabled sandbox that can write to cwd and TMPDIR) + #[arg(long = "full-auto", default_value_t = false)] + full_auto: bool, /// Full command args to run under landlock. #[arg(trailing_var_arg = true)] @@ -118,18 +118,20 @@ async fn main() -> anyhow::Result<()> { Some(Subcommand::Debug(debug_args)) => match debug_args.cmd { DebugCommand::Seatbelt(SeatbeltCommand { command, - sandbox_policy, writable_roots, + full_auto, }) => { - seatbelt::run_seatbelt(command, sandbox_policy.into(), writable_roots).await?; + let sandbox_policy = create_sandbox_policy(full_auto, &writable_roots); + seatbelt::run_seatbelt(command, sandbox_policy).await?; } #[cfg(target_os = "linux")] DebugCommand::Landlock(LandlockCommand { command, - sandbox_policy, writable_roots, + full_auto, }) => { - landlock::run_landlock(command, sandbox_policy.into(), writable_roots)?; + let sandbox_policy = create_sandbox_policy(full_auto, &writable_roots); + landlock::run_landlock(command, sandbox_policy)?; } #[cfg(not(target_os = "linux"))] DebugCommand::Landlock(_) => { @@ -140,3 +142,11 @@ async fn main() -> anyhow::Result<()> { Ok(()) } + +fn create_sandbox_policy(full_auto: bool, writable_roots: &[PathBuf]) -> SandboxPolicy { + if full_auto { + SandboxPolicy::new_full_auto_policy_with_writable_roots(writable_roots) + } else { + SandboxPolicy::new_read_only_policy_with_writable_roots(writable_roots) + } +} diff --git a/codex-rs/cli/src/seatbelt.rs b/codex-rs/cli/src/seatbelt.rs index d328f552..f4a8edde 100644 --- a/codex-rs/cli/src/seatbelt.rs +++ b/codex-rs/cli/src/seatbelt.rs @@ -1,13 +1,11 @@ use codex_core::exec::create_seatbelt_command; use codex_core::protocol::SandboxPolicy; -use std::path::PathBuf; pub(crate) async fn run_seatbelt( command: Vec, sandbox_policy: SandboxPolicy, - writable_roots: Vec, ) -> anyhow::Result<()> { - let seatbelt_command = create_seatbelt_command(command, sandbox_policy, &writable_roots); + let seatbelt_command = create_seatbelt_command(command, &sandbox_policy); let status = tokio::process::Command::new(seatbelt_command[0].clone()) .args(&seatbelt_command[1..]) .spawn() diff --git a/codex-rs/core/src/approval_mode_cli_arg.rs b/codex-rs/core/src/approval_mode_cli_arg.rs index 0da6a89e..8154e49f 100644 --- a/codex-rs/core/src/approval_mode_cli_arg.rs +++ b/codex-rs/core/src/approval_mode_cli_arg.rs @@ -4,7 +4,6 @@ use clap::ValueEnum; use crate::protocol::AskForApproval; -use crate::protocol::SandboxPolicy; #[derive(Clone, Copy, Debug, ValueEnum)] #[value(rename_all = "kebab-case")] @@ -24,19 +23,6 @@ pub enum ApprovalModeCliArg { Never, } -#[derive(Clone, Copy, Debug, ValueEnum)] -#[value(rename_all = "kebab-case")] -pub enum SandboxModeCliArg { - /// Network syscalls will be blocked - NetworkRestricted, - /// Filesystem writes will be restricted - FileWriteRestricted, - /// Network and filesystem writes will be restricted - NetworkAndFileWriteRestricted, - /// No restrictions; full "unsandboxed" mode - DangerousNoRestrictions, -} - impl From for AskForApproval { fn from(value: ApprovalModeCliArg) -> Self { match value { @@ -46,16 +32,3 @@ impl From for AskForApproval { } } } - -impl From 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, - } - } -} diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index edeaef99..384011e3 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -861,7 +861,7 @@ async fn handle_function_call( assess_command_safety( ¶ms.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; diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 95abae52..55efe5a9 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1,5 +1,6 @@ use crate::flags::OPENAI_DEFAULT_MODEL; use crate::protocol::AskForApproval; +use crate::protocol::SandboxPermission; use crate::protocol::SandboxPolicy; use dirs::home_dir; use serde::Deserialize; @@ -11,27 +12,68 @@ use std::path::PathBuf; const EMBEDDED_INSTRUCTIONS: &str = include_str!("../prompt.md"); /// Application configuration loaded from disk and merged with overrides. -#[derive(Deserialize, Debug, Clone)] +#[derive(Debug, Clone)] pub struct Config { /// Optional override of model selection. - #[serde(default = "default_model")] pub model: String, - /// Default approval policy for executing commands. - #[serde(default)] + + /// Approval policy for executing commands. pub approval_policy: AskForApproval, - #[serde(default)] + pub sandbox_policy: SandboxPolicy, /// Disable server-side response storage (sends the full conversation /// context with every request). Currently necessary for OpenAI customers /// who have opted into Zero Data Retention (ZDR). - #[serde(default)] pub disable_response_storage: bool, /// System instructions. pub instructions: Option, } +/// Base config deserialized from ~/.codex/config.toml. +#[derive(Deserialize, Debug, Clone, Default)] +pub struct ConfigToml { + /// Optional override of model selection. + pub model: Option, + + /// Default approval policy for executing commands. + pub approval_policy: Option, + + pub sandbox_permissions: Option>, + + /// Disable server-side response storage (sends the full conversation + /// context with every request). Currently necessary for OpenAI customers + /// who have opted into Zero Data Retention (ZDR). + pub disable_response_storage: Option, + + /// System instructions. + pub instructions: Option, +} + +impl ConfigToml { + /// Attempt to parse the file at `~/.codex/config.toml`. If it does not + /// exist, return a default config. Though if it exists and cannot be + /// parsed, report that to the user and force them to fix it. + fn load_from_toml() -> std::io::Result { + let config_toml_path = codex_dir()?.join("config.toml"); + match std::fs::read_to_string(&config_toml_path) { + Ok(contents) => toml::from_str::(&contents).map_err(|e| { + tracing::error!("Failed to parse config.toml: {e}"); + std::io::Error::new(std::io::ErrorKind::InvalidData, e) + }), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + tracing::info!("config.toml not found, using defaults"); + Ok(Self::default()) + } + Err(e) => { + tracing::error!("Failed to read config.toml: {e}"); + Err(e) + } + } + } +} + /// Optional overrides for user configuration (e.g., from CLI flags). #[derive(Default, Debug, Clone)] pub struct ConfigOverrides { @@ -46,11 +88,14 @@ impl Config { /// ~/.codex/config.toml, ~/.codex/instructions.md, embedded defaults, and /// any values provided in `overrides` (highest precedence). pub fn load_with_overrides(overrides: ConfigOverrides) -> std::io::Result { - let mut cfg: Config = Self::load_from_toml()?; + let cfg: ConfigToml = ConfigToml::load_from_toml()?; tracing::warn!("Config parsed from config.toml: {cfg:?}"); + Ok(Self::load_from_base_config_with_overrides(cfg, overrides)) + } + fn load_from_base_config_with_overrides(cfg: ConfigToml, overrides: ConfigOverrides) -> Self { // Instructions: user-provided instructions.md > embedded default. - cfg.instructions = + let instructions = Self::load_instructions().or_else(|| Some(EMBEDDED_INSTRUCTIONS.to_string())); // Destructure ConfigOverrides fully to ensure all overrides are applied. @@ -61,57 +106,48 @@ impl Config { disable_response_storage, } = overrides; - if let Some(model) = model { - cfg.model = model; - } - if let Some(approval_policy) = approval_policy { - cfg.approval_policy = approval_policy; - } - if let Some(sandbox_policy) = sandbox_policy { - cfg.sandbox_policy = sandbox_policy; - } - if let Some(disable_response_storage) = disable_response_storage { - cfg.disable_response_storage = disable_response_storage; - } - Ok(cfg) - } - - /// Attempt to parse the file at `~/.codex/config.toml` into a Config. - fn load_from_toml() -> std::io::Result { - let config_toml_path = codex_dir()?.join("config.toml"); - match std::fs::read_to_string(&config_toml_path) { - Ok(contents) => toml::from_str::(&contents).map_err(|e| { - tracing::error!("Failed to parse config.toml: {e}"); - std::io::Error::new(std::io::ErrorKind::InvalidData, e) - }), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - tracing::info!("config.toml not found, using defaults"); - Ok(Self::load_default_config()) - } - Err(e) => { - tracing::error!("Failed to read config.toml: {e}"); - Err(e) + let sandbox_policy = match sandbox_policy { + Some(sandbox_policy) => sandbox_policy, + None => { + // Derive a SandboxPolicy from the permissions in the config. + match cfg.sandbox_permissions { + // Note this means the user can explicitly set permissions + // to the empty list in the config file, granting it no + // permissions whatsoever. + Some(permissions) => SandboxPolicy::from(permissions), + // Default to read only rather than completely locked down. + None => SandboxPolicy::new_read_only_policy(), + } } + }; + + Self { + model: model.or(cfg.model).unwrap_or_else(default_model), + approval_policy: approval_policy + .or(cfg.approval_policy) + .unwrap_or_else(AskForApproval::default), + sandbox_policy, + disable_response_storage: disable_response_storage + .or(cfg.disable_response_storage) + .unwrap_or(false), + instructions, } } - /// Meant to be used exclusively for tests: load_with_overrides() should be - /// used in all other cases. - pub fn load_default_config_for_test() -> Self { - Self::load_default_config() - } - - fn load_default_config() -> Self { - // Load from an empty string to exercise #[serde(default)] to - // get the default values for each field. - toml::from_str::("").expect("empty string should parse as TOML") - } - fn load_instructions() -> Option { let mut p = codex_dir().ok()?; p.push("instructions.md"); std::fs::read_to_string(&p).ok() } + + /// Meant to be used exclusively for tests: `load_with_overrides()` should + /// be used in all other cases. + pub fn load_default_config_for_test() -> Self { + Self::load_from_base_config_with_overrides( + ConfigToml::default(), + ConfigOverrides::default(), + ) + } } fn default_model() -> String { diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 952b4453..cf5fbd61 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -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, - sandbox_policy: SandboxPolicy, + sandbox_policy: &SandboxPolicy, ) -> Result { - 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, - _sandbox_policy: SandboxPolicy, + _sandbox_policy: &SandboxPolicy, ) -> Result { 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, - sandbox_policy: SandboxPolicy, + sandbox_policy: &SandboxPolicy, ) -> Result { 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, - sandbox_policy: SandboxPolicy, - writable_roots: &[PathBuf], + sandbox_policy: &SandboxPolicy, ) -> Vec { - let (policies, cli_args): (Vec, Vec) = 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::::new(), + ) + } else { + let writable_roots = sandbox_policy.get_writable_roots(); + let (writable_folder_policies, cli_args): (Vec, Vec) = 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::::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 = 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 diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e7d4e32a..389694a3 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -27,5 +27,3 @@ pub use codex::Codex; mod approval_mode_cli_arg; #[cfg(feature = "cli")] pub use approval_mode_cli_arg::ApprovalModeCliArg; -#[cfg(feature = "cli")] -pub use approval_mode_cli_arg::SandboxModeCliArg; diff --git a/codex-rs/core/src/linux.rs b/codex-rs/core/src/linux.rs index 9f9d44b0..fac3ab30 100644 --- a/codex-rs/core/src/linux.rs +++ b/codex-rs/core/src/linux.rs @@ -32,14 +32,13 @@ use tokio::sync::Notify; pub async fn exec_linux( params: ExecParams, - writable_roots: &[PathBuf], ctrl_c: Arc, - sandbox_policy: SandboxPolicy, + sandbox_policy: &SandboxPolicy, ) -> Result { // Allow READ on / // Allow WRITE on /dev/null let ctrl_c_copy = ctrl_c.clone(); - let writable_roots_copy = writable_roots.to_vec(); + let sandbox_policy = sandbox_policy.clone(); // Isolate thread to run the sandbox from let tool_call_output = std::thread::spawn(move || { @@ -49,14 +48,7 @@ pub async fn exec_linux( .expect("Failed to create runtime"); rt.block_on(async { - if sandbox_policy.is_network_restricted() { - install_network_seccomp_filter_on_current_thread()?; - } - - if sandbox_policy.is_file_write_restricted() { - install_filesystem_landlock_rules_on_current_thread(writable_roots_copy)?; - } - + apply_sandbox_policy_to_current_thread(sandbox_policy)?; exec(params, ctrl_c_copy).await }) }) @@ -72,15 +64,31 @@ pub async fn exec_linux( } } +/// Apply sandbox policies inside this thread so only the child inherits +/// them, not the entire CLI process. +pub fn apply_sandbox_policy_to_current_thread(sandbox_policy: SandboxPolicy) -> Result<()> { + if !sandbox_policy.has_full_network_access() { + install_network_seccomp_filter_on_current_thread()?; + } + + if !sandbox_policy.has_full_disk_write_access() { + let writable_roots = sandbox_policy.get_writable_roots(); + install_filesystem_landlock_rules_on_current_thread(writable_roots)?; + } + + // TODO(ragona): Add appropriate restrictions if + // `sandbox_policy.has_full_disk_read_access()` is `false`. + + Ok(()) +} + /// Installs Landlock file-system rules on the current thread allowing read /// access to the entire file-system while restricting write access to /// `/dev/null` and the provided list of `writable_roots`. /// /// # Errors /// Returns [`CodexErr::Sandbox`] variants when the ruleset fails to apply. -pub fn install_filesystem_landlock_rules_on_current_thread( - writable_roots: Vec, -) -> Result<()> { +fn install_filesystem_landlock_rules_on_current_thread(writable_roots: Vec) -> Result<()> { let abi = ABI::V5; let access_rw = AccessFs::from_all(abi); let access_ro = AccessFs::from_read(abi); @@ -108,7 +116,7 @@ pub fn install_filesystem_landlock_rules_on_current_thread( /// Installs a seccomp filter that blocks outbound network access except for /// AF_UNIX domain sockets. -pub fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), SandboxErr> { +fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), SandboxErr> { // Build rule map. let mut rules: BTreeMap> = BTreeMap::new(); @@ -184,15 +192,14 @@ mod tests_linux { workdir: None, timeout_ms: Some(timeout_ms), }; - let res = process_exec_tool_call( - params, - SandboxType::LinuxSeccomp, - writable_roots, - Arc::new(Notify::new()), - SandboxPolicy::NetworkAndFileWriteRestricted, - ) - .await - .unwrap(); + + let sandbox_policy = + SandboxPolicy::new_read_only_policy_with_writable_roots(writable_roots); + let ctrl_c = Arc::new(Notify::new()); + let res = + process_exec_tool_call(params, SandboxType::LinuxSeccomp, ctrl_c, &sandbox_policy) + .await + .unwrap(); if res.exit_code != 0 { println!("stdout:\n{}", res.stdout); @@ -261,14 +268,11 @@ mod tests_linux { timeout_ms: Some(2_000), }; - let result = process_exec_tool_call( - params, - SandboxType::LinuxSeccomp, - &[], - Arc::new(Notify::new()), - SandboxPolicy::NetworkRestricted, - ) - .await; + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let ctrl_c = Arc::new(Notify::new()); + let result = + process_exec_tool_call(params, SandboxType::LinuxSeccomp, ctrl_c, &sandbox_policy) + .await; let (exit_code, stdout, stderr) = match result { Ok(output) => (output.exit_code, output.stdout, output.stderr), diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 139e2f2f..23c6e307 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -93,44 +93,169 @@ pub enum AskForApproval { } /// Determines execution restrictions for model shell commands -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] #[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 { + permissions: Vec, +} + +impl From> for SandboxPolicy { + fn from(permissions: Vec) -> Self { + Self { permissions } + } } 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 new_read_only_policy_with_writable_roots(writable_roots: &[PathBuf]) -> Self { + let mut permissions = Self::new_read_only_policy().permissions; + permissions.extend(writable_roots.iter().map(|folder| { + SandboxPermission::DiskWriteFolder { + folder: folder.clone(), + } + })); + Self { permissions } } - pub fn is_file_write_restricted(&self) -> bool { - matches!( - self, - SandboxPolicy::FileWriteRestricted | SandboxPolicy::NetworkAndFileWriteRestricted - ) + pub fn new_full_auto_policy() -> Self { + Self { + permissions: vec![ + SandboxPermission::DiskFullReadAccess, + SandboxPermission::DiskWritePlatformUserTempFolder, + SandboxPermission::DiskWriteCwd, + ], + } + } + + pub fn new_full_auto_policy_with_writable_roots(writable_roots: &[PathBuf]) -> Self { + let mut permissions = Self::new_full_auto_policy().permissions; + permissions.extend(writable_roots.iter().map(|folder| { + SandboxPermission::DiskWriteFolder { + folder: folder.clone(), + } + })); + Self { permissions } + } + + pub fn has_full_disk_read_access(&self) -> bool { + self.permissions + .iter() + .any(|perm| matches!(perm, SandboxPermission::DiskFullReadAccess)) + } + + 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 { + let mut writable_roots = Vec::::new(); + for perm in &self.permissions { + use SandboxPermission::*; + match perm { + DiskWritePlatformUserTempFolder => { + if cfg!(target_os = "macos") { + if let Some(tempdir) = std::env::var_os("TMPDIR") { + // Likely something that starts with /var/folders/... + let tmpdir_path = PathBuf::from(&tempdir); + if tmpdir_path.is_absolute() { + writable_roots.push(tmpdir_path.clone()); + match tmpdir_path.canonicalize() { + Ok(canonicalized) => { + // Likely something that starts with /private/var/folders/... + if canonicalized != tmpdir_path { + writable_roots.push(canonicalized); + } + } + Err(e) => { + tracing::error!("Failed to canonicalize TMPDIR: {e}"); + } + } + } else { + tracing::error!("TMPDIR is not an absolute path: {tempdir:?}"); + } + } + } + + // For Linux, should this be XDG_RUNTIME_DIR, /run/user/, 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)] +#[serde(rename_all = "kebab-case")] +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)] diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index e7841b2a..50ed3573 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -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>, ) -> 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, } } - }, + } } } diff --git a/codex-rs/core/src/seatbelt_readonly_policy.sbpl b/codex-rs/core/src/seatbelt_base_policy.sbpl similarity index 97% rename from codex-rs/core/src/seatbelt_readonly_policy.sbpl rename to codex-rs/core/src/seatbelt_base_policy.sbpl index c0632658..c9664651 100644 --- a/codex-rs/core/src/seatbelt_readonly_policy.sbpl +++ b/codex-rs/core/src/seatbelt_base_policy.sbpl @@ -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) diff --git a/codex-rs/core/tests/live_agent.rs b/codex-rs/core/tests/live_agent.rs index 23876498..7d2be33d 100644 --- a/codex-rs/core/tests/live_agent.rs +++ b/codex-rs/core/tests/live_agent.rs @@ -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, }, }) diff --git a/codex-rs/core/tests/previous_response_id.rs b/codex-rs/core/tests/previous_response_id.rs index 24c86916..c83d49ee 100644 --- a/codex-rs/core/tests/previous_response_id.rs +++ b/codex-rs/core/tests/previous_response_id.rs @@ -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, }, }) diff --git a/codex-rs/core/tests/stream_no_completed.rs b/codex-rs/core/tests/stream_no_completed.rs index e696ea97..e64281e3 100644 --- a/codex-rs/core/tests/stream_no_completed.rs +++ b/codex-rs/core/tests/stream_no_completed.rs @@ -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, }, }) diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index f5917a77..cd014e71 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -1,6 +1,5 @@ use clap::Parser; use clap::ValueEnum; -use codex_core::SandboxModeCliArg; use std::path::PathBuf; #[derive(Parser, Debug)] @@ -14,11 +13,9 @@ pub struct Cli { #[arg(long, short = 'm')] pub model: Option, - /// Configure the process restrictions when a command is executed. - /// - /// Uses OS-specific sandboxing tools; Seatbelt on OSX, landlock+seccomp on Linux. - #[arg(long = "sandbox", short = 's')] - pub sandbox_policy: Option, + /// Convenience alias for low-friction sandboxed automatic execution (network-disabled sandbox that can write to cwd and TMPDIR) + #[arg(long = "full-auto", default_value_t = false)] + pub full_auto: bool, /// Allow running Codex outside a Git repository. #[arg(long = "skip-git-repo-check", default_value_t = false)] diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 51e17267..9d5b9531 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -13,6 +13,7 @@ use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; +use codex_core::protocol::SandboxPolicy; use codex_core::util::is_inside_git_repo; use event_processor::EventProcessor; use owo_colors::OwoColorize; @@ -26,7 +27,7 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { let Cli { images, model, - sandbox_policy, + full_auto, skip_git_repo_check, disable_response_storage, color, @@ -61,13 +62,19 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { .with_writer(std::io::stderr) .try_init(); + let sandbox_policy = if full_auto { + Some(SandboxPolicy::new_full_auto_policy()) + } else { + None + }; + // Load configuration and determine approval policy let overrides = ConfigOverrides { model, // This CLI is intended to be headless and has no affordances for asking // the user for approval. approval_policy: Some(AskForApproval::Never), - sandbox_policy: sandbox_policy.map(Into::into), + sandbox_policy, disable_response_storage: if disable_response_storage { Some(true) } else { diff --git a/codex-rs/repl/src/cli.rs b/codex-rs/repl/src/cli.rs index a6b5bb73..567a8ea4 100644 --- a/codex-rs/repl/src/cli.rs +++ b/codex-rs/repl/src/cli.rs @@ -1,7 +1,6 @@ use clap::ArgAction; use clap::Parser; use codex_core::ApprovalModeCliArg; -use codex_core::SandboxModeCliArg; use std::path::PathBuf; /// Command‑line arguments. @@ -37,11 +36,9 @@ pub struct Cli { #[arg(long = "ask-for-approval", short = 'a')] pub approval_policy: Option, - /// Configure the process restrictions when a command is executed. - /// - /// Uses OS-specific sandboxing tools; Seatbelt on OSX, landlock+seccomp on Linux. - #[arg(long = "sandbox", short = 's')] - pub sandbox_policy: Option, + /// Convenience alias for low-friction sandboxed automatic execution (-a on-failure, network-disabled sandbox that can write to cwd and TMPDIR) + #[arg(long = "full-auto", default_value_t = false)] + pub full_auto: bool, /// Allow running Codex outside a Git repository. By default the CLI /// aborts early when the current working directory is **not** inside a diff --git a/codex-rs/repl/src/lib.rs b/codex-rs/repl/src/lib.rs index 17586332..d4bfbc2f 100644 --- a/codex-rs/repl/src/lib.rs +++ b/codex-rs/repl/src/lib.rs @@ -6,7 +6,9 @@ use std::sync::Arc; use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::protocol; +use codex_core::protocol::AskForApproval; use codex_core::protocol::FileChange; +use codex_core::protocol::SandboxPolicy; use codex_core::util::is_inside_git_repo; use codex_core::util::notify_on_sigint; use codex_core::Codex; @@ -76,11 +78,20 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { // Initialize logging before any other work so early errors are captured. init_logger(cli.verbose, !cli.no_ansi); + let (sandbox_policy, approval_policy) = if cli.full_auto { + ( + Some(SandboxPolicy::new_full_auto_policy()), + Some(AskForApproval::OnFailure), + ) + } else { + (None, cli.approval_policy.map(Into::into)) + }; + // Load config file and apply CLI overrides (model & approval policy) let overrides = ConfigOverrides { model: cli.model.clone(), - approval_policy: cli.approval_policy.map(Into::into), - sandbox_policy: cli.sandbox_policy.map(Into::into), + approval_policy, + sandbox_policy, disable_response_storage: if cli.disable_response_storage { Some(true) } else { diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index f336b0c3..1c00ae08 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -1,6 +1,5 @@ use clap::Parser; use codex_core::ApprovalModeCliArg; -use codex_core::SandboxModeCliArg; use std::path::PathBuf; #[derive(Parser, Debug)] @@ -21,11 +20,9 @@ pub struct Cli { #[arg(long = "ask-for-approval", short = 'a')] pub approval_policy: Option, - /// Configure the process restrictions when a command is executed. - /// - /// Uses OS-specific sandboxing tools; Seatbelt on OSX, landlock+seccomp on Linux. - #[arg(long = "sandbox", short = 's')] - pub sandbox_policy: Option, + /// Convenience alias for low-friction sandboxed automatic execution (-a on-failure, network-disabled sandbox that can write to cwd and TMPDIR) + #[arg(long = "full-auto", default_value_t = false)] + pub full_auto: bool, /// Allow running Codex outside a Git repository. #[arg(long = "skip-git-repo-check", default_value_t = false)] @@ -34,12 +31,4 @@ pub struct Cli { /// Disable server‑side response storage (sends the full conversation context with every request) #[arg(long = "disable-response-storage", default_value_t = false)] pub disable_response_storage: bool, - - /// Convenience alias for low-friction sandboxed automatic execution (-a on-failure, -s network-and-file-write-restricted) - #[arg(long = "full-auto", default_value_t = true)] - pub full_auto: bool, - - /// Convenience alias for supervised sandboxed execution (-a unless-allow-listed, -s network-and-file-write-restricted) - #[arg(long = "suggest", default_value_t = false)] - pub suggest: bool, } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index bf4ebec4..db43bde6 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -6,6 +6,8 @@ use app::App; use codex_core::config::Config; use codex_core::config::ConfigOverrides; +use codex_core::protocol::AskForApproval; +use codex_core::protocol::SandboxPolicy; use codex_core::util::is_inside_git_repo; use log_layer::TuiLogLayer; use std::fs::OpenOptions; @@ -33,12 +35,21 @@ pub use cli::Cli; pub fn run_main(cli: Cli) -> std::io::Result<()> { assert_env_var_set(); + let (sandbox_policy, approval_policy) = if cli.full_auto { + ( + Some(SandboxPolicy::new_full_auto_policy()), + Some(AskForApproval::OnFailure), + ) + } else { + (None, cli.approval_policy.map(Into::into)) + }; + let config = { // Load configuration and support CLI overrides. let overrides = ConfigOverrides { model: cli.model.clone(), - approval_policy: cli.approval_policy.map(Into::into), - sandbox_policy: cli.sandbox_policy.map(Into::into), + approval_policy, + sandbox_policy, disable_response_storage: if cli.disable_response_storage { Some(true) } else {