mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
sandboxing: plumb split sandbox policies through runtime (#13439)
## Why `#13434` introduces split `FileSystemSandboxPolicy` and `NetworkSandboxPolicy`, but the runtime still made most execution-time sandbox decisions from the legacy `SandboxPolicy` projection. That projection loses information about combinations like unrestricted filesystem access with restricted network access. In practice, that means the runtime can choose the wrong platform sandbox behavior or set the wrong network-restriction environment for a command even when config has already separated those concerns. This PR carries the split policies through the runtime so sandbox selection, process spawning, and exec handling can consult the policy that actually matters. ## What changed - threaded `FileSystemSandboxPolicy` and `NetworkSandboxPolicy` through `TurnContext`, `ExecRequest`, sandbox attempts, shell escalation state, unified exec, and app-server exec overrides - updated sandbox selection in `core/src/sandboxing/mod.rs` and `core/src/exec.rs` to key off `FileSystemSandboxPolicy.kind` plus `NetworkSandboxPolicy`, rather than inferring behavior only from the legacy `SandboxPolicy` - updated process spawning in `core/src/spawn.rs` and the platform wrappers to use `NetworkSandboxPolicy` when deciding whether to set `CODEX_SANDBOX_NETWORK_DISABLED` - kept additional-permissions handling and legacy `ExternalSandbox` compatibility projections aligned with the split policies, including explicit user-shell execution and Windows restricted-token routing - updated callers across `core`, `app-server`, and `linux-sandbox` to pass the split policies explicitly ## Verification - added regression coverage in `core/tests/suite/user_shell_cmd.rs` to verify `RunUserShellCommand` does not inherit `CODEX_SANDBOX_NETWORK_DISABLED` from the active turn - added coverage in `core/src/exec.rs` for Windows restricted-token sandbox selection when the legacy projection is `ExternalSandbox` - updated Linux sandbox coverage in `linux-sandbox/tests/suite/landlock.rs` to exercise the split-policy exec path - verified the current PR state with `just clippy` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13439). * #13453 * #13452 * #13451 * #13449 * #13448 * #13445 * #13440 * __->__ #13439 --------- Co-authored-by: viyatb-oai <viyatb@openai.com>
This commit is contained in:
co-authored by
viyatb-oai
parent
25fa974166
commit
22ac6b9aaa
@@ -6,13 +6,13 @@ use tokio::process::Child;
|
||||
use tokio::process::Command;
|
||||
use tracing::trace;
|
||||
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
|
||||
/// Experimental environment variable that will be set to some non-empty value
|
||||
/// if both of the following are true:
|
||||
///
|
||||
/// 1. The process was spawned by Codex as part of a shell tool call.
|
||||
/// 2. SandboxPolicy.has_full_network_access() was false for the tool call.
|
||||
/// 2. NetworkSandboxPolicy is restricted for the tool call.
|
||||
///
|
||||
/// We may try to have just one environment variable for all sandboxing
|
||||
/// attributes, so this may change in the future.
|
||||
@@ -33,15 +33,15 @@ pub enum StdioPolicy {
|
||||
/// ensuring the args and environment variables used to create the `Command`
|
||||
/// (and `Child`) honor the configuration.
|
||||
///
|
||||
/// For now, we take `SandboxPolicy` as a parameter to spawn_child() because
|
||||
/// we need to determine whether to set the
|
||||
/// For now, we take `NetworkSandboxPolicy` as a parameter to spawn_child()
|
||||
/// because we need to determine whether to set the
|
||||
/// `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` environment variable.
|
||||
pub(crate) struct SpawnChildRequest<'a> {
|
||||
pub program: PathBuf,
|
||||
pub args: Vec<String>,
|
||||
pub arg0: Option<&'a str>,
|
||||
pub cwd: PathBuf,
|
||||
pub sandbox_policy: &'a SandboxPolicy,
|
||||
pub network_sandbox_policy: NetworkSandboxPolicy,
|
||||
pub network: Option<&'a NetworkProxy>,
|
||||
pub stdio_policy: StdioPolicy,
|
||||
pub env: HashMap<String, String>,
|
||||
@@ -53,14 +53,14 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io
|
||||
args,
|
||||
arg0,
|
||||
cwd,
|
||||
sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
network,
|
||||
stdio_policy,
|
||||
mut env,
|
||||
} = request;
|
||||
|
||||
trace!(
|
||||
"spawn_child_async: {program:?} {args:?} {arg0:?} {cwd:?} {sandbox_policy:?} {stdio_policy:?} {env:?}"
|
||||
"spawn_child_async: {program:?} {args:?} {arg0:?} {cwd:?} {network_sandbox_policy:?} {stdio_policy:?} {env:?}"
|
||||
);
|
||||
|
||||
let mut cmd = Command::new(&program);
|
||||
@@ -74,7 +74,7 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io
|
||||
cmd.env_clear();
|
||||
cmd.envs(env);
|
||||
|
||||
if !sandbox_policy.has_full_network_access() {
|
||||
if !network_sandbox_policy.is_enabled() {
|
||||
cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1");
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user