mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Bypass managed network for escalated exec (#19595)
## Why `sandbox_permissions = "require_escalated"` is treated as an explicit request to approve the command and run it outside the filesystem/platform sandbox. Before this change, shell and unified exec still registered managed network approval context and could inject Codex-managed proxy state into the child process, which meant an approved escalated command could still hit a second network approval path. This PR makes that escalation boundary consistent: once a command is explicitly approved to run outside the sandbox, Codex does not also route that process through the managed network proxy. ## Security impact Command/filesystem sandbox approval now implies network approval for that command. If an untrusted command or script is allowed to run with `require_escalated`, its network calls are unsandboxed: Codex-managed network allowlists and denylists are not respected for that process, so the command can exfiltrate any data it can read. ## What changed - Skip managed network approval specs for `SandboxPermissions::RequireEscalated`. - Pass `network: None` into shell, zsh-fork shell, and unified exec sandbox preparation for explicitly escalated requests. - Strip Codex-managed proxy environment variables when `CODEX_NETWORK_PROXY_ACTIVE` is present, while preserving user proxy env when the Codex marker is absent. - Add regression coverage for the prepared exec request so the old behavior cannot silently reappear. ## Verification - `cargo test -p codex-core explicit_escalation` - `cargo clippy -p codex-core --all-targets -- -D warnings`
This commit is contained in:
committed by
GitHub
Unverified
parent
0c785598b3
commit
9aaa5d9358
@@ -6,6 +6,7 @@ small and focused and reuses the orchestrator for approvals + sandbox + retry.
|
||||
*/
|
||||
use crate::exec_env::CODEX_THREAD_ID_ENV_VAR;
|
||||
use crate::path_utils;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::shell::Shell;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
#[cfg(target_os = "macos")]
|
||||
@@ -43,6 +44,29 @@ pub(crate) fn build_sandbox_command(
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn exec_env_for_sandbox_permissions(
|
||||
env: &HashMap<String, String>,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
) -> HashMap<String, String> {
|
||||
let mut env = env.clone();
|
||||
if sandbox_permissions.requires_escalated_permissions()
|
||||
&& env.contains_key(PROXY_ACTIVE_ENV_KEY)
|
||||
{
|
||||
for key in PROXY_ENV_KEYS {
|
||||
env.remove(*key);
|
||||
}
|
||||
// Only macOS injects a Codex-owned SSH wrapper for the managed SOCKS proxy.
|
||||
#[cfg(target_os = "macos")]
|
||||
if env
|
||||
.get(PROXY_GIT_SSH_COMMAND_ENV_KEY)
|
||||
.is_some_and(|command| command.starts_with(CODEX_PROXY_GIT_SSH_COMMAND_MARKER))
|
||||
{
|
||||
env.remove(PROXY_GIT_SSH_COMMAND_ENV_KEY);
|
||||
}
|
||||
}
|
||||
env
|
||||
}
|
||||
|
||||
/// POSIX-only helper: for commands produced by `Shell::derive_exec_args`
|
||||
/// for Bash/Zsh/sh of the form `[shell_path, "-lc", "<script>"]`, and
|
||||
/// when a snapshot is configured on the session shell, rewrite the argv
|
||||
|
||||
@@ -1,11 +1,29 @@
|
||||
use super::*;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecExpiration;
|
||||
use crate::sandboxing::ExecOptions;
|
||||
use crate::shell::ShellType;
|
||||
use crate::shell_snapshot::ShellSnapshot;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_network_proxy::CODEX_PROXY_GIT_SSH_COMMAND_MARKER;
|
||||
use codex_network_proxy::ConfigReloader;
|
||||
use codex_network_proxy::ConfigState;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_network_proxy::NetworkProxyConfig;
|
||||
use codex_network_proxy::NetworkProxyConstraints;
|
||||
use codex_network_proxy::NetworkProxyState;
|
||||
use codex_network_proxy::PROXY_ACTIVE_ENV_KEY;
|
||||
use codex_network_proxy::PROXY_ENV_KEYS;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_network_proxy::PROXY_GIT_SSH_COMMAND_ENV_KEY;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use core_test_support::PathBufExt;
|
||||
use core_test_support::PathExt;
|
||||
@@ -16,6 +34,23 @@ use std::sync::Arc;
|
||||
use tempfile::tempdir;
|
||||
use tokio::sync::watch;
|
||||
|
||||
struct StaticReloader;
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl ConfigReloader for StaticReloader {
|
||||
fn source_label(&self) -> String {
|
||||
"test config state".to_string()
|
||||
}
|
||||
|
||||
async fn maybe_reload(&self) -> anyhow::Result<Option<ConfigState>> {
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
async fn reload_now(&self) -> anyhow::Result<ConfigState> {
|
||||
Err(anyhow::anyhow!("force reload is not supported in tests"))
|
||||
}
|
||||
}
|
||||
|
||||
fn shell_with_snapshot(
|
||||
shell_type: ShellType,
|
||||
shell_path: &str,
|
||||
@@ -33,6 +68,104 @@ fn shell_with_snapshot(
|
||||
}
|
||||
}
|
||||
|
||||
async fn test_network_proxy() -> anyhow::Result<NetworkProxy> {
|
||||
let state = codex_network_proxy::build_config_state(
|
||||
NetworkProxyConfig::default(),
|
||||
NetworkProxyConstraints::default(),
|
||||
)?;
|
||||
NetworkProxy::builder()
|
||||
.state(Arc::new(NetworkProxyState::with_reloader(
|
||||
state,
|
||||
Arc::new(StaticReloader),
|
||||
)))
|
||||
.managed_by_codex(/*managed_by_codex*/ false)
|
||||
.http_addr("127.0.0.1:43128".parse()?)
|
||||
.socks_addr("127.0.0.1:48081".parse()?)
|
||||
.build()
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow::Result<()> {
|
||||
let proxy = test_network_proxy().await?;
|
||||
let dir = tempdir().expect("create temp dir");
|
||||
let cwd = dir.path().abs();
|
||||
let mut env = HashMap::from([("CUSTOM_ENV".to_string(), "kept".to_string())]);
|
||||
proxy.apply_to_env(&mut env);
|
||||
|
||||
let command = vec!["/bin/echo".to_string(), "ok".to_string()];
|
||||
let command = build_sandbox_command(
|
||||
&command,
|
||||
&cwd,
|
||||
&exec_env_for_sandbox_permissions(&env, SandboxPermissions::RequireEscalated),
|
||||
/*additional_permissions*/ None,
|
||||
)
|
||||
.expect("build sandbox command");
|
||||
let options = ExecOptions {
|
||||
expiration: ExecExpiration::DefaultTimeout,
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
};
|
||||
let sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
let file_system_policy = FileSystemSandboxPolicy::from(&sandbox_policy);
|
||||
let manager = SandboxManager::new();
|
||||
let attempt = SandboxAttempt {
|
||||
sandbox: SandboxType::None,
|
||||
policy: &sandbox_policy,
|
||||
file_system_policy: &file_system_policy,
|
||||
network_policy: NetworkSandboxPolicy::Enabled,
|
||||
enforce_managed_network: false,
|
||||
manager: &manager,
|
||||
sandbox_cwd: &cwd,
|
||||
codex_linux_sandbox_exe: None,
|
||||
use_legacy_landlock: false,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
};
|
||||
|
||||
let exec_request = attempt
|
||||
.env_for(
|
||||
command,
|
||||
options,
|
||||
managed_network_for_sandbox_permissions(
|
||||
Some(&proxy),
|
||||
SandboxPermissions::RequireEscalated,
|
||||
),
|
||||
)
|
||||
.expect("prepare exec request");
|
||||
|
||||
assert_eq!(exec_request.network, None);
|
||||
for key in PROXY_ENV_KEYS {
|
||||
assert_eq!(exec_request.env.get(*key), None, "{key} should be unset");
|
||||
}
|
||||
#[cfg(target_os = "macos")]
|
||||
assert_eq!(exec_request.env.get(PROXY_GIT_SSH_COMMAND_ENV_KEY), None);
|
||||
assert_eq!(
|
||||
exec_request.env.get("CUSTOM_ENV"),
|
||||
Some(&"kept".to_string())
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_escalation_keeps_user_proxy_env_without_codex_marker() {
|
||||
let env = HashMap::from([
|
||||
(
|
||||
"HTTP_PROXY".to_string(),
|
||||
"http://user.proxy:8080".to_string(),
|
||||
),
|
||||
("CUSTOM_ENV".to_string(), "kept".to_string()),
|
||||
]);
|
||||
|
||||
let env = exec_env_for_sandbox_permissions(&env, SandboxPermissions::RequireEscalated);
|
||||
|
||||
assert_eq!(
|
||||
env.get("HTTP_PROXY"),
|
||||
Some(&"http://user.proxy:8080".to_string())
|
||||
);
|
||||
assert_eq!(env.get("CUSTOM_ENV"), Some(&"kept".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn maybe_wrap_shell_lc_with_snapshot_bootstraps_in_user_shell() {
|
||||
let dir = tempdir().expect("create temp dir");
|
||||
|
||||
@@ -20,6 +20,7 @@ use crate::shell::ShellType;
|
||||
use crate::tools::network_approval::NetworkApprovalMode;
|
||||
use crate::tools::network_approval::NetworkApprovalSpec;
|
||||
use crate::tools::runtimes::build_sandbox_command;
|
||||
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
|
||||
use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot;
|
||||
use crate::tools::sandboxing::Approvable;
|
||||
use crate::tools::sandboxing::ApprovalCtx;
|
||||
@@ -31,6 +32,7 @@ use crate::tools::sandboxing::Sandboxable;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
|
||||
use crate::tools::sandboxing::sandbox_override_for_first_attempt;
|
||||
use crate::tools::sandboxing::with_cached_approval;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
@@ -218,9 +220,10 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
req: &ShellRequest,
|
||||
ctx: &ToolCtx,
|
||||
) -> Option<NetworkApprovalSpec> {
|
||||
req.network.as_ref()?;
|
||||
let network =
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions)?;
|
||||
Some(NetworkApprovalSpec {
|
||||
network: req.network.clone(),
|
||||
network: Some(network.clone()),
|
||||
mode: NetworkApprovalMode::Immediate,
|
||||
trigger: GuardianNetworkAccessTrigger {
|
||||
call_id: ctx.call_id.clone(),
|
||||
@@ -243,12 +246,15 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
ctx: &ToolCtx,
|
||||
) -> Result<ExecToolCallOutput, ToolError> {
|
||||
let session_shell = ctx.session.user_shell();
|
||||
let managed_network =
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions);
|
||||
let env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
|
||||
let command = maybe_wrap_shell_lc_with_snapshot(
|
||||
&req.command,
|
||||
session_shell.as_ref(),
|
||||
&req.cwd,
|
||||
&req.explicit_env_overrides,
|
||||
&req.env,
|
||||
&env,
|
||||
);
|
||||
let command = if matches!(session_shell.shell_type, ShellType::PowerShell) {
|
||||
prefix_powershell_script_with_utf8(&command)
|
||||
@@ -267,18 +273,14 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
}
|
||||
}
|
||||
|
||||
let command = build_sandbox_command(
|
||||
&command,
|
||||
&req.cwd,
|
||||
&req.env,
|
||||
req.additional_permissions.clone(),
|
||||
)?;
|
||||
let command =
|
||||
build_sandbox_command(&command, &req.cwd, &env, req.additional_permissions.clone())?;
|
||||
let options = ExecOptions {
|
||||
expiration: req.timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
};
|
||||
let env = attempt
|
||||
.env_for(command, options, req.network.as_ref())
|
||||
.env_for(command, options, managed_network)
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
let out = execute_env(env, Self::stdout_stream(ctx))
|
||||
.await
|
||||
|
||||
@@ -14,10 +14,12 @@ use crate::sandboxing::ExecRequest;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::shell::ShellType;
|
||||
use crate::tools::runtimes::build_sandbox_command;
|
||||
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
|
||||
use crate::tools::sandboxing::PermissionRequestPayload;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
use codex_execpolicy::MatchOptions;
|
||||
@@ -114,18 +116,19 @@ pub(super) async fn try_run_zsh_fork(
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
let command = build_sandbox_command(
|
||||
command,
|
||||
&req.cwd,
|
||||
&req.env,
|
||||
req.additional_permissions.clone(),
|
||||
)?;
|
||||
let env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
|
||||
let command =
|
||||
build_sandbox_command(command, &req.cwd, &env, req.additional_permissions.clone())?;
|
||||
let options = ExecOptions {
|
||||
expiration: req.timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
};
|
||||
let sandbox_exec_request = attempt
|
||||
.env_for(command, options, req.network.as_ref())
|
||||
.env_for(
|
||||
command,
|
||||
options,
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions),
|
||||
)
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
let crate::sandboxing::ExecRequest {
|
||||
command,
|
||||
|
||||
@@ -17,6 +17,7 @@ use crate::shell::ShellType;
|
||||
use crate::tools::network_approval::NetworkApprovalMode;
|
||||
use crate::tools::network_approval::NetworkApprovalSpec;
|
||||
use crate::tools::runtimes::build_sandbox_command;
|
||||
use crate::tools::runtimes::exec_env_for_sandbox_permissions;
|
||||
use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot;
|
||||
use crate::tools::runtimes::shell::zsh_fork_backend;
|
||||
use crate::tools::sandboxing::Approvable;
|
||||
@@ -29,6 +30,7 @@ use crate::tools::sandboxing::Sandboxable;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use crate::tools::sandboxing::managed_network_for_sandbox_permissions;
|
||||
use crate::tools::sandboxing::sandbox_override_for_first_attempt;
|
||||
use crate::tools::sandboxing::with_cached_approval;
|
||||
use crate::unified_exec::NoopSpawnLifecycle;
|
||||
@@ -203,9 +205,10 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
req: &UnifiedExecRequest,
|
||||
ctx: &ToolCtx,
|
||||
) -> Option<NetworkApprovalSpec> {
|
||||
req.network.as_ref()?;
|
||||
let network =
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions)?;
|
||||
Some(NetworkApprovalSpec {
|
||||
network: req.network.clone(),
|
||||
network: Some(network.clone()),
|
||||
mode: NetworkApprovalMode::Deferred,
|
||||
trigger: GuardianNetworkAccessTrigger {
|
||||
call_id: ctx.call_id.clone(),
|
||||
@@ -229,6 +232,12 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
) -> Result<UnifiedExecProcess, ToolError> {
|
||||
let base_command = &req.command;
|
||||
let session_shell = ctx.session.user_shell();
|
||||
let managed_network =
|
||||
managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions);
|
||||
let mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions);
|
||||
if let Some(network) = managed_network {
|
||||
network.apply_to_env(&mut env);
|
||||
}
|
||||
let environment_is_remote = ctx
|
||||
.turn
|
||||
.environment
|
||||
@@ -242,7 +251,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
session_shell.as_ref(),
|
||||
&req.cwd,
|
||||
&req.explicit_env_overrides,
|
||||
&req.env,
|
||||
&env,
|
||||
)
|
||||
};
|
||||
let command = if matches!(session_shell.shell_type, ShellType::PowerShell) {
|
||||
@@ -251,10 +260,6 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
command
|
||||
};
|
||||
|
||||
let mut env = req.env.clone();
|
||||
if let Some(network) = req.network.as_ref() {
|
||||
network.apply_to_env(&mut env);
|
||||
}
|
||||
if let UnifiedExecShellMode::ZshFork(zsh_fork_config) = &self.shell_mode {
|
||||
let command =
|
||||
build_sandbox_command(&command, &req.cwd, &env, req.additional_permissions.clone())
|
||||
@@ -264,7 +269,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
};
|
||||
let mut exec_env = attempt
|
||||
.env_for(command, options, req.network.as_ref())
|
||||
.env_for(command, options, managed_network)
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
exec_env.exec_server_env_config = req.exec_server_env_config.clone();
|
||||
match zsh_fork_backend::maybe_prepare_unified_exec(
|
||||
@@ -322,7 +327,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
};
|
||||
let mut exec_env = attempt
|
||||
.env_for(command, options, req.network.as_ref())
|
||||
.env_for(command, options, managed_network)
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
exec_env.exec_server_env_config = req.exec_server_env_config.clone();
|
||||
let Some(environment) = ctx.turn.environment.as_ref() else {
|
||||
|
||||
@@ -265,6 +265,17 @@ pub(crate) fn sandbox_override_for_first_attempt(
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn managed_network_for_sandbox_permissions(
|
||||
network: Option<&NetworkProxy>,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
) -> Option<&NetworkProxy> {
|
||||
if sandbox_permissions.requires_escalated_permissions() {
|
||||
None
|
||||
} else {
|
||||
network
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) trait Approvable<Req> {
|
||||
type ApprovalKey: Hash + Eq + Clone + Debug + Serialize;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user