diff --git a/README.md b/README.md index 24f362f77f..d06a0dff8c 100644 --- a/README.md +++ b/README.md @@ -469,7 +469,7 @@ export OPENAI_API_KEY="your-api-key-here" # Azure OpenAI export AZURE_OPENAI_API_KEY="your-azure-api-key-here" -export AZURE_OPENAI_API_VERSION="2025-03-01-preview" (Optional) +export AZURE_OPENAI_API_VERSION="2025-04-01-preview" (Optional) # OpenRouter export OPENROUTER_API_KEY="your-openrouter-key-here" diff --git a/codex-cli/src/cli.tsx b/codex-cli/src/cli.tsx index c7e5d9ff31..0442a6c377 100644 --- a/codex-cli/src/cli.tsx +++ b/codex-cli/src/cli.tsx @@ -45,6 +45,7 @@ import { createInputItem } from "./utils/input-utils"; import { initLogger } from "./utils/logger/log"; import { isModelSupportedForResponses } from "./utils/model-utils.js"; import { parseToolCall } from "./utils/parsers"; +import { providers } from "./utils/providers"; import { onExit, setInkRenderer } from "./utils/terminal"; import chalk from "chalk"; import { spawnSync } from "child_process"; @@ -327,26 +328,44 @@ try { // ignore errors } -if (cli.flags.login) { - apiKey = await fetchApiKey(client.issuer, client.client_id); - try { - const home = os.homedir(); - const authDir = path.join(home, ".codex"); - const authFile = path.join(authDir, "auth.json"); - if (fs.existsSync(authFile)) { - const data = JSON.parse(fs.readFileSync(authFile, "utf-8")); - savedTokens = data.tokens; +// Get provider-specific API key if not OpenAI +if (provider.toLowerCase() !== "openai") { + const providerInfo = providers[provider.toLowerCase()]; + if (providerInfo) { + const providerApiKey = process.env[providerInfo.envKey]; + if (providerApiKey) { + apiKey = providerApiKey; } - } catch { - /* ignore */ } -} else if (!apiKey) { - apiKey = await fetchApiKey(client.issuer, client.client_id); } + +// Only proceed with OpenAI auth flow if: +// 1. Provider is OpenAI and no API key is set, or +// 2. Login flag is explicitly set +if (provider.toLowerCase() === "openai" && !apiKey) { + if (cli.flags.login) { + apiKey = await fetchApiKey(client.issuer, client.client_id); + try { + const home = os.homedir(); + const authDir = path.join(home, ".codex"); + const authFile = path.join(authDir, "auth.json"); + if (fs.existsSync(authFile)) { + const data = JSON.parse(fs.readFileSync(authFile, "utf-8")); + savedTokens = data.tokens; + } + } catch { + /* ignore */ + } + } else { + apiKey = await fetchApiKey(client.issuer, client.client_id); + } +} + // Ensure the API key is available as an environment variable for legacy code process.env["OPENAI_API_KEY"] = apiKey; -if (cli.flags.free) { +// Only attempt credit redemption for OpenAI provider +if (cli.flags.free && provider.toLowerCase() === "openai") { // eslint-disable-next-line no-console console.log(`${chalk.bold("codex --free")} attempting to redeem credits...`); if (!savedTokens?.refresh_token) { @@ -379,13 +398,18 @@ if (!apiKey && !NO_API_KEY_REQUIRED.has(provider.toLowerCase())) { ? `You can create a key here: ${chalk.bold( chalk.underline("https://platform.openai.com/account/api-keys"), )}\n` - : provider.toLowerCase() === "gemini" + : provider.toLowerCase() === "azure" ? `You can create a ${chalk.bold( - `${provider.toUpperCase()}_API_KEY`, - )} ` + `in the ${chalk.bold(`Google AI Studio`)}.\n` - : `You can create a ${chalk.bold( - `${provider.toUpperCase()}_API_KEY`, - )} ` + `in the ${chalk.bold(`${provider}`)} dashboard.\n` + `${provider.toUpperCase()}_OPENAI_API_KEY`, + )} ` + + `in Azure AI Foundry portal at ${chalk.bold(chalk.underline("https://ai.azure.com"))}.\n` + : provider.toLowerCase() === "gemini" + ? `You can create a ${chalk.bold( + `${provider.toUpperCase()}_API_KEY`, + )} ` + `in the ${chalk.bold(`Google AI Studio`)}.\n` + : `You can create a ${chalk.bold( + `${provider.toUpperCase()}_API_KEY`, + )} ` + `in the ${chalk.bold(`${provider}`)} dashboard.\n` }`, ); process.exit(1); diff --git a/codex-cli/src/utils/agent/agent-loop.ts b/codex-cli/src/utils/agent/agent-loop.ts index cc57239b40..8a5adbeb23 100644 --- a/codex-cli/src/utils/agent/agent-loop.ts +++ b/codex-cli/src/utils/agent/agent-loop.ts @@ -800,7 +800,8 @@ export class AgentLoop { const responseCall = !this.config.provider || - this.config.provider?.toLowerCase() === "openai" + this.config.provider?.toLowerCase() === "openai" || + this.config.provider?.toLowerCase() === "azure" ? (params: ResponseCreateParams) => this.oai.responses.create(params) : (params: ResponseCreateParams) => @@ -1188,7 +1189,8 @@ export class AgentLoop { const responseCall = !this.config.provider || - this.config.provider?.toLowerCase() === "openai" + this.config.provider?.toLowerCase() === "openai" || + this.config.provider?.toLowerCase() === "azure" ? (params: ResponseCreateParams) => this.oai.responses.create(params) : (params: ResponseCreateParams) => diff --git a/codex-cli/src/utils/config.ts b/codex-cli/src/utils/config.ts index 51761bf6d4..3fafdb44e8 100644 --- a/codex-cli/src/utils/config.ts +++ b/codex-cli/src/utils/config.ts @@ -69,7 +69,7 @@ export const OPENAI_BASE_URL = process.env["OPENAI_BASE_URL"] || ""; export let OPENAI_API_KEY = process.env["OPENAI_API_KEY"] || ""; export const AZURE_OPENAI_API_VERSION = - process.env["AZURE_OPENAI_API_VERSION"] || "2025-03-01-preview"; + process.env["AZURE_OPENAI_API_VERSION"] || "2025-04-01-preview"; export const DEFAULT_REASONING_EFFORT = "high"; export const OPENAI_ORGANIZATION = process.env["OPENAI_ORGANIZATION"] || ""; diff --git a/codex-cli/tests/agent-azure-responses-endpoint.test.ts b/codex-cli/tests/agent-azure-responses-endpoint.test.ts new file mode 100644 index 0000000000..aecf587150 --- /dev/null +++ b/codex-cli/tests/agent-azure-responses-endpoint.test.ts @@ -0,0 +1,107 @@ +/** + * tests/agent-azure-responses-endpoint.test.ts + * + * Verifies that AgentLoop calls the `/responses` endpoint when provider is set to Azure. + */ + +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Fake stream that yields a completed response event +class FakeStream { + async *[Symbol.asyncIterator]() { + yield { + type: "response.completed", + response: { id: "azure_resp", status: "completed", output: [] }, + } as any; + } +} + +let lastCreateParams: any = null; + +vi.mock("openai", () => { + class FakeDefaultClient { + public responses = { + create: async (params: any) => { + lastCreateParams = params; + return new FakeStream(); + }, + }; + } + class FakeAzureClient { + public responses = { + create: async (params: any) => { + lastCreateParams = params; + return new FakeStream(); + }, + }; + } + class APIConnectionTimeoutError extends Error {} + return { + __esModule: true, + default: FakeDefaultClient, + AzureOpenAI: FakeAzureClient, + APIConnectionTimeoutError, + }; +}); + +// Stub approvals to bypass command approval logic +vi.mock("../src/approvals.js", () => ({ + __esModule: true, + alwaysApprovedCommands: new Set(), + canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false }), + isSafeCommand: () => null, +})); + +// Stub format-command to avoid formatting side effects +vi.mock("../src/format-command.js", () => ({ + __esModule: true, + formatCommandForDisplay: (cmd: Array) => cmd.join(" "), +})); + +// Stub internal logging to keep output clean +vi.mock("../src/utils/agent/log.js", () => ({ + __esModule: true, + log: () => {}, + isLoggingEnabled: () => false, +})); + +import { AgentLoop } from "../src/utils/agent/agent-loop.js"; + +describe("AgentLoop Azure provider responses endpoint", () => { + beforeEach(() => { + lastCreateParams = null; + }); + + it("calls the /responses endpoint when provider is azure", async () => { + const cfg: any = { + model: "test-model", + provider: "azure", + instructions: "", + disableResponseStorage: false, + notify: false, + }; + const loop = new AgentLoop({ + additionalWritableRoots: [], + model: cfg.model, + config: cfg, + instructions: cfg.instructions, + approvalPolicy: { mode: "suggest" } as any, + onItem: () => {}, + onLoading: () => {}, + getCommandConfirmation: async () => ({ review: "yes" }) as any, + onLastResponseId: () => {}, + }); + + await loop.run([ + { + type: "message", + role: "user", + content: [{ type: "input_text", text: "hello" }], + }, + ]); + + expect(lastCreateParams).not.toBeNull(); + expect(lastCreateParams.model).toBe(cfg.model); + expect(Array.isArray(lastCreateParams.input)).toBe(true); + }); +}); diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index deacca5f28..a21cd4e73e 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 fa78d18ab4..c6d80c0adf 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 199541148a..bd539ceb51 100644 --- a/codex-rs/common/src/approval_mode_cli_arg.rs +++ b/codex-rs/common/src/approval_mode_cli_arg.rs @@ -1,29 +1,19 @@ //! 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")] pub enum ApprovalModeCliArg { - /// Run all commands without asking for user approval. - /// Only asks for approval if a command fails to execute, in which case it - /// will escalate to the user to ask for un-sandboxed execution. + /// Run all commands without asking for user approval. Only escalates when a command fails. OnFailure, - /// Only run "known safe" commands (e.g. ls, cat, sed) without - /// asking for user approval. Will escalate to the user if the model - /// proposes a command that is not allow-listed. + /// Only run “known safe” commands (e.g. ls, cat, sed) automatically. UnlessAllowListed, - /// Never ask for user approval - /// Execution failures are immediately returned to the model. + /// Never ask for user approval; return execution failures directly to the model. Never, } @@ -36,38 +26,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 2bcd505fc3..9b164cd739 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/core/src/config.rs b/codex-rs/core/src/config.rs index 74798129ba..7e698a211d 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; @@ -244,8 +243,10 @@ pub struct ConfigToml { // 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>, + /// Optional sandbox policy for the session. If omitted, Codex defaults to + /// the restrictive `read-only` policy. + #[serde(default)] + pub sandbox: Option, /// Disable server-side response storage (sends the full conversation /// context with every request). Currently necessary for OpenAI customers @@ -296,32 +297,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 +344,11 @@ 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 + .clone() + .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 +486,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,42 +495,6 @@ 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#" @@ -643,22 +529,6 @@ 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"]"#; - - 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")); - } - struct PrecedenceTestFixture { cwd: TempDir, codex_home: TempDir, diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 737acc7732..283d6d8ca8 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -136,157 +136,127 @@ 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, +/// Additional configuration shared by the restricted sandbox variants +/// (`ReadOnly` and `WorkspaceWrite`). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RestrictedSandboxConfig { + /// Additional folders that should be writable from within the sandbox. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub writable_roots: Vec, + + /// When set to `true`, outbound network access is allowed. `false` by + /// default. Ignored for the `DangerFullAccess` variant where network is + /// always allowed. + #[serde(default)] + pub network_access: bool, } -impl From> for SandboxPolicy { - fn from(permissions: Vec) -> Self { - Self { permissions } - } +/// Determines execution restrictions for model shell commands. +/// +/// Instead of the previous "bag of permissions" approach, the policy is now +/// chiefly expressed via a *mode* with a handful of per-mode configuration +/// knobs. This makes the user-facing TOML much easier to reason about while +/// still letting us derive the granular permissions required by the lower +/// layers (seccomp / Landlock, etc.). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +// The `tag = "sandbox"` ensures that a standalone string such as +// `sandbox = "read-only"` in user configuration files still deserialises into +// the enum, while tables like +// +// ```toml +// [sandbox] +// mode = "workspace-write" +// writable_roots = ["/tmp"] +// network_access = true +// ``` +// also work. +pub enum SandboxPolicy { + /// No restrictions whatsoever. Use with caution. + #[serde(rename = "danger-full-access")] + DangerFullAccess, + + /// Read-only access to the entire file-system. Network is *off* by default + /// but can be enabled. + #[serde(rename = "read-only")] + ReadOnly { network_access: bool }, + + /// Same as `ReadOnly` but additionally grants write access to the current + /// working directory ("workspace"). + #[serde(rename = "workspace-write")] + WorkspaceWrite(RestrictedSandboxConfig), } 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 { + network_access: false, } } - 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(), + /// Convenience helper that mirrors the semantics of the previous + /// `new_full_auto_policy()` constructor: read access everywhere, write + /// access to the workspace & tmp dirs, and network enabled. + 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(RestrictedSandboxConfig { + 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 { network_access } => *network_access, + SandboxPolicy::WorkspaceWrite(cfg) => cfg.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(cfg) => { + let mut roots = cfg.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 413fd23cb7..f14b28e702 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 925e25d670..bd59e117a2 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/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index a8c73aa75d..7ed35ce020 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -1,26 +1,19 @@ use clap::Parser; -use codex_common::SandboxPermissionOption; use std::ffi::CString; use crate::landlock::apply_sandbox_policy_to_current_thread; #[derive(Debug, Parser)] pub struct LandlockCommand { - #[clap(flatten)] - pub sandbox: SandboxPermissionOption, - /// Full command args to run under landlock. #[arg(trailing_var_arg = true)] pub command: Vec, } pub fn run_main() -> ! { - let LandlockCommand { sandbox, command } = LandlockCommand::parse(); + let LandlockCommand { 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 sandbox_policy = codex_core::protocol::SandboxPolicy::new_read_only_policy(); let cwd = match std::env::current_dir() { Ok(cwd) => cwd, diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index 03e7234449..0a013f0ea4 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -19,7 +19,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 +37,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 +65,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 +83,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 +96,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,14 +107,15 @@ 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. + // No per-tool-call override for sandbox policy now that the CLI + // `--sandbox-permission` flag is gone. We rely on the server-side + // configuration defaults instead. + let sandbox_policy: Option = None; + + // Build the `ConfigOverrides` recognised by codex-core. let overrides = codex_core::config::ConfigOverrides { model, config_profile: profile, @@ -182,86 +137,3 @@ impl CodexToolCallParam { Ok((prompt, cfg)) } } - -#[cfg(test)] -mod tests { - use super::*; - use pretty_assertions::assert_eq; - - /// We include a test to verify the exact JSON schema as "executable - /// documentation" for the schema. When can track changes to this test as a - /// way to audit changes to the generated schema. - /// - /// Seeing the fully expanded schema makes it easier to casually verify that - /// the generated JSON for enum types such as "approval-policy" is compact. - /// Ideally, modelcontextprotocol/inspector would provide a simpler UI for - /// enum fields versus open string fields to take advantage of this. - /// - /// As of 2025-05-04, there is an open PR for this: - /// https://github.com/modelcontextprotocol/inspector/pull/196 - #[test] - fn verify_codex_tool_json_schema() { - let tool = create_tool_for_codex_tool_call_param(); - #[expect(clippy::expect_used)] - let tool_json = serde_json::to_value(&tool).expect("tool serializes"); - let expected_tool_json = serde_json::json!({ - "name": "codex", - "description": "Run a Codex session. Accepts configuration parameters matching the Codex Config struct.", - "inputSchema": { - "type": "object", - "properties": { - "approval-policy": { - "description": "Execution approval policy expressed as the kebab-case variant name (`unless-allow-listed`, `auto-edit`, `on-failure`, `never`).", - "enum": [ - "auto-edit", - "unless-allow-listed", - "on-failure", - "never" - ], - "type": "string" - }, - "config": { - "description": "Individual config settings that will override what is in CODEX_HOME/config.toml.", - "additionalProperties": true, - "type": "object" - }, - "cwd": { - "description": "Working directory for the session. If relative, it is resolved against the server process's current working directory.", - "type": "string" - }, - "model": { - "description": "Optional override for the model name (e.g. \"o3\", \"o4-mini\")", - "type": "string" - }, - "profile": { - "description": "Configuration profile from config.toml to specify default options.", - "type": "string" - }, - "prompt": { - "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" - ] - } - }); - assert_eq!(expected_tool_json, tool_json); - } -} diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index 4abd684144..e4ee752ba9 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 5f3e2d69b5..78fc283461 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)) };