diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 961d0927d1..22bbdc07c7 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -504,6 +504,7 @@ dependencies = [ "mime_guess", "openssl-sys", "patch", + "path-absolutize", "predicates", "rand", "reqwest", diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index fa0a14e6cb..ba6b15d99f 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -3,11 +3,9 @@ mod landlock; mod proto; mod seatbelt; -use std::path::PathBuf; - -use clap::ArgAction; use clap::Parser; use codex_core::protocol::SandboxPolicy; +use codex_core::SandboxPermissionOption; use codex_exec::Cli as ExecCli; use codex_repl::Cli as ReplCli; use codex_tui::Cli as TuiCli; @@ -67,14 +65,13 @@ enum DebugCommand { #[derive(Debug, Parser)] struct SeatbeltCommand { - /// Writable folder for sandbox (can be specified multiple times). - #[arg(long = "writable-root", short = 'w', value_name = "DIR", action = ArgAction::Append, use_value_delimiter = false)] - writable_roots: Vec, - /// 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, + #[clap(flatten)] + pub sandbox: SandboxPermissionOption, + /// Full command args to run under seatbelt. #[arg(trailing_var_arg = true)] command: Vec, @@ -82,14 +79,13 @@ struct SeatbeltCommand { #[derive(Debug, Parser)] struct LandlockCommand { - /// Writable folder for sandbox (can be specified multiple times). - #[arg(long = "writable-root", short = 'w', value_name = "DIR", action = ArgAction::Append, use_value_delimiter = false)] - writable_roots: Vec, - /// 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, + #[clap(flatten)] + sandbox: SandboxPermissionOption, + /// Full command args to run under landlock. #[arg(trailing_var_arg = true)] command: Vec, @@ -118,19 +114,19 @@ async fn main() -> anyhow::Result<()> { Some(Subcommand::Debug(debug_args)) => match debug_args.cmd { DebugCommand::Seatbelt(SeatbeltCommand { command, - writable_roots, + sandbox, full_auto, }) => { - let sandbox_policy = create_sandbox_policy(full_auto, &writable_roots); + let sandbox_policy = create_sandbox_policy(full_auto, sandbox); seatbelt::run_seatbelt(command, sandbox_policy).await?; } #[cfg(target_os = "linux")] DebugCommand::Landlock(LandlockCommand { command, - writable_roots, + sandbox, full_auto, }) => { - let sandbox_policy = create_sandbox_policy(full_auto, &writable_roots); + let sandbox_policy = create_sandbox_policy(full_auto, sandbox); landlock::run_landlock(command, sandbox_policy)?; } #[cfg(not(target_os = "linux"))] @@ -143,10 +139,13 @@ async fn main() -> anyhow::Result<()> { Ok(()) } -fn create_sandbox_policy(full_auto: bool, writable_roots: &[PathBuf]) -> SandboxPolicy { +fn create_sandbox_policy(full_auto: bool, sandbox: SandboxPermissionOption) -> SandboxPolicy { if full_auto { - SandboxPolicy::new_full_auto_policy_with_writable_roots(writable_roots) + SandboxPolicy::new_full_auto_policy() } else { - SandboxPolicy::new_read_only_policy_with_writable_roots(writable_roots) + match sandbox.permissions.map(Into::into) { + Some(sandbox_policy) => sandbox_policy, + None => SandboxPolicy::new_read_only_policy(), + } } } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index daadec7294..0ed550f9a8 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -21,6 +21,7 @@ fs-err = "3.1.0" futures = "0.3" mime_guess = "2.0" patch = "0.7" +path-absolutize = "3.1.1" rand = "0.9" reqwest = { version = "0.12", features = ["json", "stream"] } serde = { version = "1", features = ["derive"] } diff --git a/codex-rs/core/src/approval_mode_cli_arg.rs b/codex-rs/core/src/approval_mode_cli_arg.rs index 8154e49fe9..f4e64febae 100644 --- a/codex-rs/core/src/approval_mode_cli_arg.rs +++ b/codex-rs/core/src/approval_mode_cli_arg.rs @@ -1,9 +1,14 @@ //! Standard type to use with the `--approval-mode` CLI option. //! Available when the `cli` feature is enabled for the crate. +use std::path::PathBuf; + +use clap::ArgAction; +use clap::Parser; use clap::ValueEnum; use crate::protocol::AskForApproval; +use crate::protocol::SandboxPermission; #[derive(Clone, Copy, Debug, ValueEnum)] #[value(rename_all = "kebab-case")] @@ -32,3 +37,84 @@ 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) +} + +pub(crate) 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." + ), + ) + ), + } +} diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 55efe5a94c..c9bfa138be 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1,3 +1,4 @@ +use crate::approval_mode_cli_arg::parse_sandbox_permission_with_base_path; use crate::flags::OPENAI_DEFAULT_MODEL; use crate::protocol::AskForApproval; use crate::protocol::SandboxPermission; @@ -40,6 +41,10 @@ pub struct ConfigToml { /// Default approval policy for executing commands. pub approval_policy: Option, + // 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>, /// Disable server-side response storage (sends the full conversation @@ -74,6 +79,32 @@ impl ConfigToml { } } +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 = codex_dir().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 { @@ -174,3 +205,60 @@ pub fn log_dir() -> std::io::Result { p.push("log"); Ok(p) } + +#[cfg(test)] +mod tests { + use super::*; + + /// 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 + ); + } + + /// 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"]"#; + + let err = toml::from_str::(toml_bad) + .expect_err("Deserialization should fail for invalid permission"); + + // 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")); + } +} diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 389694a38b..b1c746beb2 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -27,3 +27,5 @@ 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::SandboxPermissionOption; diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 23c6e307bb..5c2d35c159 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -132,16 +132,6 @@ impl SandboxPolicy { } } - 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() diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index cd014e71f2..1b32b52206 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -1,5 +1,6 @@ use clap::Parser; use clap::ValueEnum; +use codex_core::SandboxPermissionOption; use std::path::PathBuf; #[derive(Parser, Debug)] @@ -17,6 +18,9 @@ pub struct Cli { #[arg(long = "full-auto", default_value_t = false)] pub full_auto: bool, + #[clap(flatten)] + pub sandbox: SandboxPermissionOption, + /// Allow running Codex outside a Git repository. #[arg(long = "skip-git-repo-check", default_value_t = false)] pub skip_git_repo_check: bool, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 9d5b95316a..1541102e32 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -28,6 +28,7 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { images, model, full_auto, + sandbox, skip_git_repo_check, disable_response_storage, color, @@ -65,7 +66,7 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { let sandbox_policy = if full_auto { Some(SandboxPolicy::new_full_auto_policy()) } else { - None + sandbox.permissions.clone().map(Into::into) }; // Load configuration and determine approval policy diff --git a/codex-rs/repl/src/cli.rs b/codex-rs/repl/src/cli.rs index 567a8ea491..c9fa1ee9ae 100644 --- a/codex-rs/repl/src/cli.rs +++ b/codex-rs/repl/src/cli.rs @@ -1,6 +1,7 @@ use clap::ArgAction; use clap::Parser; use codex_core::ApprovalModeCliArg; +use codex_core::SandboxPermissionOption; use std::path::PathBuf; /// Command‑line arguments. @@ -40,6 +41,9 @@ pub struct Cli { #[arg(long = "full-auto", default_value_t = false)] pub full_auto: bool, + #[clap(flatten)] + pub sandbox: SandboxPermissionOption, + /// Allow running Codex outside a Git repository. By default the CLI /// aborts early when the current working directory is **not** inside a /// Git repo because most agents rely on `git` for interacting with the diff --git a/codex-rs/repl/src/lib.rs b/codex-rs/repl/src/lib.rs index d4bfbc2f95..fea756b773 100644 --- a/codex-rs/repl/src/lib.rs +++ b/codex-rs/repl/src/lib.rs @@ -84,7 +84,8 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { Some(AskForApproval::OnFailure), ) } else { - (None, cli.approval_policy.map(Into::into)) + let sandbox_policy = cli.sandbox.permissions.clone().map(Into::into); + (sandbox_policy, cli.approval_policy.map(Into::into)) }; // Load config file and apply CLI overrides (model & approval policy) diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index 1c00ae0862..43a1f5b165 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -1,5 +1,6 @@ use clap::Parser; use codex_core::ApprovalModeCliArg; +use codex_core::SandboxPermissionOption; use std::path::PathBuf; #[derive(Parser, Debug)] @@ -24,6 +25,9 @@ pub struct Cli { #[arg(long = "full-auto", default_value_t = false)] pub full_auto: bool, + #[clap(flatten)] + pub sandbox: SandboxPermissionOption, + /// Allow running Codex outside a Git repository. #[arg(long = "skip-git-repo-check", default_value_t = false)] pub skip_git_repo_check: bool, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index db43bde6f1..e23b8c6902 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -41,7 +41,8 @@ pub fn run_main(cli: Cli) -> std::io::Result<()> { Some(AskForApproval::OnFailure), ) } else { - (None, cli.approval_policy.map(Into::into)) + let sandbox_policy = cli.sandbox.permissions.clone().map(Into::into); + (sandbox_policy, cli.approval_policy.map(Into::into)) }; let config = {