valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: add support for --sandbox flag (#1476)

Browse Source 
On a high-level, we try to design `config.toml` so that you don't have
to "comment out a lot of stuff" when testing different options.

Previously, defining a sandbox policy was somewhat at odds with this
principle because you would define the policy as attributes of
`[sandbox]` like so:

```toml
[sandbox]
mode = "workspace-write"
writable_roots = [ "/tmp" ]
```

but if you wanted to temporarily change to a read-only sandbox, you
might feel compelled to modify your file to be:

```toml
[sandbox]
mode = "read-only"
# mode = "workspace-write"
# writable_roots = [ "/tmp" ]
```

Technically, commenting out `writable_roots` would not be strictly
necessary, as `mode = "read-only"` would ignore `writable_roots`, but
it's still a reasonable thing to do to keep things tidy.

Currently, the various values for `mode` do not support that many
attributes, so this is not that hard to maintain, but one could imagine
this becoming more complex in the future.

In this PR, we change Codex CLI so that it no longer recognizes
`[sandbox]`. Instead, it introduces a top-level option, `sandbox_mode`,
and `[sandbox_workspace_write]` is used to further configure the sandbox
when when `sandbox_mode = "workspace-write"` is used:

```toml
sandbox_mode = "workspace-write"

[sandbox_workspace_write]
writable_roots = [ "/tmp" ]
```

This feels a bit more future-proof in that it is less tedious to
configure different sandboxes:

```toml
sandbox_mode = "workspace-write"

[sandbox_read_only]
# read-only options here...

[sandbox_workspace_write]
writable_roots = [ "/tmp" ]

[sandbox_danger_full_access]
# danger-full-access options here...
```

In this scheme, you never need to comment out the configuration for an
individual sandbox type: you only need to redefine `sandbox_mode`.

Relatedly, previous to this change, a user had to do `-c
sandbox.mode=read-only` to change the mode on the command line. With
this change, things are arguably a bit cleaner because the equivalent
option is `-c sandbox_mode=read-only` (and now `-c
sandbox_workspace_write=...` can be set separately).

Though more importantly, we introduce the `-s/--sandbox` option to the
CLI, which maps directly to `sandbox_mode` in `config.toml`, making
config override behavior easier to reason about. Moreover, as you can
see in the updates to the various Markdown files, it is much easier to
explain how to configure sandboxing when things like `--sandbox
read-only` can be used as an example.

Relatedly, this cleanup also made it straightforward to add support for
a `sandbox` option for Codex when used as an MCP server (see the changes
to `mcp-server/src/codex_tool_config.rs`).

Fixes https://github.com/openai/codex/issues/1248.
This commit is contained in:
Michael Bolin
2025-07-07 22:31:30 -07:00
committed by GitHub
parent 0a44c42533
commit e0c08cea4f
13 changed files with 233 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
codex-rs/core/src/config.rs
View File

@@ -3,6 +3,8 @@ use crate::config_types::History;
use crate::config_types::McpServerConfig;
use crate::config_types::ReasoningEffort;
use crate::config_types::ReasoningSummary;
use crate::config_types::SandboxMode;
use crate::config_types::SandboxWorkplaceWrite;
use crate::config_types::ShellEnvironmentPolicy;
use crate::config_types::ShellEnvironmentPolicyToml;
use crate::config_types::Tui;
@@ -253,8 +255,11 @@ pub struct ConfigToml {
#[serde(default)]
pub shell_environment_policy: ShellEnvironmentPolicyToml,
/// If omitted, Codex defaults to the restrictive `read-only` policy.
pub sandbox: Option<SandboxPolicy>,
/// Sandbox mode to use.
pub sandbox_mode: Option<SandboxMode>,
/// Sandbox configuration to apply if `sandbox` is `WorkspaceWrite`.
pub sandbox_workspace_write: Option<SandboxWorkplaceWrite>,
/// Disable server-side response storage (sends the full conversation
/// context with every request). Currently necessary for OpenAI customers
@@ -305,13 +310,33 @@ pub struct ConfigToml {
pub model_reasoning_summary: Option<ReasoningSummary>,
}
impl ConfigToml {
/// Derive the effective sandbox policy from the configuration.
fn derive_sandbox_policy(&self, sandbox_mode_override: Option<SandboxMode>) -> SandboxPolicy {
let resolved_sandbox_mode = sandbox_mode_override
.or(self.sandbox_mode)
.unwrap_or_default();
match resolved_sandbox_mode {
SandboxMode::ReadOnly => SandboxPolicy::new_read_only_policy(),
SandboxMode::WorkspaceWrite => match self.sandbox_workspace_write.as_ref() {
Some(s) => SandboxPolicy::WorkspaceWrite {
writable_roots: s.writable_roots.clone(),
network_access: s.network_access,
},
None => SandboxPolicy::new_workspace_write_policy(),
},
SandboxMode::DangerFullAccess => SandboxPolicy::DangerFullAccess,
}
}
}
/// Optional overrides for user configuration (e.g., from CLI flags).
#[derive(Default, Debug, Clone)]
pub struct ConfigOverrides {
pub model: Option<String>,
pub cwd: Option<PathBuf>,
pub approval_policy: Option<AskForApproval>,
pub sandbox_policy: Option<SandboxPolicy>,
pub sandbox_mode: Option<SandboxMode>,
pub model_provider: Option<String>,
pub config_profile: Option<String>,
pub codex_linux_sandbox_exe: Option<PathBuf>,
@@ -332,16 +357,16 @@ impl Config {
model,
cwd,
approval_policy,
sandbox_policy,
sandbox_mode,
model_provider,
config_profile: config_profile_key,
codex_linux_sandbox_exe,
} = overrides;
let config_profile = match config_profile_key.or(cfg.profile) {
let config_profile = match config_profile_key.as_ref().or(cfg.profile.as_ref()) {
Some(key) => cfg
.profiles
.get(&key)
.get(key)
.ok_or_else(|| {
std::io::Error::new(
std::io::ErrorKind::NotFound,
@@ -352,10 +377,7 @@ impl Config {
None => ConfigProfile::default(),
};
let sandbox_policy = sandbox_policy.unwrap_or_else(|| {
cfg.sandbox
.unwrap_or_else(SandboxPolicy::new_read_only_policy)
});
let sandbox_policy = cfg.derive_sandbox_policy(sandbox_mode);
let mut model_providers = built_in_model_providers();
// Merge user-defined providers into the built-in list.
@@ -549,30 +571,38 @@ persistence = "none"
#[test]
fn test_sandbox_config_parsing() {
let sandbox_full_access = r#"
[sandbox]
mode = "danger-full-access"
sandbox_mode = "danger-full-access"
[sandbox_workspace_write]
network_access = false # This should be ignored.
"#;
let sandbox_full_access_cfg = toml::from_str::<ConfigToml>(sandbox_full_access)
.expect("TOML deserialization should succeed");
let sandbox_mode_override = None;
assert_eq!(
Some(SandboxPolicy::DangerFullAccess),
sandbox_full_access_cfg.sandbox
SandboxPolicy::DangerFullAccess,
sandbox_full_access_cfg.derive_sandbox_policy(sandbox_mode_override)
);
let sandbox_read_only = r#"
[sandbox]
mode = "read-only"
sandbox_mode = "read-only"
[sandbox_workspace_write]
network_access = true # This should be ignored.
"#;
let sandbox_read_only_cfg = toml::from_str::<ConfigToml>(sandbox_read_only)
.expect("TOML deserialization should succeed");
assert_eq!(Some(SandboxPolicy::ReadOnly), sandbox_read_only_cfg.sandbox);
let sandbox_mode_override = None;
assert_eq!(
SandboxPolicy::ReadOnly,
sandbox_read_only_cfg.derive_sandbox_policy(sandbox_mode_override)
);
let sandbox_workspace_write = r#"
[sandbox]
mode = "workspace-write"
sandbox_mode = "workspace-write"
[sandbox_workspace_write]
writable_roots = [
"/tmp",
]
@@ -580,12 +610,13 @@ writable_roots = [
let sandbox_workspace_write_cfg = toml::from_str::<ConfigToml>(sandbox_workspace_write)
.expect("TOML deserialization should succeed");
let sandbox_mode_override = None;
assert_eq!(
Some(SandboxPolicy::WorkspaceWrite {
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![PathBuf::from("/tmp")],
network_access: false
}),
sandbox_workspace_write_cfg.sandbox
network_access: false,
},
sandbox_workspace_write_cfg.derive_sandbox_policy(sandbox_mode_override)
);
}