From e320493090ff6d45b7d185ec1f23feddaaa3a04a Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sun, 12 Apr 2026 15:40:04 -0700 Subject: [PATCH] refactor sandbox violation monitoring --- codex-rs/core/src/exec.rs | 55 +--- codex-rs/core/src/tools/network_approval.rs | 2 + .../tools/runtimes/shell/unix_escalation.rs | 4 +- codex-rs/core/src/unified_exec/process.rs | 4 +- codex-rs/sandboxing/src/lib.rs | 10 + codex-rs/sandboxing/src/violation.rs | 278 ++++++++++++++++++ codex-rs/sandboxing/src/violation_tests.rs | 121 ++++++++ 7 files changed, 418 insertions(+), 56 deletions(-) create mode 100644 codex-rs/sandboxing/src/violation.rs create mode 100644 codex-rs/sandboxing/src/violation_tests.rs diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index a6d958b107..6faabc9b74 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -44,6 +44,7 @@ use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; +use codex_sandboxing::record_filesystem_sandbox_violation; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use codex_utils_pty::process_group::kill_child_process_group; @@ -751,7 +752,7 @@ fn finalize_exec_result( })); } - if is_likely_sandbox_denied(sandbox_type, &exec_output) { + if record_filesystem_sandbox_violation(sandbox_type, &exec_output).is_some() { return Err(CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(exec_output), network_policy_decision: None, @@ -776,57 +777,7 @@ 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 + codex_sandboxing::is_likely_sandbox_denied(sandbox_type, exec_output) } #[derive(Debug)] diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 39441d96f5..2b46eb8688 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -27,6 +27,7 @@ use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::WarningEvent; +use codex_sandboxing::record_network_sandbox_violation; use indexmap::IndexMap; use std::collections::HashMap; use std::collections::HashSet; @@ -677,6 +678,7 @@ pub(crate) fn build_blocked_request_observer( Arc::new(move |blocked: BlockedRequest| { let network_approval = Arc::clone(&network_approval); async move { + record_network_sandbox_violation(&blocked); network_approval.record_blocked_request(blocked).await; } }) diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index fef8db5ca9..568bfbd364 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -2,7 +2,6 @@ use super::ShellRequest; use crate::exec::ExecCapturePolicy; use crate::exec::ExecExpiration; use crate::exec::cancel_when_either; -use crate::exec::is_likely_sandbox_denied; use crate::guardian::GuardianApprovalRequest; use crate::guardian::guardian_rejection_message; use crate::guardian::guardian_timeout_message; @@ -46,6 +45,7 @@ use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; +use codex_sandboxing::record_filesystem_sandbox_violation; use codex_shell_command::bash::parse_shell_lc_plain_commands; use codex_shell_command::bash::parse_shell_lc_single_command_prefix; use codex_shell_escalation::EscalateServer; @@ -997,7 +997,7 @@ fn map_exec_result( }))); } - if is_likely_sandbox_denied(sandbox, &output) { + if record_filesystem_sandbox_violation(sandbox, &output).is_some() { return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), network_policy_decision: None, diff --git a/codex-rs/core/src/unified_exec/process.rs b/codex-rs/core/src/unified_exec/process.rs index 9671429d01..f5f26edb7f 100644 --- a/codex-rs/core/src/unified_exec/process.rs +++ b/codex-rs/core/src/unified_exec/process.rs @@ -12,7 +12,6 @@ use tokio::task::JoinHandle; use tokio::time::Duration; use tokio_util::sync::CancellationToken; -use crate::exec::is_likely_sandbox_denied; use codex_exec_server::ExecProcess; use codex_exec_server::ReadResponse as ExecReadResponse; use codex_exec_server::StartedExecProcess; @@ -21,6 +20,7 @@ use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::exec_output::StreamOutput; use codex_protocol::protocol::TruncationPolicy; use codex_sandboxing::SandboxType; +use codex_sandboxing::record_filesystem_sandbox_violation; use codex_utils_output_truncation::formatted_truncate_text; use codex_utils_pty::ExecCommandSession; use codex_utils_pty::SpawnedPty; @@ -265,7 +265,7 @@ impl UnifiedExecProcess { aggregated_output: StreamOutput::new(text.to_string()), ..Default::default() }; - if is_likely_sandbox_denied(sandbox_type, &exec_output) { + if record_filesystem_sandbox_violation(sandbox_type, &exec_output).is_some() { let snippet = formatted_truncate_text( text, TruncationPolicy::Tokens(UNIFIED_EXEC_OUTPUT_MAX_TOKENS), diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index c70393db8a..c354a93c4a 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -5,6 +5,7 @@ mod manager; pub mod policy_transforms; #[cfg(target_os = "macos")] pub mod seatbelt; +mod violation; #[cfg(target_os = "linux")] pub use bwrap::find_system_bwrap_in_path; @@ -19,6 +20,15 @@ pub use manager::SandboxType; pub use manager::SandboxablePreference; pub use manager::compatibility_sandbox_policy_for_permission_profile; pub use manager::get_platform_sandbox; +pub use violation::FileSystemSandboxViolation; +pub use violation::FileSystemSandboxViolationReason; +pub use violation::NetworkSandboxViolation; +pub use violation::SandboxViolationEvent; +pub use violation::classify_filesystem_sandbox_violation; +pub use violation::is_likely_sandbox_denied; +pub use violation::record_filesystem_sandbox_violation; +pub use violation::record_network_sandbox_violation; +pub use violation::record_sandbox_violation; use codex_protocol::error::CodexErr; diff --git a/codex-rs/sandboxing/src/violation.rs b/codex-rs/sandboxing/src/violation.rs new file mode 100644 index 0000000000..8517227370 --- /dev/null +++ b/codex-rs/sandboxing/src/violation.rs @@ -0,0 +1,278 @@ +use crate::SandboxType; +use codex_network_proxy::BlockedRequest; +use codex_protocol::exec_output::ExecToolCallOutput; +use tracing::warn; + +const EXIT_CODE_SIGNAL_BASE: i32 = 128; +const OUTPUT_SNIPPET_MAX_CHARS: usize = 512; + +const SANDBOX_DENIED_KEYWORDS: [(FileSystemSandboxViolationReason, &str); 7] = [ + ( + FileSystemSandboxViolationReason::OperationNotPermitted, + "operation not permitted", + ), + ( + FileSystemSandboxViolationReason::PermissionDenied, + "permission denied", + ), + ( + FileSystemSandboxViolationReason::ReadOnlyFileSystem, + "read-only file system", + ), + (FileSystemSandboxViolationReason::Seccomp, "seccomp"), + (FileSystemSandboxViolationReason::Sandbox, "sandbox"), + (FileSystemSandboxViolationReason::Landlock, "landlock"), + ( + FileSystemSandboxViolationReason::FailedToWriteFile, + "failed to write file", + ), +]; + +// Quick rejects: well-known non-sandbox shell exit codes. +// 2: misuse of shell builtins +// 126: permission denied +// 127: command not found +const QUICK_REJECT_EXIT_CODES: [i32; 3] = [2, 126, 127]; + +/// A normalized sandbox violation observed by Codex sandbox enforcement. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum SandboxViolationEvent { + FileSystem(FileSystemSandboxViolation), + Network(NetworkSandboxViolation), +} + +/// A filesystem sandbox denial inferred from a sandboxed process result. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct FileSystemSandboxViolation { + pub sandbox_type: SandboxType, + pub reason: FileSystemSandboxViolationReason, + pub path: Option, + pub output_snippet: String, +} + +/// Normalized reasons used when classifying filesystem sandbox denials. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum FileSystemSandboxViolationReason { + OperationNotPermitted, + PermissionDenied, + ReadOnlyFileSystem, + Seccomp, + Sandbox, + Landlock, + FailedToWriteFile, + SignalSyscall, +} + +impl FileSystemSandboxViolationReason { + pub const fn as_str(self) -> &'static str { + match self { + Self::OperationNotPermitted => "operation_not_permitted", + Self::PermissionDenied => "permission_denied", + Self::ReadOnlyFileSystem => "read_only_file_system", + Self::Seccomp => "seccomp", + Self::Sandbox => "sandbox", + Self::Landlock => "landlock", + Self::FailedToWriteFile => "failed_to_write_file", + Self::SignalSyscall => "sigsys", + } + } +} + +/// A network sandbox denial reported by the managed network proxy. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct NetworkSandboxViolation { + pub host: String, + pub reason: String, + pub client: Option, + pub method: Option, + pub protocol: String, + pub decision: Option, + pub source: Option, + pub port: Option, + pub timestamp: i64, +} + +impl NetworkSandboxViolation { + pub fn from_blocked_request(blocked: &BlockedRequest) -> Self { + Self { + host: blocked.host.clone(), + reason: blocked.reason.clone(), + client: blocked.client.clone(), + method: blocked.method.clone(), + protocol: blocked.protocol.clone(), + decision: blocked.decision.clone(), + source: blocked.source.clone(), + port: blocked.port, + timestamp: blocked.timestamp, + } + } +} + +/// Classify a sandboxed process result as a filesystem sandbox violation. +pub fn classify_filesystem_sandbox_violation( + sandbox_type: SandboxType, + exec_output: &ExecToolCallOutput, +) -> Option { + if sandbox_type == SandboxType::None || exec_output.exit_code == 0 { + return None; + } + + if let Some(reason) = filesystem_reason_from_output(exec_output) { + return Some(FileSystemSandboxViolation { + sandbox_type, + reason, + path: extract_denied_path(exec_output), + output_snippet: output_snippet(exec_output), + }); + } + + if QUICK_REJECT_EXIT_CODES.contains(&exec_output.exit_code) { + return None; + } + + #[cfg(unix)] + { + if sandbox_type == SandboxType::LinuxSeccomp + && exec_output.exit_code == EXIT_CODE_SIGNAL_BASE + libc::SIGSYS + { + return Some(FileSystemSandboxViolation { + sandbox_type, + reason: FileSystemSandboxViolationReason::SignalSyscall, + path: None, + output_snippet: output_snippet(exec_output), + }); + } + } + + None +} + +/// Preserve the legacy boolean sandbox-denial check for call sites that only need a retry decision. +pub fn is_likely_sandbox_denied( + sandbox_type: SandboxType, + exec_output: &ExecToolCallOutput, +) -> bool { + classify_filesystem_sandbox_violation(sandbox_type, exec_output).is_some() +} + +/// Record a filesystem sandbox violation, returning the classified event when one was found. +pub fn record_filesystem_sandbox_violation( + sandbox_type: SandboxType, + exec_output: &ExecToolCallOutput, +) -> Option { + let violation = classify_filesystem_sandbox_violation(sandbox_type, exec_output)?; + record_sandbox_violation(&SandboxViolationEvent::FileSystem(violation.clone())); + Some(violation) +} + +/// Record a network sandbox violation from a managed-proxy blocked request. +pub fn record_network_sandbox_violation(blocked: &BlockedRequest) -> NetworkSandboxViolation { + let violation = NetworkSandboxViolation::from_blocked_request(blocked); + record_sandbox_violation(&SandboxViolationEvent::Network(violation.clone())); + violation +} + +/// Emit a sandbox violation to the tracing stack. +pub fn record_sandbox_violation(event: &SandboxViolationEvent) { + match event { + SandboxViolationEvent::FileSystem(violation) => { + let path = violation.path.as_deref().unwrap_or("unknown"); + warn!( + "recorded sandbox violation: resource=filesystem sandbox={} reason={} path={}", + violation.sandbox_type.as_metric_tag(), + violation.reason.as_str(), + path + ); + } + SandboxViolationEvent::Network(violation) => { + warn!( + "recorded sandbox violation: resource=network protocol={} host={} port={:?} reason={} method={:?} client={:?} decision={:?} source={:?}", + violation.protocol, + violation.host, + violation.port, + violation.reason, + violation.method, + violation.client, + violation.decision, + violation.source + ); + } + } +} + +fn filesystem_reason_from_output( + exec_output: &ExecToolCallOutput, +) -> Option { + [ + &exec_output.stderr.text, + &exec_output.stdout.text, + &exec_output.aggregated_output.text, + ] + .into_iter() + .find_map(|section| { + let lower = section.to_lowercase(); + SANDBOX_DENIED_KEYWORDS + .iter() + .find_map(|(reason, needle)| lower.contains(needle).then_some(*reason)) + }) +} + +fn extract_denied_path(exec_output: &ExecToolCallOutput) -> Option { + [ + &exec_output.stderr.text, + &exec_output.stdout.text, + &exec_output.aggregated_output.text, + ] + .into_iter() + .find_map(|section| extract_denied_path_from_text(section)) +} + +fn extract_denied_path_from_text(text: &str) -> Option { + const PATH_MARKERS: [&str; 3] = [ + ": operation not permitted", + ": permission denied", + ": read-only file system", + ]; + + for line in text.lines() { + let lower = line.to_lowercase(); + for marker in PATH_MARKERS { + let Some(marker_start) = lower.find(marker) else { + continue; + }; + let candidate_prefix = &line[..marker_start]; + let candidate = candidate_prefix + .rsplit_once(": ") + .map_or(candidate_prefix, |(_, path)| path) + .trim() + .trim_matches('"') + .trim_matches('\''); + if candidate.starts_with('/') + || candidate.starts_with("./") + || candidate.starts_with("../") + { + return Some(candidate.to_string()); + } + } + } + + None +} + +fn output_snippet(exec_output: &ExecToolCallOutput) -> String { + [ + &exec_output.stderr.text, + &exec_output.stdout.text, + &exec_output.aggregated_output.text, + ] + .into_iter() + .find_map(|section| { + let trimmed = section.trim(); + (!trimmed.is_empty()).then(|| trimmed.chars().take(OUTPUT_SNIPPET_MAX_CHARS).collect()) + }) + .unwrap_or_default() +} + +#[cfg(test)] +#[path = "violation_tests.rs"] +mod tests; diff --git a/codex-rs/sandboxing/src/violation_tests.rs b/codex-rs/sandboxing/src/violation_tests.rs new file mode 100644 index 0000000000..f0ad3894b4 --- /dev/null +++ b/codex-rs/sandboxing/src/violation_tests.rs @@ -0,0 +1,121 @@ +use super::*; +use codex_network_proxy::BlockedRequest; +use codex_network_proxy::BlockedRequestArgs; +use codex_protocol::exec_output::ExecToolCallOutput; +use codex_protocol::exec_output::StreamOutput; +use pretty_assertions::assert_eq; +use std::time::Duration; + +fn make_exec_output( + exit_code: i32, + stdout: &str, + stderr: &str, + aggregated: &str, +) -> ExecToolCallOutput { + ExecToolCallOutput { + exit_code, + stdout: StreamOutput::new(stdout.to_string()), + stderr: StreamOutput::new(stderr.to_string()), + aggregated_output: StreamOutput::new(aggregated.to_string()), + duration: Duration::from_millis(1), + timed_out: false, + } +} + +#[test] +fn classifies_filesystem_violation_with_path() { + let output = make_exec_output( + /*exit_code*/ 1, + "", + "bash: /private/tmp/denied: Operation not permitted", + "", + ); + + assert_eq!( + classify_filesystem_sandbox_violation(SandboxType::MacosSeatbelt, &output), + Some(FileSystemSandboxViolation { + sandbox_type: SandboxType::MacosSeatbelt, + reason: FileSystemSandboxViolationReason::OperationNotPermitted, + path: Some("/private/tmp/denied".to_string()), + output_snippet: "bash: /private/tmp/denied: Operation not permitted".to_string(), + }) + ); +} + +#[test] +fn classifies_filesystem_violation_from_aggregated_output() { + let output = make_exec_output( + /*exit_code*/ 101, + "", + "", + "cargo failed: Read-only file system when writing target", + ); + + assert_eq!( + classify_filesystem_sandbox_violation(SandboxType::MacosSeatbelt, &output), + Some(FileSystemSandboxViolation { + sandbox_type: SandboxType::MacosSeatbelt, + reason: FileSystemSandboxViolationReason::ReadOnlyFileSystem, + path: None, + output_snippet: "cargo failed: Read-only file system when writing target".to_string(), + }) + ); +} + +#[cfg(unix)] +#[test] +fn classifies_linux_sigsys_exit() { + let output = make_exec_output( + /*exit_code*/ EXIT_CODE_SIGNAL_BASE + libc::SIGSYS, + "", + "", + "", + ); + + assert_eq!( + classify_filesystem_sandbox_violation(SandboxType::LinuxSeccomp, &output), + Some(FileSystemSandboxViolation { + sandbox_type: SandboxType::LinuxSeccomp, + reason: FileSystemSandboxViolationReason::SignalSyscall, + path: None, + output_snippet: String::new(), + }) + ); +} + +#[test] +fn preserves_boolean_denial_semantics_for_non_sandbox_mode() { + let output = make_exec_output(/*exit_code*/ 1, "", "Operation not permitted", ""); + + assert!(!is_likely_sandbox_denied(SandboxType::None, &output)); +} + +#[test] +fn converts_blocked_request_to_network_violation() { + let blocked = BlockedRequest::new(BlockedRequestArgs { + host: "example.com".to_string(), + reason: "not_allowed".to_string(), + client: Some("curl".to_string()), + method: Some("CONNECT".to_string()), + mode: None, + protocol: "https".to_string(), + decision: Some("block".to_string()), + source: Some("policy".to_string()), + port: Some(443), + }); + + assert_eq!( + NetworkSandboxViolation::from_blocked_request(&blocked), + NetworkSandboxViolation { + host: "example.com".to_string(), + reason: "not_allowed".to_string(), + client: Some("curl".to_string()), + method: Some("CONNECT".to_string()), + protocol: "https".to_string(), + decision: Some("block".to_string()), + source: Some("policy".to_string()), + port: Some(443), + timestamp: blocked.timestamp, + } + ); +}