Merge 0d6f705d51 into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2025-06-24 14:52:51 -07:00
committed by GitHub
17 changed files with 283 additions and 517 deletions

View File

@@ -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"

View File

@@ -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);

View File

@@ -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) =>

View File

@@ -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"] || "";

View File

@@ -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<string>(),
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<string>) => 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);
});
});

View File

@@ -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<String>,
config_overrides: CliConfigOverrides,
codex_linux_sandbox_exe: Option<PathBuf>,
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()
}
}

View File

@@ -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,

View File

@@ -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<ApprovalModeCliArg> 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<Vec<SandboxPermission>>,
}
/// 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<SandboxPermission> {
let base_path = std::env::current_dir()?;
parse_sandbox_permission_with_base_path(raw, base_path)
}

View File

@@ -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;

View File

@@ -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<Vec<SandboxPermission>>,
/// Optional sandbox policy for the session. If omitted, Codex defaults to
/// the restrictive `read-only` policy.
#[serde(default)]
pub sandbox: Option<SandboxPolicy>,
/// 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<ReasoningSummary>,
}
fn deserialize_sandbox_permissions<'de, D>(
deserializer: D,
) -> Result<Option<Vec<SandboxPermission>>, D::Error>
where
D: serde::Deserializer<'de>,
{
let permissions: Option<Vec<String>> = 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::<Result<Vec<_>, 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<PathBuf> {
Ok(p)
}
pub fn parse_sandbox_permission_with_base_path(
raw: &str,
base_path: PathBuf,
) -> std::io::Result<SandboxPermission> {
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=<PATH> 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::<ConfigToml>(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,

View File

@@ -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<SandboxPermission>,
/// 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<PathBuf>,
/// 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<Vec<SandboxPermission>> for SandboxPolicy {
fn from(permissions: Vec<SandboxPermission>) -> 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<PathBuf> {
let mut writable_roots = Vec::<PathBuf>::new();
for perm in &self.permissions {
use SandboxPermission::*;
match perm {
DiskWritePlatformUserTempFolder => {
if cfg!(target_os = "macos") {
if let Some(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/<uid>, 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)]

View File

@@ -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<PathBuf>,

View File

@@ -31,7 +31,6 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> 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<PathBuf>) -> 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

View File

@@ -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<String>,
}
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,

View File

@@ -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<String>,
@@ -37,22 +37,14 @@ pub(crate) struct CodexToolCallParam {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub approval_policy: Option<CodexToolCallApprovalPolicy>,
/// 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<Vec<CodexToolCallSandboxPermission>>,
/// Individual config settings that will override what is in
/// CODEX_HOME/config.toml.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub config: Option<HashMap<String, serde_json::Value>>,
}
// 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<CodexToolCallApprovalPolicy> 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<CodexToolCallSandboxPermission> 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::<CodexToolCallParam>();
@@ -129,12 +83,12 @@ pub(crate) fn create_tool_for_codex_tool_call_param() -> Tool {
serde_json::from_value::<ToolInputSchema>(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<PathBuf>,
@@ -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::<Vec<_>>())
});
// 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<SandboxPolicy> = 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);
}
}

View File

@@ -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<PathBuf>,

View File

@@ -48,11 +48,11 @@ pub use cli::Cli;
pub fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> 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))
};