diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 66b4fa3e..bb533be1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -697,7 +697,6 @@ version = "0.0.0" dependencies = [ "anyhow", "clap", - "codex-common", "codex-core", "landlock", "libc", diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index deacca5f..a21cd4e7 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -1,7 +1,6 @@ use std::path::PathBuf; use codex_common::CliConfigOverrides; -use codex_common::SandboxPermissionOption; use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::exec::StdioPolicy; @@ -20,13 +19,11 @@ pub async fn run_command_under_seatbelt( ) -> anyhow::Result<()> { let SeatbeltCommand { full_auto, - sandbox, config_overrides, command, } = command; run_command_under_sandbox( full_auto, - sandbox, command, config_overrides, codex_linux_sandbox_exe, @@ -41,13 +38,11 @@ pub async fn run_command_under_landlock( ) -> anyhow::Result<()> { let LandlockCommand { full_auto, - sandbox, config_overrides, command, } = command; run_command_under_sandbox( full_auto, - sandbox, command, config_overrides, codex_linux_sandbox_exe, @@ -63,13 +58,12 @@ enum SandboxType { async fn run_command_under_sandbox( full_auto: bool, - sandbox: SandboxPermissionOption, command: Vec, config_overrides: CliConfigOverrides, codex_linux_sandbox_exe: Option, sandbox_type: SandboxType, ) -> anyhow::Result<()> { - let sandbox_policy = create_sandbox_policy(full_auto, sandbox); + let sandbox_policy = create_sandbox_policy(full_auto); let cwd = std::env::current_dir()?; let config = Config::load_with_cli_overrides( config_overrides @@ -110,13 +104,10 @@ async fn run_command_under_sandbox( handle_exit_status(status); } -pub fn create_sandbox_policy(full_auto: bool, sandbox: SandboxPermissionOption) -> SandboxPolicy { +pub fn create_sandbox_policy(full_auto: bool) -> SandboxPolicy { if full_auto { - SandboxPolicy::new_full_auto_policy() + SandboxPolicy::new_workspace_write_policy() } else { - match sandbox.permissions.map(Into::into) { - Some(sandbox_policy) => sandbox_policy, - None => SandboxPolicy::new_read_only_policy(), - } + SandboxPolicy::new_read_only_policy() } } diff --git a/codex-rs/cli/src/lib.rs b/codex-rs/cli/src/lib.rs index fa78d18a..c6d80c0a 100644 --- a/codex-rs/cli/src/lib.rs +++ b/codex-rs/cli/src/lib.rs @@ -5,7 +5,6 @@ pub mod proto; use clap::Parser; use codex_common::CliConfigOverrides; -use codex_common::SandboxPermissionOption; #[derive(Debug, Parser)] pub struct SeatbeltCommand { @@ -13,9 +12,6 @@ pub struct SeatbeltCommand { #[arg(long = "full-auto", default_value_t = false)] pub full_auto: bool, - #[clap(flatten)] - pub sandbox: SandboxPermissionOption, - #[clap(skip)] pub config_overrides: CliConfigOverrides, @@ -30,9 +26,6 @@ pub struct LandlockCommand { #[arg(long = "full-auto", default_value_t = false)] pub full_auto: bool, - #[clap(flatten)] - pub sandbox: SandboxPermissionOption, - #[clap(skip)] pub config_overrides: CliConfigOverrides, diff --git a/codex-rs/common/src/approval_mode_cli_arg.rs b/codex-rs/common/src/approval_mode_cli_arg.rs index 19954114..94bd8e89 100644 --- a/codex-rs/common/src/approval_mode_cli_arg.rs +++ b/codex-rs/common/src/approval_mode_cli_arg.rs @@ -1,13 +1,9 @@ //! Standard type to use with the `--approval-mode` CLI option. //! Available when the `cli` feature is enabled for the crate. -use clap::ArgAction; -use clap::Parser; use clap::ValueEnum; -use codex_core::config::parse_sandbox_permission_with_base_path; use codex_core::protocol::AskForApproval; -use codex_core::protocol::SandboxPermission; #[derive(Clone, Copy, Debug, ValueEnum)] #[value(rename_all = "kebab-case")] @@ -36,38 +32,3 @@ impl From for AskForApproval { } } } - -#[derive(Parser, Debug)] -pub struct SandboxPermissionOption { - /// Specify this flag multiple times to specify the full set of permissions - /// to grant to Codex. - /// - /// ```shell - /// codex -s disk-full-read-access \ - /// -s disk-write-cwd \ - /// -s disk-write-platform-user-temp-folder \ - /// -s disk-write-platform-global-temp-folder - /// ``` - /// - /// Note disk-write-folder takes a value: - /// - /// ```shell - /// -s disk-write-folder=$HOME/.pyenv/shims - /// ``` - /// - /// These permissions are quite broad and should be used with caution: - /// - /// ```shell - /// -s disk-full-write-access - /// -s network-full-access - /// ``` - #[arg(long = "sandbox-permission", short = 's', action = ArgAction::Append, value_parser = parse_sandbox_permission)] - pub permissions: Option>, -} - -/// Custom value-parser so we can keep the CLI surface small *and* -/// still handle the parameterised `disk-write-folder` case. -fn parse_sandbox_permission(raw: &str) -> std::io::Result { - let base_path = std::env::current_dir()?; - parse_sandbox_permission_with_base_path(raw, base_path) -} diff --git a/codex-rs/common/src/lib.rs b/codex-rs/common/src/lib.rs index c2283640..074f648f 100644 --- a/codex-rs/common/src/lib.rs +++ b/codex-rs/common/src/lib.rs @@ -6,8 +6,6 @@ pub mod elapsed; #[cfg(feature = "cli")] pub use approval_mode_cli_arg::ApprovalModeCliArg; -#[cfg(feature = "cli")] -pub use approval_mode_cli_arg::SandboxPermissionOption; #[cfg(any(feature = "cli", test))] mod config_override; diff --git a/codex-rs/config.md b/codex-rs/config.md index ffa735ff..0da42b9a 100644 --- a/codex-rs/config.md +++ b/codex-rs/config.md @@ -106,7 +106,6 @@ Here is an example of a `config.toml` that defines multiple profiles: ```toml model = "o3" approval_policy = "unless-allow-listed" -sandbox_permissions = ["disk-full-read-access"] disable_response_storage = false # Setting `profile` is equivalent to specifying `--profile o3` on the command @@ -170,31 +169,42 @@ To disable reasoning summaries, set `model_reasoning_summary` to `"none"` in you model_reasoning_summary = "none" # disable reasoning summaries ``` -## sandbox_permissions +## sandbox -List of permissions to grant to the sandbox that Codex uses to execute untrusted commands: +The `sandbox` configuration determines the _sandbox policy_ that Codex uses to execute untrusted commands. The `mode` determines the "base policy." Currently, only `workspace-write` supports additional configuration options, but this may change in the future. + +The default policy is `read-only`, which means commands can read any file on disk, but attempts to write a file or access the network will be blocked. ```toml -# This is comparable to --full-auto in the TypeScript Codex CLI, though -# specifying `disk-write-platform-global-temp-folder` adds /tmp as a writable -# folder in addition to $TMPDIR. -sandbox_permissions = [ - "disk-full-read-access", - "disk-write-platform-user-temp-folder", - "disk-write-platform-global-temp-folder", - "disk-write-cwd", -] +[sandbox] +mode = "read-only" ``` -To add additional writable folders, use `disk-write-folder`, which takes a parameter (this can be specified multiple times): +A more relaxed policy is `workspace-write`. When specified, the current working directory for the Codex task will be writable (as well as `$TMPDIR` on macOS). Note that the CLI defaults to using `cwd` where it was spawned, though this can be overridden using `--cwd/-C`. ```toml -sandbox_permissions = [ - # ... - "disk-write-folder=/Users/mbolin/.pyenv/shims", +[sandbox] +mode = "workspace-write" + +# By default, only the cwd for the Codex session will be writable (and $TMPDIR on macOS), +# but you can specify additional writable folders in this array. +writable_roots = [ + "/tmp", ] +network_access = false # Like read-only, this also defaults to false and can be omitted. ``` +To disable sandboxing altogether, specify `danger-full-access` like so: + +```toml +[sandbox] +mode = "danger-full-access" +``` + +This is reasonable to use if Codex is running in an environment that provides its own sandboxing (such as a Docker container) such that further sandboxing is unnecessary. + +Though using this option may also be necessary if you try to use Codex in environments where its native sandboxing mechanisms are unsupported, such as older Linux kernels or on Windows. + ## mcp_servers Defines the list of MCP servers that Codex can consult for tool use. Currently, only servers that are launched by executing a program that communicate over stdio are supported. For servers that use the SSE transport, consider an adapter like [mcp-proxy](https://github.com/sparfenyuk/mcp-proxy). diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 74798129..bea37e90 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -11,7 +11,6 @@ use crate::flags::OPENAI_DEFAULT_MODEL; use crate::model_provider_info::ModelProviderInfo; use crate::model_provider_info::built_in_model_providers; use crate::protocol::AskForApproval; -use crate::protocol::SandboxPermission; use crate::protocol::SandboxPolicy; use dirs::home_dir; use serde::Deserialize; @@ -241,11 +240,8 @@ pub struct ConfigToml { #[serde(default)] pub shell_environment_policy: ShellEnvironmentPolicyToml, - // The `default` attribute ensures that the field is treated as `None` when - // the key is omitted from the TOML. Without it, Serde treats the field as - // required because we supply a custom deserializer. - #[serde(default, deserialize_with = "deserialize_sandbox_permissions")] - pub sandbox_permissions: Option>, + /// If omitted, Codex defaults to the restrictive `read-only` policy. + pub sandbox: Option, /// Disable server-side response storage (sends the full conversation /// context with every request). Currently necessary for OpenAI customers @@ -296,32 +292,6 @@ pub struct ConfigToml { pub model_reasoning_summary: Option, } -fn deserialize_sandbox_permissions<'de, D>( - deserializer: D, -) -> Result>, D::Error> -where - D: serde::Deserializer<'de>, -{ - let permissions: Option> = Option::deserialize(deserializer)?; - - match permissions { - Some(raw_permissions) => { - let base_path = find_codex_home().map_err(serde::de::Error::custom)?; - - let converted = raw_permissions - .into_iter() - .map(|raw| { - parse_sandbox_permission_with_base_path(&raw, base_path.clone()) - .map_err(serde::de::Error::custom) - }) - .collect::, D::Error>>()?; - - Ok(Some(converted)) - } - None => Ok(None), - } -} - /// Optional overrides for user configuration (e.g., from CLI flags). #[derive(Default, Debug, Clone)] pub struct ConfigOverrides { @@ -369,20 +339,10 @@ impl Config { None => ConfigProfile::default(), }; - 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(), - } - } - }; + let sandbox_policy = sandbox_policy.unwrap_or_else(|| { + cfg.sandbox + .unwrap_or_else(SandboxPolicy::new_read_only_policy) + }); let mut model_providers = built_in_model_providers(); // Merge user-defined providers into the built-in list. @@ -520,50 +480,6 @@ pub fn log_dir(cfg: &Config) -> std::io::Result { Ok(p) } -pub fn parse_sandbox_permission_with_base_path( - raw: &str, - base_path: PathBuf, -) -> std::io::Result { - use SandboxPermission::*; - - if let Some(path) = raw.strip_prefix("disk-write-folder=") { - return if path.is_empty() { - Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "--sandbox-permission disk-write-folder= requires a non-empty PATH", - )) - } else { - use path_absolutize::*; - - let file = PathBuf::from(path); - let absolute_path = if file.is_relative() { - file.absolutize_from(base_path) - } else { - file.absolutize() - } - .map(|path| path.into_owned())?; - Ok(DiskWriteFolder { - folder: absolute_path, - }) - }; - } - - match raw { - "disk-full-read-access" => Ok(DiskFullReadAccess), - "disk-write-platform-user-temp-folder" => Ok(DiskWritePlatformUserTempFolder), - "disk-write-platform-global-temp-folder" => Ok(DiskWritePlatformGlobalTempFolder), - "disk-write-cwd" => Ok(DiskWriteCwd), - "disk-full-write-access" => Ok(DiskFullWriteAccess), - "network-full-access" => Ok(NetworkFullAccess), - _ => Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!( - "`{raw}` is not a recognised permission.\nRun with `--help` to see the accepted values." - ), - )), - } -} - #[cfg(test)] mod tests { #![allow(clippy::expect_used, clippy::unwrap_used)] @@ -573,51 +489,14 @@ mod tests { use pretty_assertions::assert_eq; use tempfile::TempDir; - /// Verify that the `sandbox_permissions` field on `ConfigToml` correctly - /// differentiates between a value that is completely absent in the - /// provided TOML (i.e. `None`) and one that is explicitly specified as an - /// empty array (i.e. `Some(vec![])`). This ensures that downstream logic - /// that treats these two cases differently (default read-only policy vs a - /// fully locked-down sandbox) continues to function. - #[test] - fn test_sandbox_permissions_none_vs_empty_vec() { - // Case 1: `sandbox_permissions` key is *absent* from the TOML source. - let toml_source_without_key = ""; - let cfg_without_key: ConfigToml = toml::from_str(toml_source_without_key) - .expect("TOML deserialization without key should succeed"); - assert!(cfg_without_key.sandbox_permissions.is_none()); - - // Case 2: `sandbox_permissions` is present but set to an *empty array*. - let toml_source_with_empty = "sandbox_permissions = []"; - let cfg_with_empty: ConfigToml = toml::from_str(toml_source_with_empty) - .expect("TOML deserialization with empty array should succeed"); - assert_eq!(Some(vec![]), cfg_with_empty.sandbox_permissions); - - // Case 3: `sandbox_permissions` contains a non-empty list of valid values. - let toml_source_with_values = r#" - sandbox_permissions = ["disk-full-read-access", "network-full-access"] - "#; - let cfg_with_values: ConfigToml = toml::from_str(toml_source_with_values) - .expect("TOML deserialization with valid permissions should succeed"); - - assert_eq!( - Some(vec![ - SandboxPermission::DiskFullReadAccess, - SandboxPermission::NetworkFullAccess - ]), - cfg_with_values.sandbox_permissions - ); - } - #[test] fn test_toml_parsing() { let history_with_persistence = r#" [history] persistence = "save-all" "#; - let history_with_persistence_cfg: ConfigToml = - toml::from_str::(history_with_persistence) - .expect("TOML deserialization should succeed"); + let history_with_persistence_cfg = toml::from_str::(history_with_persistence) + .expect("TOML deserialization should succeed"); assert_eq!( Some(History { persistence: HistoryPersistence::SaveAll, @@ -631,9 +510,8 @@ persistence = "save-all" persistence = "none" "#; - let history_no_persistence_cfg: ConfigToml = - toml::from_str::(history_no_persistence) - .expect("TOML deserialization should succeed"); + let history_no_persistence_cfg = toml::from_str::(history_no_persistence) + .expect("TOML deserialization should succeed"); assert_eq!( Some(History { persistence: HistoryPersistence::None, @@ -643,20 +521,47 @@ persistence = "none" ); } - /// Deserializing a TOML string containing an *invalid* permission should - /// fail with a helpful error rather than silently defaulting or - /// succeeding. #[test] - fn test_sandbox_permissions_illegal_value() { - let toml_bad = r#"sandbox_permissions = ["not-a-real-permission"]"#; + fn test_sandbox_config_parsing() { + let sandbox_full_access = r#" +[sandbox] +mode = "danger-full-access" +network_access = false # This should be ignored. +"#; + let sandbox_full_access_cfg = toml::from_str::(sandbox_full_access) + .expect("TOML deserialization should succeed"); + assert_eq!( + Some(SandboxPolicy::DangerFullAccess), + sandbox_full_access_cfg.sandbox + ); - let err = toml::from_str::(toml_bad) - .expect_err("Deserialization should fail for invalid permission"); + let sandbox_read_only = r#" +[sandbox] +mode = "read-only" +network_access = true # This should be ignored. +"#; - // Make sure the error message contains the invalid value so users have - // useful feedback. - let msg = err.to_string(); - assert!(msg.contains("not-a-real-permission")); + let sandbox_read_only_cfg = toml::from_str::(sandbox_read_only) + .expect("TOML deserialization should succeed"); + assert_eq!(Some(SandboxPolicy::ReadOnly), sandbox_read_only_cfg.sandbox); + + let sandbox_workspace_write = r#" +[sandbox] +mode = "workspace-write" +writable_roots = [ + "/tmp", +] +"#; + + let sandbox_workspace_write_cfg = toml::from_str::(sandbox_workspace_write) + .expect("TOML deserialization should succeed"); + assert_eq!( + Some(SandboxPolicy::WorkspaceWrite { + writable_roots: vec![PathBuf::from("/tmp")], + network_access: false + }), + sandbox_workspace_write_cfg.sandbox + ); } struct PrecedenceTestFixture { @@ -682,7 +587,6 @@ persistence = "none" let toml = r#" model = "o3" approval_policy = "unless-allow-listed" -sandbox_permissions = ["disk-full-read-access"] disable_response_storage = false # Can be used to determine which profile to use if not specified by diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index bf724048..3b37cb53 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -225,41 +225,20 @@ fn create_linux_sandbox_command_args( sandbox_policy: &SandboxPolicy, cwd: &Path, ) -> Vec { - let mut linux_cmd: Vec = vec![]; + #[expect(clippy::expect_used)] + let sandbox_policy_cwd = cwd.to_str().expect("cwd must be valid UTF-8").to_string(); - // Translate individual permissions. - // Use high-level helper methods to infer flags when we cannot see the - // exact permission list. - if sandbox_policy.has_full_disk_read_access() { - linux_cmd.extend(["-s", "disk-full-read-access"].map(String::from)); - } + #[expect(clippy::expect_used)] + let sandbox_policy_json = + serde_json::to_string(sandbox_policy).expect("Failed to serialize SandboxPolicy to JSON"); - if sandbox_policy.has_full_disk_write_access() { - linux_cmd.extend(["-s", "disk-full-write-access"].map(String::from)); - } else { - // Derive granular writable paths (includes cwd if `DiskWriteCwd` is - // present). - for root in sandbox_policy.get_writable_roots_with_cwd(cwd) { - // Check if this path corresponds exactly to cwd to map to - // `disk-write-cwd`, otherwise use the generic folder rule. - if root == cwd { - linux_cmd.extend(["-s", "disk-write-cwd"].map(String::from)); - } else { - linux_cmd.extend([ - "-s".to_string(), - format!("disk-write-folder={}", root.to_string_lossy()), - ]); - } - } - } - - if sandbox_policy.has_full_network_access() { - linux_cmd.extend(["-s", "network-full-access"].map(String::from)); - } - - // Separator so that command arguments starting with `-` are not parsed as - // options of the helper itself. - linux_cmd.push("--".to_string()); + let mut linux_cmd: Vec = vec![ + sandbox_policy_cwd, + sandbox_policy_json, + // Separator so that command arguments starting with `-` are not parsed as + // options of the helper itself. + "--".to_string(), + ]; // Append the original tool command. linux_cmd.extend(command); diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 737acc77..f3250de4 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -6,6 +6,7 @@ use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; +use std::str::FromStr; use mcp_types::CallToolResult; use serde::Deserialize; @@ -136,157 +137,110 @@ pub enum AskForApproval { Never, } -/// Determines execution restrictions for model shell commands -#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] -#[serde(rename_all = "kebab-case")] -pub struct SandboxPolicy { - permissions: Vec, +/// Determines execution restrictions for model shell commands. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "mode", rename_all = "kebab-case")] +pub enum SandboxPolicy { + /// No restrictions whatsoever. Use with caution. + #[serde(rename = "danger-full-access")] + DangerFullAccess, + + /// Read-only access to the entire file-system. + #[serde(rename = "read-only")] + ReadOnly, + + /// Same as `ReadOnly` but additionally grants write access to the current + /// working directory ("workspace"). + #[serde(rename = "workspace-write")] + WorkspaceWrite { + /// Additional folders (beyond cwd and possibly TMPDIR) that should be + /// writable from within the sandbox. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + writable_roots: Vec, + + /// When set to `true`, outbound network access is allowed. `false` by + /// default. + #[serde(default)] + network_access: bool, + }, } -impl From> for SandboxPolicy { - fn from(permissions: Vec) -> Self { - Self { permissions } +impl FromStr for SandboxPolicy { + type Err = serde_json::Error; + + fn from_str(s: &str) -> Result { + serde_json::from_str(s) } } impl SandboxPolicy { + /// Returns a policy with read-only disk access and no network. pub fn new_read_only_policy() -> Self { - Self { - permissions: vec![SandboxPermission::DiskFullReadAccess], - } + SandboxPolicy::ReadOnly } - 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(), + /// Returns a policy that can read the entire disk, but can only write to + /// the current working directory and the per-user tmp dir on macOS. It does + /// not allow network access. + pub fn new_workspace_write_policy() -> Self { + let mut writable_roots = vec![]; + + // Also include the per-user tmp dir on macOS. + if cfg!(target_os = "macos") { + if let Some(tmpdir) = std::env::var_os("TMPDIR") { + writable_roots.push(PathBuf::from(tmpdir)); } - })); - Self { permissions } - } + } - pub fn new_full_auto_policy() -> Self { - Self { - permissions: vec![ - SandboxPermission::DiskFullReadAccess, - SandboxPermission::DiskWritePlatformUserTempFolder, - SandboxPermission::DiskWriteCwd, - ], + SandboxPolicy::WorkspaceWrite { + writable_roots, + network_access: false, } } + /// Always returns `true` for now, as we do not yet support restricting read + /// access. pub fn has_full_disk_read_access(&self) -> bool { - self.permissions - .iter() - .any(|perm| matches!(perm, SandboxPermission::DiskFullReadAccess)) + true } pub fn has_full_disk_write_access(&self) -> bool { - self.permissions - .iter() - .any(|perm| matches!(perm, SandboxPermission::DiskFullWriteAccess)) + match self { + SandboxPolicy::DangerFullAccess => true, + SandboxPolicy::ReadOnly => false, + SandboxPolicy::WorkspaceWrite { .. } => false, + } } pub fn has_full_network_access(&self) -> bool { - self.permissions - .iter() - .any(|perm| matches!(perm, SandboxPermission::NetworkFullAccess)) + match self { + SandboxPolicy::DangerFullAccess => true, + SandboxPolicy::ReadOnly => false, + SandboxPolicy::WorkspaceWrite { network_access, .. } => *network_access, + } } + /// Returns the list of writable roots that should be passed down to the + /// Landlock rules installer, tailored to the current working directory. pub fn get_writable_roots_with_cwd(&self, cwd: &Path) -> 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 => { - writable_roots.push(cwd.to_path_buf()); - } - 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. - } + match self { + SandboxPolicy::DangerFullAccess => Vec::new(), + SandboxPolicy::ReadOnly => Vec::new(), + SandboxPolicy::WorkspaceWrite { writable_roots, .. } => { + let mut roots = writable_roots.clone(); + roots.push(cwd.to_path_buf()); + roots } } - writable_roots } + // TODO(mbolin): This conflates sandbox policy and approval policy and + // should go away. pub fn is_unrestricted(&self) -> bool { - self.has_full_disk_read_access() - && self.has_full_disk_write_access() - && self.has_full_network_access() + matches!(self, SandboxPolicy::DangerFullAccess) } } -/// 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, PartialEq)] diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 413fd23c..f14b28e7 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -1,7 +1,6 @@ use clap::Parser; use clap::ValueEnum; use codex_common::CliConfigOverrides; -use codex_common::SandboxPermissionOption; use std::path::PathBuf; #[derive(Parser, Debug)] @@ -23,9 +22,6 @@ pub struct Cli { #[arg(long = "full-auto", default_value_t = false)] pub full_auto: bool, - #[clap(flatten)] - pub sandbox: SandboxPermissionOption, - /// Tell the agent to use the specified directory as its working root. #[clap(long = "cd", short = 'C', value_name = "DIR")] pub cwd: Option, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 925e25d6..bd59e117 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -31,7 +31,6 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any model, config_profile, full_auto, - sandbox, cwd, skip_git_repo_check, color, @@ -85,9 +84,9 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any }; let sandbox_policy = if full_auto { - Some(SandboxPolicy::new_full_auto_policy()) + Some(SandboxPolicy::new_workspace_write_policy()) } else { - sandbox.permissions.clone().map(Into::into) + None }; // Load configuration and determine approval policy diff --git a/codex-rs/linux-sandbox/Cargo.toml b/codex-rs/linux-sandbox/Cargo.toml index 8d1e3a1c..c8cd1078 100644 --- a/codex-rs/linux-sandbox/Cargo.toml +++ b/codex-rs/linux-sandbox/Cargo.toml @@ -15,15 +15,9 @@ path = "src/lib.rs" workspace = true [dependencies] +anyhow = "1" clap = { version = "4", features = ["derive"] } codex-core = { path = "../core" } -codex-common = { path = "../common", features = ["cli"] } - -# Used for error handling in the helper that unifies runtime dispatch across -# binaries. -anyhow = "1" -# Required to construct a Tokio runtime for async execution of the caller's -# entry-point. tokio = { version = "1", features = ["rt-multi-thread"] } [dev-dependencies] diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index a8c73aa7..ac6ac445 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -1,13 +1,16 @@ use clap::Parser; -use codex_common::SandboxPermissionOption; use std::ffi::CString; +use std::path::PathBuf; use crate::landlock::apply_sandbox_policy_to_current_thread; #[derive(Debug, Parser)] pub struct LandlockCommand { - #[clap(flatten)] - pub sandbox: SandboxPermissionOption, + /// It is possible that the cwd used in the context of the sandbox policy + /// is different from the cwd of the process to spawn. + pub sandbox_policy_cwd: PathBuf, + + pub sandbox_policy: codex_core::protocol::SandboxPolicy, /// Full command args to run under landlock. #[arg(trailing_var_arg = true)] @@ -15,21 +18,13 @@ pub struct LandlockCommand { } pub fn run_main() -> ! { - let LandlockCommand { sandbox, command } = LandlockCommand::parse(); + let LandlockCommand { + sandbox_policy_cwd, + sandbox_policy, + command, + } = LandlockCommand::parse(); - let sandbox_policy = match sandbox.permissions.map(Into::into) { - Some(sandbox_policy) => sandbox_policy, - None => codex_core::protocol::SandboxPolicy::new_read_only_policy(), - }; - - let cwd = match std::env::current_dir() { - Ok(cwd) => cwd, - Err(e) => { - panic!("failed to getcwd(): {e:?}"); - } - }; - - if let Err(e) = apply_sandbox_policy_to_current_thread(&sandbox_policy, &cwd) { + if let Err(e) = apply_sandbox_policy_to_current_thread(&sandbox_policy, &sandbox_policy_cwd) { panic!("error running landlock: {e:?}"); } diff --git a/codex-rs/linux-sandbox/tests/landlock.rs b/codex-rs/linux-sandbox/tests/landlock.rs index 17bdd9d8..b495c346 100644 --- a/codex-rs/linux-sandbox/tests/landlock.rs +++ b/codex-rs/linux-sandbox/tests/landlock.rs @@ -46,7 +46,10 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { env: create_env_from_core_vars(), }; - let sandbox_policy = SandboxPolicy::new_read_only_policy_with_writable_roots(writable_roots); + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: writable_roots.to_vec(), + network_access: false, + }; let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program)); let ctrl_c = Arc::new(Notify::new()); diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index 03e72344..0afefc15 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -1,7 +1,6 @@ //! Configuration object accepted by the `codex` MCP tool-call. use codex_core::protocol::AskForApproval; -use codex_core::protocol::SandboxPolicy; use mcp_types::Tool; use mcp_types::ToolInputSchema; use schemars::JsonSchema; @@ -19,7 +18,7 @@ pub(crate) struct CodexToolCallParam { /// The *initial user prompt* to start the Codex conversation. pub prompt: String, - /// Optional override for the model name (e.g. "o3", "o4-mini") + /// Optional override for the model name (e.g. "o3", "o4-mini"). #[serde(default, skip_serializing_if = "Option::is_none")] pub model: Option, @@ -37,22 +36,14 @@ pub(crate) struct CodexToolCallParam { #[serde(default, skip_serializing_if = "Option::is_none")] pub approval_policy: Option, - /// Sandbox permissions using the same string values accepted by the CLI - /// (e.g. "disk-write-cwd", "network-full-access"). - #[serde(default, skip_serializing_if = "Option::is_none")] - pub sandbox_permissions: Option>, - /// Individual config settings that will override what is in /// CODEX_HOME/config.toml. #[serde(default, skip_serializing_if = "Option::is_none")] pub config: Option>, } -// Create custom enums for use with `CodexToolCallApprovalPolicy` where we -// intentionally exclude docstrings from the generated schema because they -// introduce anyOf in the the generated JSON schema, which makes it more complex -// without adding any real value since we aspire to use self-descriptive names. - +// Custom enum mirroring `AskForApproval`, but constrained to the subset we +// expose via the tool-call schema. #[derive(Debug, Clone, Deserialize, JsonSchema)] #[serde(rename_all = "kebab-case")] pub(crate) enum CodexToolCallApprovalPolicy { @@ -73,50 +64,12 @@ impl From for AskForApproval { } } -// TODO: Support additional writable folders via a separate property on -// CodexToolCallParam. - -#[derive(Debug, Clone, Deserialize, JsonSchema)] -#[serde(rename_all = "kebab-case")] -pub(crate) enum CodexToolCallSandboxPermission { - DiskFullReadAccess, - DiskWriteCwd, - DiskWritePlatformUserTempFolder, - DiskWritePlatformGlobalTempFolder, - DiskFullWriteAccess, - NetworkFullAccess, -} - -impl From for codex_core::protocol::SandboxPermission { - fn from(value: CodexToolCallSandboxPermission) -> Self { - match value { - CodexToolCallSandboxPermission::DiskFullReadAccess => { - codex_core::protocol::SandboxPermission::DiskFullReadAccess - } - CodexToolCallSandboxPermission::DiskWriteCwd => { - codex_core::protocol::SandboxPermission::DiskWriteCwd - } - CodexToolCallSandboxPermission::DiskWritePlatformUserTempFolder => { - codex_core::protocol::SandboxPermission::DiskWritePlatformUserTempFolder - } - CodexToolCallSandboxPermission::DiskWritePlatformGlobalTempFolder => { - codex_core::protocol::SandboxPermission::DiskWritePlatformGlobalTempFolder - } - CodexToolCallSandboxPermission::DiskFullWriteAccess => { - codex_core::protocol::SandboxPermission::DiskFullWriteAccess - } - CodexToolCallSandboxPermission::NetworkFullAccess => { - codex_core::protocol::SandboxPermission::NetworkFullAccess - } - } - } -} - +/// Builds a `Tool` definition (JSON schema etc.) for the Codex tool-call. pub(crate) fn create_tool_for_codex_tool_call_param() -> Tool { let schema = SchemaSettings::draft2019_09() .with(|s| { s.inline_subschemas = true; - s.option_add_null_type = false + s.option_add_null_type = false; }) .into_generator() .into_root_schema_for::(); @@ -129,12 +82,12 @@ pub(crate) fn create_tool_for_codex_tool_call_param() -> Tool { serde_json::from_value::(schema_value).unwrap_or_else(|e| { panic!("failed to create Tool from schema: {e}"); }); + Tool { name: "codex".to_string(), input_schema: tool_input_schema, description: Some( - "Run a Codex session. Accepts configuration parameters matching the Codex Config struct." - .to_string(), + "Run a Codex session. Accepts configuration parameters matching the Codex Config struct.".to_string(), ), annotations: None, } @@ -142,7 +95,7 @@ pub(crate) fn create_tool_for_codex_tool_call_param() -> Tool { impl CodexToolCallParam { /// Returns the initial user prompt to start the Codex conversation and the - /// Config. + /// effective Config object generated from the supplied parameters. pub fn into_config( self, codex_linux_sandbox_exe: Option, @@ -153,20 +106,18 @@ impl CodexToolCallParam { profile, cwd, approval_policy, - sandbox_permissions, config: cli_overrides, } = self; - let sandbox_policy = sandbox_permissions.map(|perms| { - SandboxPolicy::from(perms.into_iter().map(Into::into).collect::>()) - }); - // Build ConfigOverrides recognised by codex-core. + // Build the `ConfigOverrides` recognised by codex-core. let overrides = codex_core::config::ConfigOverrides { model, config_profile: profile, cwd: cwd.map(PathBuf::from), approval_policy: approval_policy.map(Into::into), - sandbox_policy, + // Note we may want to expose a field on CodexToolCallParam to + // facilitate configuring the sandbox policy. + sandbox_policy: None, model_provider: None, codex_linux_sandbox_exe, }; @@ -230,7 +181,7 @@ mod tests { "type": "string" }, "model": { - "description": "Optional override for the model name (e.g. \"o3\", \"o4-mini\")", + "description": "Optional override for the model name (e.g. \"o3\", \"o4-mini\").", "type": "string" }, "profile": { @@ -241,21 +192,6 @@ mod tests { "description": "The *initial user prompt* to start the Codex conversation.", "type": "string" }, - "sandbox-permissions": { - "description": "Sandbox permissions using the same string values accepted by the CLI (e.g. \"disk-write-cwd\", \"network-full-access\").", - "items": { - "enum": [ - "disk-full-read-access", - "disk-write-cwd", - "disk-write-platform-user-temp-folder", - "disk-write-platform-global-temp-folder", - "disk-full-write-access", - "network-full-access" - ], - "type": "string" - }, - "type": "array" - } }, "required": [ "prompt" diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index 4abd6841..e4ee752b 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -1,7 +1,6 @@ use clap::Parser; use codex_common::ApprovalModeCliArg; use codex_common::CliConfigOverrides; -use codex_common::SandboxPermissionOption; use std::path::PathBuf; #[derive(Parser, Debug)] @@ -30,9 +29,6 @@ pub struct Cli { #[arg(long = "full-auto", default_value_t = false)] pub full_auto: bool, - #[clap(flatten)] - pub sandbox: SandboxPermissionOption, - /// Tell the agent to use the specified directory as its working root. #[clap(long = "cd", short = 'C', value_name = "DIR")] pub cwd: Option, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 5f3e2d69..78fc2834 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -48,11 +48,11 @@ pub use cli::Cli; pub fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> std::io::Result<()> { let (sandbox_policy, approval_policy) = if cli.full_auto { ( - Some(SandboxPolicy::new_full_auto_policy()), + Some(SandboxPolicy::new_workspace_write_policy()), Some(AskForApproval::OnFailure), ) } else { - let sandbox_policy = cli.sandbox.permissions.clone().map(Into::into); + let sandbox_policy = None; (sandbox_policy, cli.approval_policy.map(Into::into)) };