mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
fix: sandbox zsh fork unified exec trampoline
This commit is contained in:
@@ -617,7 +617,7 @@ impl EscalationPolicy for CoreShellActionProvider {
|
||||
let decision_driven_by_policy =
|
||||
Self::decision_driven_by_policy(&evaluation.matched_rules, evaluation.decision);
|
||||
let needs_escalation =
|
||||
self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy;
|
||||
self.sandbox_permissions.requests_sandbox_override() || decision_driven_by_policy;
|
||||
|
||||
let decision_source = if decision_driven_by_policy {
|
||||
DecisionSource::PrefixRule
|
||||
@@ -844,7 +844,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
EscalationExecution::Unsandboxed => PreparedExec {
|
||||
command,
|
||||
cwd: workdir.to_path_buf(),
|
||||
env,
|
||||
env: exec_env_for_sandbox_permissions(&env, SandboxPermissions::RequireEscalated),
|
||||
arg0: Some(first_arg.clone()),
|
||||
},
|
||||
EscalationExecution::TurnDefault => {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::CoreShellActionProvider;
|
||||
use super::CoreShellCommandExecutor;
|
||||
use super::InterceptedExecPolicyContext;
|
||||
use super::ParsedShellCommand;
|
||||
use super::commands_for_intercepted_exec_policy;
|
||||
@@ -16,6 +17,9 @@ use codex_execpolicy::PolicyParser;
|
||||
use codex_execpolicy::RuleMatch;
|
||||
use codex_hooks::Hooks;
|
||||
use codex_hooks::HooksConfig;
|
||||
use codex_network_proxy::PROXY_ACTIVE_ENV_KEY;
|
||||
use codex_network_proxy::PROXY_ENV_KEYS;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
@@ -29,13 +33,16 @@ use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::GuardianCommandSource;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::policy_transforms::effective_permission_profile;
|
||||
use codex_shell_escalation::EscalationExecution;
|
||||
use codex_shell_escalation::EscalationPermissions;
|
||||
use codex_shell_escalation::ExecResult;
|
||||
use codex_shell_escalation::ResolvedPermissionProfile;
|
||||
use codex_shell_escalation::ShellCommandExecutor;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
@@ -318,6 +325,100 @@ fn shell_request_escalation_execution_is_explicit() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unsandboxed_intercepted_exec_strips_managed_network_env() -> anyhow::Result<()> {
|
||||
let workdir = test_sandbox_cwd();
|
||||
let executor = CoreShellCommandExecutor {
|
||||
command: Vec::new(),
|
||||
cwd: workdir.clone(),
|
||||
permission_profile: PermissionProfile::workspace_write(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
|
||||
sandbox: SandboxType::None,
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
arg0: None,
|
||||
sandbox_policy_cwd: workdir.clone(),
|
||||
windows_sandbox_workspace_roots: vec![workdir.clone()],
|
||||
codex_linux_sandbox_exe: None,
|
||||
use_legacy_landlock: false,
|
||||
};
|
||||
let mut env = HashMap::new();
|
||||
env.insert(PROXY_ACTIVE_ENV_KEY.to_string(), "1".to_string());
|
||||
for key in PROXY_ENV_KEYS {
|
||||
env.insert((*key).to_string(), format!("proxy-{key}"));
|
||||
}
|
||||
|
||||
let prepared = executor
|
||||
.prepare_escalated_exec(
|
||||
&AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?,
|
||||
&["curl".to_string(), "example.com".to_string()],
|
||||
&workdir,
|
||||
env,
|
||||
EscalationExecution::Unsandboxed,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(!prepared.env.contains_key(PROXY_ACTIVE_ENV_KEY));
|
||||
for key in PROXY_ENV_KEYS {
|
||||
assert!(!prepared.env.contains_key(*key));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn preapproved_additional_permissions_escalate_intercepted_exec() -> anyhow::Result<()> {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let requested_permissions = AdditionalPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
/*read*/ None,
|
||||
Some(vec![
|
||||
AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(),
|
||||
]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let workdir = test_sandbox_cwd();
|
||||
let permission_profile = effective_permission_profile(
|
||||
&PermissionProfile::workspace_write(),
|
||||
Some(&requested_permissions),
|
||||
);
|
||||
let provider = CoreShellActionProvider {
|
||||
policy: Arc::new(RwLock::new(codex_execpolicy::Policy::empty())),
|
||||
session: Arc::new(session),
|
||||
turn: Arc::new(turn_context),
|
||||
call_id: "preapproved-additional-permissions".to_string(),
|
||||
tool_name: GuardianCommandSource::Shell,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: permission_profile.clone(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_policy_cwd: workdir.clone(),
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
approval_sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prompt_permissions: Some(requested_permissions),
|
||||
stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)),
|
||||
};
|
||||
|
||||
let action = codex_shell_escalation::EscalationPolicy::determine_action(
|
||||
&provider,
|
||||
&AbsolutePathBuf::from_absolute_path("/usr/bin/printf")?,
|
||||
&["printf".to_string(), "hello".to_string()],
|
||||
&workdir,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let expected = codex_shell_escalation::EscalationDecision::Escalate(
|
||||
EscalationExecution::Permissions(EscalationPermissions::ResolvedPermissionProfile(
|
||||
ResolvedPermissionProfile { permission_profile },
|
||||
)),
|
||||
);
|
||||
assert_eq!(action, expected);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Result<()> {
|
||||
let (session, mut turn_context) = make_session_and_context().await;
|
||||
|
||||
@@ -45,6 +45,7 @@ use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::SandboxErr;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
use codex_shell_command::powershell::prefix_powershell_script_with_utf8;
|
||||
use codex_tools::UnifiedExecShellMode;
|
||||
@@ -117,6 +118,65 @@ impl<'a> UnifiedExecRuntime<'a> {
|
||||
shell_mode,
|
||||
}
|
||||
}
|
||||
|
||||
fn first_attempt_sandbox_permissions(&self, req: &UnifiedExecRequest) -> SandboxPermissions {
|
||||
if matches!(&self.shell_mode, UnifiedExecShellMode::ZshFork(_))
|
||||
&& req.sandbox_permissions.requires_escalated_permissions()
|
||||
{
|
||||
SandboxPermissions::UseDefault
|
||||
} else {
|
||||
req.sandbox_permissions
|
||||
}
|
||||
}
|
||||
|
||||
fn first_attempt_exec_approval_requirement(
|
||||
&self,
|
||||
req: &UnifiedExecRequest,
|
||||
) -> ExecApprovalRequirement {
|
||||
if matches!(&self.shell_mode, UnifiedExecShellMode::ZshFork(_))
|
||||
&& let ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment,
|
||||
} = &req.exec_approval_requirement
|
||||
{
|
||||
ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: proposed_execpolicy_amendment.clone(),
|
||||
}
|
||||
} else {
|
||||
req.exec_approval_requirement.clone()
|
||||
}
|
||||
}
|
||||
|
||||
fn launch_sandbox_permissions(
|
||||
&self,
|
||||
req: &UnifiedExecRequest,
|
||||
attempt: &SandboxAttempt<'_>,
|
||||
) -> SandboxPermissions {
|
||||
if matches!(&self.shell_mode, UnifiedExecShellMode::ZshFork(_))
|
||||
&& attempt.sandbox != SandboxType::None
|
||||
&& req.sandbox_permissions.requires_escalated_permissions()
|
||||
{
|
||||
SandboxPermissions::UseDefault
|
||||
} else {
|
||||
req.sandbox_permissions
|
||||
}
|
||||
}
|
||||
|
||||
fn launch_additional_permissions(
|
||||
&self,
|
||||
req: &UnifiedExecRequest,
|
||||
attempt: &SandboxAttempt<'_>,
|
||||
) -> Option<AdditionalPermissionProfile> {
|
||||
if matches!(&self.shell_mode, UnifiedExecShellMode::ZshFork(_))
|
||||
&& attempt.sandbox != SandboxType::None
|
||||
&& req.sandbox_permissions.requires_escalated_permissions()
|
||||
{
|
||||
None
|
||||
} else {
|
||||
req.additional_permissions.clone()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Sandboxable for UnifiedExecRuntime<'_> {
|
||||
@@ -202,7 +262,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
&self,
|
||||
req: &UnifiedExecRequest,
|
||||
) -> Option<ExecApprovalRequirement> {
|
||||
Some(req.exec_approval_requirement.clone())
|
||||
Some(self.first_attempt_exec_approval_requirement(req))
|
||||
}
|
||||
|
||||
fn permission_request_payload(
|
||||
@@ -216,7 +276,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
}
|
||||
|
||||
fn sandbox_permissions(&self, req: &UnifiedExecRequest) -> SandboxPermissions {
|
||||
req.sandbox_permissions
|
||||
self.first_attempt_sandbox_permissions(req)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -257,9 +317,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);
|
||||
let launch_sandbox_permissions = self.launch_sandbox_permissions(req, attempt);
|
||||
let managed_network = managed_network_for_sandbox_permissions(
|
||||
req.network.as_ref(),
|
||||
launch_sandbox_permissions,
|
||||
);
|
||||
let mut env = exec_env_for_sandbox_permissions(&req.env, launch_sandbox_permissions);
|
||||
if let Some(network) = managed_network {
|
||||
network.apply_to_env(&mut env);
|
||||
}
|
||||
@@ -301,9 +364,13 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
};
|
||||
|
||||
if let UnifiedExecShellMode::ZshFork(zsh_fork_config) = &self.shell_mode {
|
||||
let command =
|
||||
build_sandbox_command(&command, &req.cwd, &env, req.additional_permissions.clone())
|
||||
.map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?;
|
||||
let command = build_sandbox_command(
|
||||
&command,
|
||||
&req.cwd,
|
||||
&env,
|
||||
self.launch_additional_permissions(req, attempt),
|
||||
)
|
||||
.map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?;
|
||||
let options = unified_exec_options(attempt.network_denial_cancellation_token.clone());
|
||||
let mut exec_env = attempt
|
||||
.env_for(command, options, managed_network)
|
||||
@@ -385,8 +452,15 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_tools::ZshForkConfig;
|
||||
use std::time::Duration;
|
||||
use tempfile::tempdir;
|
||||
|
||||
@@ -448,4 +522,149 @@ mod tests {
|
||||
|
||||
assert_eq!(runtime.sandbox_cwd(&request), Some(&sandbox_cwd));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn zsh_fork_first_attempt_uses_turn_default_sandbox_permissions() {
|
||||
let manager = UnifiedExecProcessManager::default();
|
||||
let request = test_request(
|
||||
SandboxPermissions::RequireEscalated,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
);
|
||||
let direct_runtime = UnifiedExecRuntime::new(&manager, UnifiedExecShellMode::Direct);
|
||||
let zsh_fork_runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode());
|
||||
|
||||
assert_eq!(
|
||||
direct_runtime.sandbox_permissions(&request),
|
||||
SandboxPermissions::RequireEscalated
|
||||
);
|
||||
assert_eq!(
|
||||
zsh_fork_runtime.sandbox_permissions(&request),
|
||||
SandboxPermissions::UseDefault
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn zsh_fork_first_attempt_preserves_additional_permissions_request() {
|
||||
let manager = UnifiedExecProcessManager::default();
|
||||
let request = test_request(
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
);
|
||||
let zsh_fork_runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode());
|
||||
|
||||
assert_eq!(
|
||||
zsh_fork_runtime.sandbox_permissions(&request),
|
||||
SandboxPermissions::WithAdditionalPermissions
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn zsh_fork_sandboxed_launch_preserves_additional_permissions() {
|
||||
let manager = UnifiedExecProcessManager::default();
|
||||
let runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode());
|
||||
let additional_permissions = AdditionalPermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
let mut request = test_request(
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
);
|
||||
request.additional_permissions = Some(additional_permissions.clone());
|
||||
let cwd = request.sandbox_cwd.clone();
|
||||
let workspace_roots = vec![cwd.clone()];
|
||||
let sandbox_manager = SandboxManager::new();
|
||||
let permissions = PermissionProfile::workspace_write();
|
||||
let attempt = SandboxAttempt {
|
||||
sandbox: SandboxType::MacosSeatbelt,
|
||||
permissions: &permissions,
|
||||
enforce_managed_network: true,
|
||||
manager: &sandbox_manager,
|
||||
sandbox_cwd: &cwd,
|
||||
workspace_roots: workspace_roots.as_slice(),
|
||||
codex_linux_sandbox_exe: None,
|
||||
use_legacy_landlock: false,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
network_denial_cancellation_token: None,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
runtime.launch_sandbox_permissions(&request, &attempt),
|
||||
SandboxPermissions::WithAdditionalPermissions
|
||||
);
|
||||
assert_eq!(
|
||||
runtime.launch_additional_permissions(&request, &attempt),
|
||||
Some(additional_permissions)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn zsh_fork_execpolicy_allow_does_not_bypass_trampoline_sandbox() {
|
||||
let manager = UnifiedExecProcessManager::default();
|
||||
let request = test_request(
|
||||
SandboxPermissions::UseDefault,
|
||||
ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: true,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
);
|
||||
let runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode());
|
||||
|
||||
assert_eq!(
|
||||
runtime.exec_approval_requirement(&request),
|
||||
Some(ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
fn test_request(
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
exec_approval_requirement: ExecApprovalRequirement,
|
||||
) -> UnifiedExecRequest {
|
||||
let cwd = AbsolutePathBuf::try_from(std::env::current_dir().unwrap())
|
||||
.expect("current dir is absolute");
|
||||
UnifiedExecRequest {
|
||||
command: vec!["zsh".to_string(), "-c".to_string(), "echo hi".to_string()],
|
||||
shell_type: ShellType::Zsh,
|
||||
hook_command: "echo hi".to_string(),
|
||||
process_id: 1000,
|
||||
cwd: cwd.clone(),
|
||||
sandbox_cwd: cwd,
|
||||
environment: Arc::new(Environment::default_for_tests()),
|
||||
env: HashMap::new(),
|
||||
exec_server_env_config: None,
|
||||
explicit_env_overrides: HashMap::new(),
|
||||
network: None,
|
||||
tty: false,
|
||||
sandbox_permissions,
|
||||
additional_permissions: None,
|
||||
#[cfg(unix)]
|
||||
additional_permissions_preapproved: false,
|
||||
justification: None,
|
||||
exec_approval_requirement,
|
||||
}
|
||||
}
|
||||
|
||||
fn zsh_fork_mode() -> UnifiedExecShellMode {
|
||||
let cwd = std::env::current_dir().expect("read current dir");
|
||||
UnifiedExecShellMode::ZshFork(ZshForkConfig {
|
||||
shell_zsh_path: AbsolutePathBuf::try_from(cwd.join("zsh")).expect("absolute zsh path"),
|
||||
main_execve_wrapper_exe: AbsolutePathBuf::try_from(cwd.join("execve-wrapper"))
|
||||
.expect("absolute wrapper path"),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user