mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Allow codex sandbox to consume MCP sandbox state (#29358)
## Summary - let `codex sandbox` accept the JSON value from `codex/sandbox-state-meta` - require the payload `permissionProfile` instead of falling back to ambient permissions - reuse the existing macOS, Linux, and Windows launch paths, treating external sandbox state conservatively as read-only - let opaque forwarders add runtime read roots and disable direct network access without decoding the payload Builds on #29113, which is now on `main`. ## Tests - `just test -p codex-cli debug_sandbox::tests` - `cargo build -p codex-rmcp-client --bin test_stdio_server` - `just test -p codex-core stdio_mcp_tool_call_includes_sandbox_state_meta` - `just test -p codex-mcp` - `just fmt`
This commit is contained in:
@@ -6,6 +6,7 @@ mod seatbelt;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Stdio;
|
||||
|
||||
use anyhow::Context as _;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
@@ -16,6 +17,8 @@ use codex_core::exec_env::create_env;
|
||||
use codex_core::spawn::CODEX_SANDBOX_ENV_VAR;
|
||||
use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR;
|
||||
use codex_protocol::config_types::SandboxMode;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::SandboxEnforcement;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_sandboxing::landlock::allow_network_for_proxy;
|
||||
use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_permission_profile;
|
||||
@@ -45,6 +48,7 @@ pub async fn run_command_under_seatbelt(
|
||||
loader_overrides: LoaderOverrides,
|
||||
) -> anyhow::Result<()> {
|
||||
let SeatbeltCommand {
|
||||
sandbox_state,
|
||||
permissions_profile,
|
||||
config_profile: _,
|
||||
cwd,
|
||||
@@ -60,6 +64,7 @@ pub async fn run_command_under_seatbelt(
|
||||
);
|
||||
run_command_under_sandbox(
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state,
|
||||
permissions_profile,
|
||||
cwd,
|
||||
managed_requirements_mode,
|
||||
@@ -90,6 +95,7 @@ pub async fn run_command_under_landlock(
|
||||
loader_overrides: LoaderOverrides,
|
||||
) -> anyhow::Result<()> {
|
||||
let LandlockCommand {
|
||||
sandbox_state,
|
||||
permissions_profile,
|
||||
config_profile: _,
|
||||
cwd,
|
||||
@@ -103,6 +109,7 @@ pub async fn run_command_under_landlock(
|
||||
);
|
||||
run_command_under_sandbox(
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state,
|
||||
permissions_profile,
|
||||
cwd,
|
||||
managed_requirements_mode,
|
||||
@@ -124,6 +131,7 @@ pub async fn run_command_under_windows_sandbox(
|
||||
loader_overrides: LoaderOverrides,
|
||||
) -> anyhow::Result<()> {
|
||||
let WindowsCommand {
|
||||
sandbox_state,
|
||||
permissions_profile,
|
||||
config_profile: _,
|
||||
cwd,
|
||||
@@ -137,6 +145,7 @@ pub async fn run_command_under_windows_sandbox(
|
||||
);
|
||||
run_command_under_sandbox(
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state,
|
||||
permissions_profile,
|
||||
cwd,
|
||||
managed_requirements_mode,
|
||||
@@ -161,6 +170,7 @@ enum SandboxType {
|
||||
|
||||
#[derive(Debug)]
|
||||
struct DebugSandboxConfigOptions {
|
||||
sandbox_state: crate::SandboxStateArgs,
|
||||
permissions_profile: Option<String>,
|
||||
cwd: Option<PathBuf>,
|
||||
managed_requirements_mode: ManagedRequirementsMode,
|
||||
@@ -187,7 +197,7 @@ impl ManagedRequirementsMode {
|
||||
}
|
||||
|
||||
async fn run_command_under_sandbox(
|
||||
config_options: DebugSandboxConfigOptions,
|
||||
mut config_options: DebugSandboxConfigOptions,
|
||||
command: Vec<String>,
|
||||
config_overrides: CliConfigOverrides,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
@@ -196,6 +206,34 @@ async fn run_command_under_sandbox(
|
||||
#[cfg_attr(not(target_os = "macos"), allow(unused_variables))]
|
||||
allow_unix_sockets: &[AbsolutePathBuf],
|
||||
) -> anyhow::Result<()> {
|
||||
let sandbox_state = config_options
|
||||
.sandbox_state
|
||||
.sandbox_state_json
|
||||
.as_deref()
|
||||
.map(serde_json::from_str::<codex_mcp::SandboxState>)
|
||||
.transpose()
|
||||
.map_err(|err| anyhow::anyhow!("invalid --sandbox-state-json value: {err}"))?;
|
||||
let sandbox_state_readable_root = config_options
|
||||
.sandbox_state
|
||||
.sandbox_state_readable_root
|
||||
.clone();
|
||||
let sandbox_state_disable_network = config_options.sandbox_state.sandbox_state_disable_network;
|
||||
let codex_linux_sandbox_exe = match sandbox_state.as_ref() {
|
||||
Some(state) => {
|
||||
config_options.cwd = Some(
|
||||
state
|
||||
.sandbox_cwd
|
||||
.to_abs_path()
|
||||
.context("sandbox state cwd is not native to this host")?
|
||||
.to_path_buf(),
|
||||
);
|
||||
state
|
||||
.codex_linux_sandbox_exe
|
||||
.clone()
|
||||
.or(codex_linux_sandbox_exe)
|
||||
}
|
||||
None => codex_linux_sandbox_exe,
|
||||
};
|
||||
let config = load_debug_sandbox_config(
|
||||
config_overrides
|
||||
.parse_overrides()
|
||||
@@ -220,12 +258,75 @@ async fn run_command_under_sandbox(
|
||||
&config.permissions.shell_environment_policy,
|
||||
/*thread_id*/ None,
|
||||
);
|
||||
let mut permission_profile = match sandbox_state.as_ref() {
|
||||
Some(state) => match &state.permission_profile {
|
||||
PermissionProfile::External { .. } => {
|
||||
// `External` only says that the producer relies on an outer sandbox; it does not
|
||||
// include filesystem permissions we can recreate here. The consumer may not share
|
||||
// that sandbox, so use a locally enforceable read-only profile instead of spawning
|
||||
// without a sandbox.
|
||||
PermissionProfile::read_only()
|
||||
}
|
||||
permission_profile => permission_profile.clone(),
|
||||
},
|
||||
None => config.permissions.effective_permission_profile(),
|
||||
};
|
||||
if matches!(permission_profile, PermissionProfile::Disabled) && sandbox_state_disable_network {
|
||||
anyhow::bail!(
|
||||
"--sandbox-state-disable-network cannot be applied to a disabled permission profile"
|
||||
);
|
||||
}
|
||||
if !matches!(permission_profile, PermissionProfile::Disabled)
|
||||
&& (!sandbox_state_readable_root.is_empty() || sandbox_state_disable_network)
|
||||
{
|
||||
let file_system = permission_profile
|
||||
.file_system_sandbox_policy()
|
||||
.with_additional_readable_roots(&cwd, &sandbox_state_readable_root);
|
||||
let network = if sandbox_state_disable_network {
|
||||
NetworkSandboxPolicy::Restricted
|
||||
} else {
|
||||
permission_profile.network_sandbox_policy()
|
||||
};
|
||||
permission_profile = PermissionProfile::from_runtime_permissions(&file_system, network);
|
||||
}
|
||||
let use_legacy_landlock = sandbox_state.as_ref().map_or_else(
|
||||
|| config.features.use_legacy_landlock(),
|
||||
|state| state.use_legacy_landlock,
|
||||
);
|
||||
|
||||
match permission_profile.enforcement() {
|
||||
SandboxEnforcement::Managed => {}
|
||||
SandboxEnforcement::Disabled | SandboxEnforcement::External => {
|
||||
let (program, args) = command
|
||||
.split_first()
|
||||
.context("sandbox command must not be empty")?;
|
||||
let mut child = spawn_debug_sandbox_child(
|
||||
PathBuf::from(program),
|
||||
args.to_vec(),
|
||||
/*arg0*/ None,
|
||||
cwd.to_path_buf(),
|
||||
permission_profile.network_sandbox_policy(),
|
||||
env,
|
||||
|_| {},
|
||||
)
|
||||
.await?;
|
||||
handle_exit_status(child.wait().await?);
|
||||
}
|
||||
}
|
||||
|
||||
// Special-case Windows sandbox: execute and exit the process to emulate inherited stdio.
|
||||
if let SandboxType::Windows = sandbox_type {
|
||||
#[cfg(target_os = "windows")]
|
||||
{
|
||||
run_command_under_windows_session(&config, command, cwd, workspace_roots, env).await;
|
||||
run_command_under_windows_session(
|
||||
&config,
|
||||
&permission_profile,
|
||||
command,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
env,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
{
|
||||
@@ -244,7 +345,7 @@ async fn run_command_under_sandbox(
|
||||
let network_proxy = match config.permissions.network.as_ref() {
|
||||
Some(spec) => Some(
|
||||
spec.start_proxy(
|
||||
config.permissions.permission_profile(),
|
||||
&permission_profile,
|
||||
/*policy_decider*/ None,
|
||||
/*blocked_request_observer*/ None,
|
||||
managed_network_requirements_enabled,
|
||||
@@ -266,7 +367,7 @@ async fn run_command_under_sandbox(
|
||||
None => None,
|
||||
};
|
||||
let runtime_permission_profile = with_managed_mitm_ca_readable_root(
|
||||
config.permissions.effective_permission_profile(),
|
||||
permission_profile,
|
||||
managed_mitm_ca_trust_bundle_path.as_ref(),
|
||||
sandbox_policy_cwd.as_path(),
|
||||
);
|
||||
@@ -308,7 +409,6 @@ async fn run_command_under_sandbox(
|
||||
let codex_linux_sandbox_exe = config
|
||||
.codex_linux_sandbox_exe
|
||||
.expect("codex-linux-sandbox executable not found");
|
||||
let use_legacy_landlock = config.features.use_legacy_landlock();
|
||||
let network_sandbox_policy = runtime_permission_profile.network_sandbox_policy();
|
||||
let args = create_linux_sandbox_command_args_for_permission_profile(
|
||||
command,
|
||||
@@ -364,6 +464,7 @@ async fn run_command_under_sandbox(
|
||||
#[cfg(target_os = "windows")]
|
||||
async fn run_command_under_windows_session(
|
||||
config: &Config,
|
||||
permission_profile: &PermissionProfile,
|
||||
command: Vec<String>,
|
||||
cwd: AbsolutePathBuf,
|
||||
workspace_roots: Vec<AbsolutePathBuf>,
|
||||
@@ -374,10 +475,9 @@ async fn run_command_under_windows_session(
|
||||
use codex_windows_sandbox::WindowsSandboxSessionRequest;
|
||||
use codex_windows_sandbox::spawn_windows_sandbox_session_for_level;
|
||||
|
||||
let permission_profile = config.permissions.effective_permission_profile();
|
||||
let empty_paths: &[AbsolutePathBuf] = &[];
|
||||
let spawned = spawn_windows_sandbox_session_for_level(WindowsSandboxSessionRequest {
|
||||
permission_profile: &permission_profile,
|
||||
permission_profile,
|
||||
workspace_roots: workspace_roots.as_slice(),
|
||||
codex_home: config.codex_home.as_path(),
|
||||
command,
|
||||
@@ -464,6 +564,7 @@ async fn load_debug_sandbox_config_with_codex_home(
|
||||
strict_config: bool,
|
||||
) -> anyhow::Result<Config> {
|
||||
let DebugSandboxConfigOptions {
|
||||
sandbox_state: _,
|
||||
permissions_profile,
|
||||
cwd,
|
||||
managed_requirements_mode,
|
||||
@@ -649,6 +750,7 @@ mod tests {
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state: Default::default(),
|
||||
permissions_profile: None,
|
||||
cwd: None,
|
||||
managed_requirements_mode: ManagedRequirementsMode::Include,
|
||||
@@ -717,6 +819,7 @@ mod tests {
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state: Default::default(),
|
||||
permissions_profile: None,
|
||||
cwd: None,
|
||||
managed_requirements_mode: ManagedRequirementsMode::Include,
|
||||
@@ -774,6 +877,7 @@ mod tests {
|
||||
cli_overrides,
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state: Default::default(),
|
||||
permissions_profile: None,
|
||||
cwd: None,
|
||||
managed_requirements_mode: ManagedRequirementsMode::Include,
|
||||
@@ -832,6 +936,7 @@ mod tests {
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state: Default::default(),
|
||||
permissions_profile: None,
|
||||
cwd: None,
|
||||
managed_requirements_mode: ManagedRequirementsMode::Include,
|
||||
@@ -859,6 +964,7 @@ mod tests {
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state: Default::default(),
|
||||
permissions_profile: Some(":workspace".to_string()),
|
||||
cwd: None,
|
||||
managed_requirements_mode: ManagedRequirementsMode::Ignore,
|
||||
@@ -898,6 +1004,7 @@ mod tests {
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state: Default::default(),
|
||||
permissions_profile: Some("limited-read-test".to_string()),
|
||||
cwd: None,
|
||||
managed_requirements_mode: ManagedRequirementsMode::Ignore,
|
||||
@@ -937,6 +1044,7 @@ mod tests {
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
DebugSandboxConfigOptions {
|
||||
sandbox_state: Default::default(),
|
||||
permissions_profile: Some(":workspace".to_string()),
|
||||
cwd: Some(cwd.path().to_path_buf()),
|
||||
managed_requirements_mode: ManagedRequirementsMode::Ignore,
|
||||
|
||||
+35
-2
@@ -2,6 +2,7 @@ pub(crate) mod debug_sandbox;
|
||||
mod exit_status;
|
||||
pub(crate) mod login;
|
||||
|
||||
use clap::Args;
|
||||
use clap::Parser;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_cli::CliConfigOverrides;
|
||||
@@ -21,10 +22,36 @@ pub use login::run_login_with_device_code;
|
||||
pub use login::run_login_with_device_code_fallback_to_browser;
|
||||
pub use login::run_logout;
|
||||
|
||||
#[derive(Debug, Default, Args)]
|
||||
pub struct SandboxStateArgs {
|
||||
/// JSON value from `codex/sandbox-state-meta` to apply directly.
|
||||
#[arg(
|
||||
long = "sandbox-state-json",
|
||||
value_name = "JSON",
|
||||
conflicts_with_all = ["permissions_profile", "cwd", "include_managed_config"]
|
||||
)]
|
||||
pub sandbox_state_json: Option<String>,
|
||||
|
||||
/// Add a readable root to the supplied sandbox state. Repeat for multiple roots.
|
||||
#[arg(
|
||||
long,
|
||||
requires = "sandbox_state_json",
|
||||
value_parser = parse_absolute_path
|
||||
)]
|
||||
pub sandbox_state_readable_root: Vec<AbsolutePathBuf>,
|
||||
|
||||
/// Disable direct network access in the supplied sandbox state.
|
||||
#[arg(long, requires = "sandbox_state_json", default_value_t = false)]
|
||||
pub sandbox_state_disable_network: bool,
|
||||
}
|
||||
|
||||
// These command structs share common sandbox options, but remain separate
|
||||
// because each host backend has a slightly different option surface.
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct SeatbeltCommand {
|
||||
#[command(flatten)]
|
||||
pub sandbox_state: SandboxStateArgs,
|
||||
|
||||
/// Named permissions profile to apply from the active configuration stack.
|
||||
#[arg(long = "permissions-profile", short = 'P', value_name = "NAME")]
|
||||
pub permissions_profile: Option<String>,
|
||||
@@ -51,7 +78,7 @@ pub struct SeatbeltCommand {
|
||||
pub include_managed_config: bool,
|
||||
|
||||
/// Allow the sandboxed command to bind/connect AF_UNIX sockets rooted at this path. Relative paths are resolved against the current directory. Repeat to allow multiple paths.
|
||||
#[arg(long = "allow-unix-socket", value_parser = parse_allow_unix_socket_path)]
|
||||
#[arg(long = "allow-unix-socket", value_parser = parse_absolute_path)]
|
||||
pub allow_unix_sockets: Vec<AbsolutePathBuf>,
|
||||
|
||||
/// While the command runs, capture macOS sandbox denials via `log stream` and print them after exit
|
||||
@@ -66,13 +93,16 @@ pub struct SeatbeltCommand {
|
||||
pub command: Vec<String>,
|
||||
}
|
||||
|
||||
fn parse_allow_unix_socket_path(raw: &str) -> Result<AbsolutePathBuf, String> {
|
||||
fn parse_absolute_path(raw: &str) -> Result<AbsolutePathBuf, String> {
|
||||
AbsolutePathBuf::relative_to_current_dir(raw)
|
||||
.map_err(|err| format!("invalid path {raw}: {err}"))
|
||||
}
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct LandlockCommand {
|
||||
#[command(flatten)]
|
||||
pub sandbox_state: SandboxStateArgs,
|
||||
|
||||
/// Named permissions profile to apply from the active configuration stack.
|
||||
#[arg(long = "permissions-profile", short = 'P', value_name = "NAME")]
|
||||
pub permissions_profile: Option<String>,
|
||||
@@ -108,6 +138,9 @@ pub struct LandlockCommand {
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct WindowsCommand {
|
||||
#[command(flatten)]
|
||||
pub sandbox_state: SandboxStateArgs,
|
||||
|
||||
/// Named permissions profile to apply from the active configuration stack.
|
||||
#[arg(long = "permissions-profile", short = 'P', value_name = "NAME")]
|
||||
pub permissions_profile: Option<String>,
|
||||
|
||||
@@ -17,11 +17,10 @@ use codex_utils_path_uri::PathUri;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct SandboxState {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
pub permission_profile: PermissionProfile,
|
||||
pub codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
pub sandbox_cwd: PathUri,
|
||||
#[serde(default)]
|
||||
|
||||
@@ -744,7 +744,7 @@ async fn augment_mcp_tool_request_meta_with_sandbox_state(
|
||||
};
|
||||
let permission_profile = turn_context.permission_profile();
|
||||
let sandbox_state = serde_json::to_value(SandboxState {
|
||||
permission_profile: Some(permission_profile),
|
||||
permission_profile,
|
||||
codex_linux_sandbox_exe: turn_context.config.codex_linux_sandbox_exe.clone(),
|
||||
sandbox_cwd,
|
||||
use_legacy_landlock: turn_context.config.features.use_legacy_landlock(),
|
||||
|
||||
@@ -26,6 +26,7 @@ use codex_exec_server::Environment;
|
||||
use codex_exec_server::HttpRequestParams;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_mcp::MCP_SANDBOX_STATE_META_CAPABILITY;
|
||||
use codex_mcp::SandboxState;
|
||||
use codex_models_manager::manager::RefreshStrategy;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
|
||||
@@ -933,13 +934,16 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()>
|
||||
let sandbox_meta = meta
|
||||
.get(MCP_SANDBOX_STATE_META_CAPABILITY)
|
||||
.expect("sandbox state metadata should be present");
|
||||
assert_eq!(sandbox_meta.get("sandboxPolicy"), None);
|
||||
let expected_sandbox_cwd = PathUri::from_abs_path(&fixture.config.cwd).to_string();
|
||||
let sandbox_state: SandboxState = serde_json::from_value(sandbox_meta.clone())?;
|
||||
assert_eq!(
|
||||
sandbox_meta.get("sandboxCwd").and_then(Value::as_str),
|
||||
Some(expected_sandbox_cwd.as_str())
|
||||
sandbox_state,
|
||||
SandboxState {
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
codex_linux_sandbox_exe: fixture.config.codex_linux_sandbox_exe.clone(),
|
||||
sandbox_cwd: PathUri::from_abs_path(&fixture.config.cwd),
|
||||
use_legacy_landlock: false,
|
||||
}
|
||||
);
|
||||
assert_eq!(sandbox_meta.get("useLegacyLandlock"), Some(&json!(false)));
|
||||
|
||||
server.verify().await;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user