mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Report remote sandbox denials semantically (#29424)
## Why #29113 moved remote sandbox setup and enforcement to the exec server. That gives the executor ownership of the platform-specific work: a Linux executor chooses and runs a Linux sandbox even when the Codex orchestrator is running on macOS or Windows. It also means the orchestrator no longer knows which concrete sandbox the executor selected. When that sandbox blocks a remote command, the orchestrator currently sees only a failed process and can treat the denial as an ordinary command failure. The existing sandbox approval and retry path is then skipped. This PR lets the executor report one portable fact: > This command probably failed because the executor sandbox blocked it. The executor keeps its concrete sandbox type private. The protocol sends only the semantic result. ## Example Suppose a local macOS Codex session asks a Linux devbox to write outside the allowed workspace. Before this PR: ```text Linux sandbox blocks the write -> remote process exits with "Permission denied" -> local orchestrator sees an ordinary command failure -> the normal sandbox approval and retry path can be skipped ``` With this PR: ```text Linux sandbox blocks the write -> executor reports sandboxDenied: true -> unified exec returns UnifiedExecError::SandboxDenied -> the existing approval prompt is shown -> an approved retry runs through the existing unsandboxed retry path ``` ## What changes ### The executor remembers its selected sandbox The prepared remote process now retains the executor-selected `SandboxType`. This value never crosses the executor boundary. Commands started without a sandbox retain `SandboxType::None` and are never reported as sandbox denials. ### The executor uses the existing denial heuristic The existing local denial heuristic moves from `codex-core` into the shared `codex-sandboxing` crate. When a sandboxed remote process exits, the executor: 1. waits the same short output grace period used by local unified exec; 2. reads the output currently available in the existing retained output buffer; 3. runs the existing heuristic using the exit code and common denial messages; 4. stores the yes/no result before publishing the process exit. This deliberately matches the old local unified-exec behavior. It does not add a new streaming classifier, another output buffer, or stronger output-retention guarantees. ### The protocol reports a portable boolean `process/read` gains `sandboxDenied`: ```json { "exited": true, "exitCode": 1, "closed": false, "sandboxDenied": true } ``` The field defaults to `false` when an older executor omits it. The response does not expose the executor sandbox implementation or executor-native paths. ### Unified exec uses the existing error path The exec-server client carries `sandboxDenied` into the unified process state. If it is true, unified exec returns the existing `SandboxDenied` error instead of trying to classify remote output using an orchestrator-side sandbox type. Remote process exit remains visible as soon as the process exits. This PR does not wait for stdout or stderr to close and does not change the existing process lifecycle. ## Scope This PR is intentionally limited to matching the existing local unified-exec behavior for the initial command execution path. It does not add: - incremental denial tracking across the full output stream; - new denial handling for commands completed later through `write_stdin`; - new guarantees for preserving the semantic flag during the narrow reconnect-recovery race. Those can be considered separately if the same behavior is added for local execution. ## Test coverage One remote end-to-end integration test covers the complete intended flow: ```text remote read-only sandbox -> denied write -> executor reports the denial -> Codex requests approval -> user approves -> retry succeeds on the remote executor ``` Existing lifecycle coverage continues to verify that remote process exit is reported before late output streams close.
This commit is contained in:
@@ -42,6 +42,7 @@ use codex_sandboxing::SandboxTransformRequest;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
use codex_sandboxing::WindowsSandboxFilesystemOverrides;
|
||||
pub(crate) use codex_sandboxing::is_likely_sandbox_denied;
|
||||
#[cfg(test)]
|
||||
use codex_sandboxing::permission_profile_supports_windows_restricted_token_sandbox;
|
||||
use codex_sandboxing::resolve_windows_elevated_filesystem_overrides;
|
||||
@@ -819,68 +820,6 @@ fn finalize_exec_result(
|
||||
}
|
||||
}
|
||||
|
||||
/// We don't have a fully deterministic way to tell if our command failed
|
||||
/// because of the sandbox - a command in the user's zshrc file might hit an
|
||||
/// error, but the command itself might fail or succeed for other reasons.
|
||||
/// For now, we conservatively check for well known command failure exit codes and
|
||||
/// also look for common sandbox denial keywords in the command output.
|
||||
pub(crate) fn is_likely_sandbox_denied(
|
||||
sandbox_type: SandboxType,
|
||||
exec_output: &ExecToolCallOutput,
|
||||
) -> bool {
|
||||
if sandbox_type == SandboxType::None || exec_output.exit_code == 0 {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Quick rejects: well-known non-sandbox shell exit codes
|
||||
// 2: misuse of shell builtins
|
||||
// 126: permission denied
|
||||
// 127: command not found
|
||||
const SANDBOX_DENIED_KEYWORDS: [&str; 7] = [
|
||||
"operation not permitted",
|
||||
"permission denied",
|
||||
"read-only file system",
|
||||
"seccomp",
|
||||
"sandbox",
|
||||
"landlock",
|
||||
"failed to write file",
|
||||
];
|
||||
|
||||
let has_sandbox_keyword = [
|
||||
&exec_output.stderr.text,
|
||||
&exec_output.stdout.text,
|
||||
&exec_output.aggregated_output.text,
|
||||
]
|
||||
.into_iter()
|
||||
.any(|section| {
|
||||
let lower = section.to_lowercase();
|
||||
SANDBOX_DENIED_KEYWORDS
|
||||
.iter()
|
||||
.any(|needle| lower.contains(needle))
|
||||
});
|
||||
|
||||
if has_sandbox_keyword {
|
||||
return true;
|
||||
}
|
||||
|
||||
const QUICK_REJECT_EXIT_CODES: [i32; 3] = [2, 126, 127];
|
||||
if QUICK_REJECT_EXIT_CODES.contains(&exec_output.exit_code) {
|
||||
return false;
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
{
|
||||
const SIGSYS_CODE: i32 = libc::SIGSYS;
|
||||
if sandbox_type == SandboxType::LinuxSeccomp
|
||||
&& exec_output.exit_code == EXIT_CODE_SIGNAL_BASE + SIGSYS_CODE
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct RawExecToolCallOutput {
|
||||
pub exit_status: ExitStatus,
|
||||
|
||||
@@ -228,6 +228,7 @@ impl BlockingTerminateExecProcess {
|
||||
exit_code: None,
|
||||
closed: false,
|
||||
failure: None,
|
||||
sandbox_denied: false,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -286,17 +287,14 @@ async fn blocking_terminate_unified_process(
|
||||
) -> anyhow::Result<Arc<UnifiedExecProcess>> {
|
||||
let (wake_tx, _wake_rx) = watch::channel(0);
|
||||
Ok(Arc::new(
|
||||
UnifiedExecProcess::from_exec_server_started(
|
||||
StartedExecProcess {
|
||||
process: Arc::new(BlockingTerminateExecProcess {
|
||||
process_id: process_id.to_string().into(),
|
||||
terminate_started,
|
||||
allow_terminate,
|
||||
wake_tx,
|
||||
}),
|
||||
},
|
||||
SandboxType::None,
|
||||
)
|
||||
UnifiedExecProcess::from_exec_server_started(StartedExecProcess {
|
||||
process: Arc::new(BlockingTerminateExecProcess {
|
||||
process_id: process_id.to_string().into(),
|
||||
terminate_started,
|
||||
allow_terminate,
|
||||
wake_tx,
|
||||
}),
|
||||
})
|
||||
.await?,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -285,8 +285,9 @@ impl UnifiedExecProcess {
|
||||
&self,
|
||||
text: &str,
|
||||
) -> Result<(), UnifiedExecError> {
|
||||
let executor_reported_denial = self.state_rx.borrow().sandbox_denied;
|
||||
let sandbox_type = self.sandbox_type();
|
||||
if sandbox_type == SandboxType::None || !self.has_exited() {
|
||||
if !self.has_exited() || (!executor_reported_denial && sandbox_type == SandboxType::None) {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -297,7 +298,7 @@ impl UnifiedExecProcess {
|
||||
aggregated_output: StreamOutput::new(text.to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
if is_likely_sandbox_denied(sandbox_type, &exec_output) {
|
||||
if executor_reported_denial || is_likely_sandbox_denied(sandbox_type, &exec_output) {
|
||||
let snippet = formatted_truncate_text(
|
||||
text,
|
||||
TruncationPolicy::Tokens(UNIFIED_EXEC_OUTPUT_MAX_TOKENS),
|
||||
@@ -374,10 +375,13 @@ impl UnifiedExecProcess {
|
||||
|
||||
pub(super) async fn from_exec_server_started(
|
||||
started: StartedExecProcess,
|
||||
sandbox_type: SandboxType,
|
||||
) -> Result<Self, UnifiedExecError> {
|
||||
let process_handle = ProcessHandle::ExecServer(Arc::clone(&started.process));
|
||||
let mut managed = Self::new(process_handle, sandbox_type, /*spawn_lifecycle*/ None);
|
||||
let mut managed = Self::new(
|
||||
process_handle,
|
||||
SandboxType::None,
|
||||
/*spawn_lifecycle*/ None,
|
||||
);
|
||||
let output_handles = managed.output_handles();
|
||||
managed.output_task = Some(Self::spawn_exec_server_output_task(
|
||||
started,
|
||||
@@ -437,6 +441,7 @@ impl UnifiedExecProcess {
|
||||
exit_code,
|
||||
closed,
|
||||
failure,
|
||||
sandbox_denied,
|
||||
} = response;
|
||||
|
||||
for chunk in chunks {
|
||||
@@ -457,6 +462,12 @@ impl UnifiedExecProcess {
|
||||
break;
|
||||
}
|
||||
|
||||
if sandbox_denied {
|
||||
let mut state = state_tx.borrow().clone();
|
||||
state.sandbox_denied = true;
|
||||
let _ = state_tx.send_replace(state);
|
||||
}
|
||||
|
||||
if exited {
|
||||
let state = state_tx.borrow().clone();
|
||||
let _ = state_tx.send_replace(state.exited(exit_code));
|
||||
|
||||
@@ -10,6 +10,7 @@ use tokio::sync::watch;
|
||||
use tokio::time::Duration;
|
||||
use tokio::time::Instant;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::codex_thread::BackgroundTerminalInfo;
|
||||
use crate::exec_env::CODEX_THREAD_ID_ENV_VAR;
|
||||
@@ -157,8 +158,14 @@ fn exec_server_params_for_request(
|
||||
tty: bool,
|
||||
) -> codex_exec_server::ExecParams {
|
||||
let (env_policy, env) = exec_server_env_for_request(request);
|
||||
// Sandbox retries reuse the unified-exec ID but start a distinct executor process.
|
||||
let exec_server_process_id = if request.exec_server_sandbox.is_some() {
|
||||
format!("{process_id}-{}", Uuid::new_v4())
|
||||
} else {
|
||||
process_id.to_string()
|
||||
};
|
||||
codex_exec_server::ExecParams {
|
||||
process_id: exec_server_process_id(process_id).into(),
|
||||
process_id: exec_server_process_id.into(),
|
||||
argv: request.command.clone(),
|
||||
cwd: request.cwd.clone(),
|
||||
env_policy,
|
||||
@@ -198,10 +205,6 @@ impl Drop for InitialExecCommandGuard {
|
||||
}
|
||||
}
|
||||
|
||||
fn exec_server_process_id(process_id: i32) -> String {
|
||||
process_id.to_string()
|
||||
}
|
||||
|
||||
async fn unregister_network_approval_for_entry(entry: &ProcessEntry) {
|
||||
if let Some(network_approval) = entry.network_approval.as_ref()
|
||||
&& let Some(session) = entry.session.upgrade()
|
||||
@@ -1041,7 +1044,7 @@ impl UnifiedExecProcessManager {
|
||||
.await
|
||||
.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;
|
||||
spawn_lifecycle.after_spawn();
|
||||
return UnifiedExecProcess::from_exec_server_started(started, request.sandbox).await;
|
||||
return UnifiedExecProcess::from_exec_server_started(started).await;
|
||||
}
|
||||
|
||||
// TODO(anp): Keep PathUri through the local PTY/process launch boundary.
|
||||
|
||||
@@ -76,7 +76,7 @@ fn exec_server_params_use_path_uri_and_env_policy_overlay_contract() {
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy::unrestricted();
|
||||
let network_sandbox_policy = codex_protocol::permissions::NetworkSandboxPolicy::Restricted;
|
||||
let permission_profile = codex_protocol::models::PermissionProfile::Disabled;
|
||||
let request = ExecRequest {
|
||||
let mut request = ExecRequest {
|
||||
command: vec!["bash".to_string(), "-lc".to_string(), "true".to_string()],
|
||||
cwd: cwd.clone().into(),
|
||||
env: HashMap::from([
|
||||
@@ -106,7 +106,7 @@ fn exec_server_params_use_path_uri_and_env_policy_overlay_contract() {
|
||||
windows_sandbox_workspace_roots: vec![cwd],
|
||||
windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
permission_profile,
|
||||
permission_profile: permission_profile.clone(),
|
||||
file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
windows_sandbox_filesystem_overrides: None,
|
||||
@@ -128,11 +128,16 @@ fn exec_server_params_use_path_uri_and_env_policy_overlay_contract() {
|
||||
("CODEX_THREAD_ID".to_string(), "thread-1".to_string()),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_server_process_id_matches_unified_exec_process_id() {
|
||||
assert_eq!(exec_server_process_id(/*process_id*/ 4321), "4321");
|
||||
request.exec_server_sandbox = Some(
|
||||
codex_exec_server::FileSystemSandboxContext::from_permission_profile(permission_profile),
|
||||
);
|
||||
let first =
|
||||
exec_server_params_for_request(/*process_id*/ 123, &request, /*tty*/ true);
|
||||
let second =
|
||||
exec_server_params_for_request(/*process_id*/ 123, &request, /*tty*/ true);
|
||||
assert!(first.process_id.as_str().starts_with("123-"));
|
||||
assert!(second.process_id.as_str().starts_with("123-"));
|
||||
assert_ne!(first.process_id, second.process_id);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
|
||||
@@ -3,6 +3,7 @@ pub(crate) struct ProcessState {
|
||||
pub(crate) has_exited: bool,
|
||||
pub(crate) exit_code: Option<i32>,
|
||||
pub(crate) failure_message: Option<String>,
|
||||
pub(crate) sandbox_denied: bool,
|
||||
}
|
||||
|
||||
impl ProcessState {
|
||||
@@ -11,6 +12,7 @@ impl ProcessState {
|
||||
has_exited: true,
|
||||
exit_code,
|
||||
failure_message: self.failure_message.clone(),
|
||||
sandbox_denied: self.sandbox_denied,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,6 +21,7 @@ impl ProcessState {
|
||||
has_exited: true,
|
||||
exit_code: self.exit_code,
|
||||
failure_message: Some(message),
|
||||
sandbox_denied: self.sandbox_denied,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,7 +10,6 @@ use codex_exec_server::ReadResponse;
|
||||
use codex_exec_server::StartedExecProcess;
|
||||
use codex_exec_server::WriteResponse;
|
||||
use codex_exec_server::WriteStatus;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::VecDeque;
|
||||
use std::sync::Arc;
|
||||
@@ -40,6 +39,7 @@ impl MockExecProcess {
|
||||
exit_code: None,
|
||||
closed: false,
|
||||
failure: None,
|
||||
sandbox_denied: false,
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -103,7 +103,7 @@ async fn remote_process(
|
||||
}),
|
||||
};
|
||||
|
||||
UnifiedExecProcess::from_exec_server_started(started, SandboxType::None)
|
||||
UnifiedExecProcess::from_exec_server_started(started)
|
||||
.await
|
||||
.expect("remote process should start")
|
||||
}
|
||||
@@ -190,6 +190,7 @@ async fn remote_process_waits_for_early_exit_event() {
|
||||
exit_code: Some(17),
|
||||
closed: true,
|
||||
failure: None,
|
||||
sandbox_denied: false,
|
||||
}])),
|
||||
terminate_error: None,
|
||||
wake_tx: wake_tx.clone(),
|
||||
@@ -201,7 +202,7 @@ async fn remote_process_waits_for_early_exit_event() {
|
||||
let _ = wake_tx.send(1);
|
||||
});
|
||||
|
||||
let process = UnifiedExecProcess::from_exec_server_started(started, SandboxType::None)
|
||||
let process = UnifiedExecProcess::from_exec_server_started(started)
|
||||
.await
|
||||
.expect("remote process should observe early exit");
|
||||
|
||||
|
||||
@@ -87,6 +87,7 @@ async fn submit_turn_with_approval_and_environments(
|
||||
test: &TestCodex,
|
||||
prompt: &str,
|
||||
environments: Vec<TurnEnvironmentSelection>,
|
||||
approval_policy: AskForApproval,
|
||||
) -> Result<()> {
|
||||
let turn_environment_selections = codex_protocol::protocol::TurnEnvironmentSelections::new(
|
||||
test.config.cwd.clone(),
|
||||
@@ -103,7 +104,7 @@ async fn submit_turn_with_approval_and_environments(
|
||||
additional_context: Default::default(),
|
||||
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
|
||||
environments: Some(turn_environment_selections),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approval_policy: Some(approval_policy),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
sandbox_policy: Some(SandboxPolicy::new_read_only_policy()),
|
||||
collaboration_mode: Some(codex_protocol::config_types::CollaborationMode {
|
||||
@@ -312,6 +313,151 @@ async fn explicit_remote_shell_runs_in_remote_cwd() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_sandbox_denial_requests_approval_and_retries() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_wine_exec!(Ok(()), "requires the Docker-backed POSIX executor");
|
||||
let Some(_remote_env) = get_remote_test_env() else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
const CALL_ID: &str = "remote-sandbox-denial";
|
||||
const CONTENTS: &str = "remote sandbox retry succeeded";
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let test = unified_exec_test(&server).await?;
|
||||
let nonce = SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis();
|
||||
let remote_cwd = PathBuf::from(format!("/tmp/codex-remote-denial-cwd-{nonce}")).abs();
|
||||
let target_path = PathBuf::from(format!("/tmp/codex-remote-denial-target-{nonce}")).abs();
|
||||
let remote_cwd_uri = PathUri::from_path(&remote_cwd)?;
|
||||
let target_uri = PathUri::from_path(&target_path)?;
|
||||
test.fs()
|
||||
.create_directory(
|
||||
&remote_cwd_uri,
|
||||
CreateDirectoryOptions { recursive: true },
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await?;
|
||||
test.fs()
|
||||
.remove(
|
||||
&target_uri,
|
||||
RemoveOptions {
|
||||
recursive: false,
|
||||
force: true,
|
||||
},
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let command = format!("printf {CONTENTS:?} > {target_path:?} && cat {target_path:?}");
|
||||
let response_mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-remote-denial-1"),
|
||||
ev_function_call(
|
||||
CALL_ID,
|
||||
"exec_command",
|
||||
&json!({
|
||||
"shell": "/bin/sh",
|
||||
"cmd": command,
|
||||
"login": false,
|
||||
"yield_time_ms": 5_000,
|
||||
"environment_id": REMOTE_ENVIRONMENT_ID,
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
ev_completed("resp-remote-denial-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-remote-denial-2"),
|
||||
ev_assistant_message("msg-remote-denial", "done"),
|
||||
ev_completed("resp-remote-denial-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn_with_approval_and_environments(
|
||||
&test,
|
||||
"retry a sandbox-denied command in the remote environment",
|
||||
vec![TurnEnvironmentSelection {
|
||||
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
||||
cwd: PathUri::from_abs_path(&remote_cwd),
|
||||
}],
|
||||
AskForApproval::OnFailure,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let event = wait_for_event(&test.codex, |event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
})
|
||||
.await;
|
||||
let EventMsg::ExecApprovalRequest(approval) = event else {
|
||||
panic!("expected remote sandbox approval before completion: {event:?}");
|
||||
};
|
||||
assert_eq!(approval.call_id, CALL_ID);
|
||||
assert_eq!(
|
||||
approval.environment_id.as_deref(),
|
||||
Some(REMOTE_ENVIRONMENT_ID)
|
||||
);
|
||||
assert_eq!(
|
||||
approval.reason.as_deref(),
|
||||
Some("command failed; retry without sandbox?")
|
||||
);
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::Approved,
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
response_mock
|
||||
.function_call_output_text(CALL_ID)
|
||||
.is_some_and(|output| output.contains(CONTENTS)),
|
||||
"approved retry should return the remote command output"
|
||||
);
|
||||
assert_eq!(
|
||||
test.fs()
|
||||
.read_file_text(&target_uri, /*sandbox*/ None)
|
||||
.await?,
|
||||
CONTENTS
|
||||
);
|
||||
|
||||
test.fs()
|
||||
.remove(
|
||||
&target_uri,
|
||||
RemoveOptions {
|
||||
recursive: false,
|
||||
force: true,
|
||||
},
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await?;
|
||||
test.fs()
|
||||
.remove(
|
||||
&remote_cwd_uri,
|
||||
RemoveOptions {
|
||||
recursive: true,
|
||||
force: true,
|
||||
},
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn deferred_executor_does_not_duplicate_initial_environment_context() -> Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
@@ -834,6 +980,7 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result
|
||||
cwd: PathUri::from_abs_path(&remote_cwd),
|
||||
},
|
||||
],
|
||||
AskForApproval::OnRequest,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1115,6 +1262,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> {
|
||||
&test,
|
||||
"apply patch in local environment",
|
||||
environments.clone(),
|
||||
AskForApproval::OnRequest,
|
||||
)
|
||||
.await?;
|
||||
let approval = expect_patch_approval(&test, "call-local").await;
|
||||
@@ -1134,6 +1282,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> {
|
||||
&test,
|
||||
"apply patch in remote environment",
|
||||
environments.clone(),
|
||||
AskForApproval::OnRequest,
|
||||
)
|
||||
.await?;
|
||||
let approval = expect_patch_approval(&test, "call-remote").await;
|
||||
@@ -1158,6 +1307,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> {
|
||||
&test,
|
||||
"apply patch again in remote environment",
|
||||
environments,
|
||||
AskForApproval::OnRequest,
|
||||
)
|
||||
.await?;
|
||||
wait_for_completion_without_patch_approval(&test).await;
|
||||
|
||||
@@ -926,6 +926,7 @@ impl SessionState {
|
||||
exit_code: None,
|
||||
closed: true,
|
||||
failure: Some(message),
|
||||
sandbox_denied: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1869,6 +1870,7 @@ mod tests {
|
||||
exit_code: None,
|
||||
closed: false,
|
||||
failure: None,
|
||||
sandbox_denied: false,
|
||||
})
|
||||
.expect("read response should serialize"),
|
||||
}),
|
||||
|
||||
@@ -58,6 +58,7 @@ impl SessionState {
|
||||
exit_code,
|
||||
closed,
|
||||
failure,
|
||||
sandbox_denied: _,
|
||||
} = response;
|
||||
if let Some(message) = failure {
|
||||
return Err(ExecServerError::Protocol(format!(
|
||||
|
||||
@@ -10,7 +10,11 @@ use std::time::Duration;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_protocol::config_types::EnvironmentVariablePattern;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicy;
|
||||
use codex_protocol::exec_output::ExecToolCallOutput;
|
||||
use codex_protocol::exec_output::StreamOutput;
|
||||
use codex_protocol::shell_environment;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::is_likely_sandbox_denied;
|
||||
use codex_utils_pty::ExecCommandSession;
|
||||
use codex_utils_pty::ProcessSignal as PtyProcessSignal;
|
||||
use codex_utils_pty::TerminalSize;
|
||||
@@ -87,6 +91,8 @@ struct RunningProcess {
|
||||
output_notify: Arc<Notify>,
|
||||
open_streams: usize,
|
||||
closed: bool,
|
||||
sandbox: SandboxType,
|
||||
sandbox_denied: bool,
|
||||
}
|
||||
|
||||
/// Bounded cache of stdin write ids that have already been accepted for one process.
|
||||
@@ -313,6 +319,8 @@ impl LocalProcess {
|
||||
output_notify: Arc::clone(&output_notify),
|
||||
open_streams: 2,
|
||||
closed: false,
|
||||
sandbox: prepared.sandbox,
|
||||
sandbox_denied: false,
|
||||
})),
|
||||
);
|
||||
}
|
||||
@@ -407,6 +415,7 @@ impl LocalProcess {
|
||||
exit_code: process.exit_code,
|
||||
closed: process.closed,
|
||||
failure: None,
|
||||
sandbox_denied: process.sandbox_denied,
|
||||
},
|
||||
Arc::clone(&process.output_notify),
|
||||
)
|
||||
@@ -797,12 +806,46 @@ async fn watch_exit(
|
||||
output_notify: Arc<Notify>,
|
||||
) {
|
||||
let exit_code = exit_rx.await.unwrap_or(-1);
|
||||
let sandboxed = {
|
||||
let processes = inner.processes.lock().await;
|
||||
matches!(
|
||||
processes.get(&process_id),
|
||||
Some(ProcessEntry::Running(process)) if process.sandbox != SandboxType::None
|
||||
)
|
||||
};
|
||||
if sandboxed {
|
||||
let _ = tokio::time::timeout(Duration::from_millis(20), output_notify.notified()).await;
|
||||
}
|
||||
let notification = {
|
||||
let mut processes = inner.processes.lock().await;
|
||||
if let Some(ProcessEntry::Running(process)) = processes.get_mut(&process_id) {
|
||||
let seq = process.next_seq;
|
||||
process.next_seq += 1;
|
||||
process.exit_code = Some(exit_code);
|
||||
if process.sandbox != SandboxType::None {
|
||||
let mut stdout = Vec::new();
|
||||
let mut stderr = Vec::new();
|
||||
let mut aggregated = Vec::new();
|
||||
for chunk in &process.output {
|
||||
match chunk.stream {
|
||||
ExecOutputStream::Stdout | ExecOutputStream::Pty => {
|
||||
stdout.extend_from_slice(&chunk.chunk);
|
||||
}
|
||||
ExecOutputStream::Stderr => stderr.extend_from_slice(&chunk.chunk),
|
||||
}
|
||||
aggregated.extend_from_slice(&chunk.chunk);
|
||||
}
|
||||
let exec_output = ExecToolCallOutput {
|
||||
exit_code,
|
||||
stdout: StreamOutput::new(String::from_utf8_lossy(&stdout).into_owned()),
|
||||
stderr: StreamOutput::new(String::from_utf8_lossy(&stderr).into_owned()),
|
||||
aggregated_output: StreamOutput::new(
|
||||
String::from_utf8_lossy(&aggregated).into_owned(),
|
||||
),
|
||||
..Default::default()
|
||||
};
|
||||
process.sandbox_denied = is_likely_sandbox_denied(process.sandbox, &exec_output);
|
||||
}
|
||||
let _ = process.wake_tx.send(seq);
|
||||
process
|
||||
.events
|
||||
@@ -1000,6 +1043,7 @@ mod tests {
|
||||
exit_code: Some(0),
|
||||
closed: false,
|
||||
failure: None,
|
||||
sandbox_denied: false,
|
||||
}
|
||||
);
|
||||
|
||||
@@ -1124,6 +1168,8 @@ mod tests {
|
||||
output_notify: Arc::clone(&output_notify),
|
||||
open_streams: 2,
|
||||
closed: false,
|
||||
sandbox: SandboxType::None,
|
||||
sandbox_denied: false,
|
||||
})),
|
||||
);
|
||||
assert!(previous.is_none());
|
||||
|
||||
@@ -20,6 +20,7 @@ pub(crate) struct PreparedExecRequest {
|
||||
pub(crate) cwd: AbsolutePathBuf,
|
||||
pub(crate) env: HashMap<String, String>,
|
||||
pub(crate) arg0: Option<String>,
|
||||
pub(crate) sandbox: SandboxType,
|
||||
}
|
||||
|
||||
pub(crate) fn prepare_exec_request(
|
||||
@@ -33,6 +34,7 @@ pub(crate) fn prepare_exec_request(
|
||||
cwd: native_path(¶ms.cwd, "cwd")?,
|
||||
env,
|
||||
arg0: params.arg0.clone(),
|
||||
sandbox: SandboxType::None,
|
||||
});
|
||||
};
|
||||
let runtime_paths = runtime_paths
|
||||
@@ -116,6 +118,7 @@ pub(crate) fn prepare_exec_request(
|
||||
cwd: native_path(&request.cwd, "cwd")?,
|
||||
env: request.env,
|
||||
arg0: request.arg0,
|
||||
sandbox: request.sandbox,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -153,6 +153,9 @@ pub struct ReadResponse {
|
||||
pub exit_code: Option<i32>,
|
||||
pub closed: bool,
|
||||
pub failure: Option<String>,
|
||||
/// Whether the executor classified the process failure as a sandbox denial.
|
||||
#[serde(default)]
|
||||
pub sandbox_denied: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
|
||||
@@ -230,7 +230,7 @@ async fn assert_exec_process_streams_output(use_remote: bool) -> Result<()> {
|
||||
.await?;
|
||||
assert_eq!(session.process.process_id().as_str(), process_id);
|
||||
|
||||
let StartedExecProcess { process } = session;
|
||||
let StartedExecProcess { process, .. } = session;
|
||||
let wake_rx = process.subscribe_wake();
|
||||
let (output, exit_code, closed) = collect_process_output_from_reads(process, wake_rx).await?;
|
||||
assert_eq!(output, "session output\n");
|
||||
@@ -263,7 +263,7 @@ async fn assert_exec_process_pushes_events(use_remote: bool) -> Result<()> {
|
||||
.await?;
|
||||
assert_eq!(session.process.process_id().as_str(), process_id);
|
||||
|
||||
let StartedExecProcess { process } = session;
|
||||
let StartedExecProcess { process, .. } = session;
|
||||
let actual = collect_process_event_snapshots(process).await?;
|
||||
assert_eq!(
|
||||
actual,
|
||||
@@ -312,7 +312,7 @@ async fn assert_exec_process_replays_events_after_close(use_remote: bool) -> Res
|
||||
.await?;
|
||||
assert_eq!(session.process.process_id().as_str(), process_id);
|
||||
|
||||
let StartedExecProcess { process } = session;
|
||||
let StartedExecProcess { process, .. } = session;
|
||||
let wake_rx = process.subscribe_wake();
|
||||
let read_result = collect_process_output_from_reads(Arc::clone(&process), wake_rx).await?;
|
||||
assert_eq!(
|
||||
@@ -362,7 +362,7 @@ async fn assert_exec_process_retains_output_after_exit_until_streams_close(
|
||||
.await?;
|
||||
assert_eq!(session.process.process_id().as_str(), process_id);
|
||||
|
||||
let StartedExecProcess { process } = session;
|
||||
let StartedExecProcess { process, .. } = session;
|
||||
|
||||
let exit_response = timeout(
|
||||
Duration::from_secs(2),
|
||||
@@ -439,7 +439,7 @@ async fn assert_exec_process_write_then_read(use_remote: bool) -> Result<()> {
|
||||
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
session.process.write(b"hello\n".to_vec()).await?;
|
||||
let StartedExecProcess { process } = session;
|
||||
let StartedExecProcess { process, .. } = session;
|
||||
let wake_rx = process.subscribe_wake();
|
||||
let (output, exit_code, closed) = collect_process_output_from_reads(process, wake_rx).await?;
|
||||
|
||||
@@ -479,7 +479,7 @@ async fn assert_exec_process_write_then_read_without_tty(use_remote: bool) -> Re
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
let write_response = session.process.write(b"hello\n".to_vec()).await?;
|
||||
assert_eq!(write_response.status, WriteStatus::Accepted);
|
||||
let StartedExecProcess { process } = session;
|
||||
let StartedExecProcess { process, .. } = session;
|
||||
let wake_rx = process.subscribe_wake();
|
||||
let actual = collect_process_output_from_reads(process, wake_rx).await?;
|
||||
|
||||
@@ -513,7 +513,7 @@ async fn assert_exec_process_rejects_write_without_pipe_stdin(use_remote: bool)
|
||||
|
||||
let write_response = session.process.write(b"ignored\n".to_vec()).await?;
|
||||
assert_eq!(write_response.status, WriteStatus::StdinClosed);
|
||||
let StartedExecProcess { process } = session;
|
||||
let StartedExecProcess { process, .. } = session;
|
||||
let wake_rx = process.subscribe_wake();
|
||||
let (output, exit_code, closed) = collect_process_output_from_reads(process, wake_rx).await?;
|
||||
|
||||
@@ -547,7 +547,7 @@ async fn assert_exec_process_signal_interrupts_process(use_remote: bool) -> Resu
|
||||
.await?;
|
||||
assert_eq!(session.process.process_id().as_str(), process_id);
|
||||
|
||||
let StartedExecProcess { process } = session;
|
||||
let StartedExecProcess { process, .. } = session;
|
||||
let mut wake_rx = process.subscribe_wake();
|
||||
let mut ready_output = String::new();
|
||||
let mut after_seq = None;
|
||||
@@ -645,7 +645,7 @@ async fn assert_exec_process_preserves_queued_events_before_subscribe(
|
||||
|
||||
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||
|
||||
let StartedExecProcess { process } = session;
|
||||
let StartedExecProcess { process, .. } = session;
|
||||
let wake_rx = process.subscribe_wake();
|
||||
let (output, exit_code, closed) = collect_process_output_from_reads(process, wake_rx).await?;
|
||||
assert_eq!(output, "queued output\n");
|
||||
|
||||
@@ -70,7 +70,7 @@ async fn exec_server_starts_process_over_websocket() -> anyhow::Result<()> {
|
||||
assert_eq!(
|
||||
process_start_response,
|
||||
ExecResponse {
|
||||
process_id: ProcessId::from("proc-1")
|
||||
process_id: ProcessId::from("proc-1"),
|
||||
}
|
||||
);
|
||||
|
||||
@@ -135,7 +135,7 @@ async fn exec_server_defaults_omitted_pipe_stdin_to_closed_stdin() -> anyhow::Re
|
||||
assert_eq!(
|
||||
process_start_response,
|
||||
ExecResponse {
|
||||
process_id: ProcessId::from("proc-default-stdin")
|
||||
process_id: ProcessId::from("proc-default-stdin"),
|
||||
}
|
||||
);
|
||||
|
||||
|
||||
@@ -157,7 +157,7 @@ async fn remote_environment_routes_encrypted_exec_server_rpc() -> Result<()> {
|
||||
assert_eq!(
|
||||
response,
|
||||
ExecResponse {
|
||||
process_id: ProcessId::from("proc-1")
|
||||
process_id: ProcessId::from("proc-1"),
|
||||
}
|
||||
);
|
||||
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
use codex_protocol::exec_output::ExecToolCallOutput;
|
||||
|
||||
use crate::SandboxType;
|
||||
|
||||
/// Returns whether a failed command was likely denied by the selected sandbox.
|
||||
pub fn is_likely_sandbox_denied(
|
||||
sandbox_type: SandboxType,
|
||||
exec_output: &ExecToolCallOutput,
|
||||
) -> bool {
|
||||
if sandbox_type == SandboxType::None || exec_output.exit_code == 0 {
|
||||
return false;
|
||||
}
|
||||
|
||||
const SANDBOX_DENIED_KEYWORDS: [&str; 7] = [
|
||||
"operation not permitted",
|
||||
"permission denied",
|
||||
"read-only file system",
|
||||
"seccomp",
|
||||
"sandbox",
|
||||
"landlock",
|
||||
"failed to write file",
|
||||
];
|
||||
|
||||
let has_sandbox_keyword = [
|
||||
&exec_output.stderr.text,
|
||||
&exec_output.stdout.text,
|
||||
&exec_output.aggregated_output.text,
|
||||
]
|
||||
.into_iter()
|
||||
.any(|section| {
|
||||
let lower = section.to_lowercase();
|
||||
SANDBOX_DENIED_KEYWORDS
|
||||
.iter()
|
||||
.any(|needle| lower.contains(needle))
|
||||
});
|
||||
|
||||
if has_sandbox_keyword {
|
||||
return true;
|
||||
}
|
||||
|
||||
const QUICK_REJECT_EXIT_CODES: [i32; 3] = [2, 126, 127];
|
||||
if QUICK_REJECT_EXIT_CODES.contains(&exec_output.exit_code) {
|
||||
return false;
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
{
|
||||
const EXIT_CODE_SIGNAL_BASE: i32 = 128;
|
||||
if sandbox_type == SandboxType::LinuxSeccomp
|
||||
&& exec_output.exit_code == EXIT_CODE_SIGNAL_BASE + libc::SIGSYS
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
#[cfg(target_os = "linux")]
|
||||
mod bwrap;
|
||||
mod denial;
|
||||
pub mod landlock;
|
||||
mod manager;
|
||||
pub mod policy_transforms;
|
||||
@@ -11,6 +12,7 @@ mod windows;
|
||||
pub use bwrap::find_system_bwrap_in_path;
|
||||
#[cfg(target_os = "linux")]
|
||||
pub use bwrap::system_bwrap_warning;
|
||||
pub use denial::is_likely_sandbox_denied;
|
||||
pub use manager::SandboxCommand;
|
||||
pub use manager::SandboxDirectSpawnTransformRequest;
|
||||
pub use manager::SandboxExecRequest;
|
||||
|
||||
Reference in New Issue
Block a user