From 0e6f153732cc12bb8d82672c05b4d4840268af33 Mon Sep 17 00:00:00 2001 From: ioanbudea-OAI Date: Tue, 10 Nov 2026 02:11:20 +0000 Subject: [PATCH] Derive sandbox telemetry labels directly from permission profiles (#60500) ## What changed Compute the `sandbox_policy` label for `codex.conversation_starts` from the effective `PermissionProfile` without constructing a legacy `SandboxPolicy`. Preserve the historical labels and native-path compatibility fallbacks, including `workspace-write` for writable profiles that cannot be represented by a legacy policy. Share the full-disk write compatibility check with session startup and the write-intent check with legacy policy conversion. ## Testing Extend telemetry tests to assert sandbox labels in both logs and traces across disabled, external, read-only, workspace-write, and full-disk profiles, including unrenderable writable URIs and read restrictions. Add protocol assertions for legacy write-intent detection. GitOrigin-RevId: 00f11d763c366453e7bb0899546a4ebbdb93c4d5 --- codex-rs/core/src/session/mod.rs | 9 +- codex-rs/core/src/session/session.rs | 5 +- codex-rs/otel/src/events/session_telemetry.rs | 7 +- .../tests/suite/otel_export_routing_policy.rs | 143 ++++++++++++++++-- codex-rs/protocol/src/models.rs | 24 +++ codex-rs/protocol/src/permissions.rs | 52 +++++-- codex-rs/protocol/src/permissions_tests.rs | 8 + 7 files changed, 212 insertions(+), 36 deletions(-) diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 32084e62c7..9d68f4a359 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -842,12 +842,9 @@ impl Session { let permission_profile = config.permissions.effective_permission_profile(); if permission_profile.enforcement() != SandboxEnforcement::External && permission_profile.network_sandbox_policy().is_enabled() - && { - let file_system = permission_profile.file_system_sandbox_policy(); - file_system.has_full_disk_write_access() - // Preserve the legacy fallback for writable URIs that cannot be projected locally. - && file_system.entries.iter().all(|entry| entry.legacy_writable_path().is_ok()) - } + && permission_profile + .file_system_sandbox_policy() + .has_legacy_full_disk_write_access() { let config = Arc::make_mut(&mut config); let permission_profile = PermissionProfile::workspace_write(); diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 2afcb80afe..54374144fd 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1552,9 +1552,8 @@ impl Session { config.model_context_window, config.model_auto_compact_token_limit, config.permissions.approval_policy.value(), - config - .permissions - .legacy_sandbox_policy(session_configuration.cwd().as_path()), + &config.permissions.effective_permission_profile(), + session_configuration.cwd().as_path(), mcp_server_names.iter().map(String::as_str).collect(), &crate::config::legacy_multi_agent_disabled_sources(&config.config_layer_stack), ); diff --git a/codex-rs/otel/src/events/session_telemetry.rs b/codex-rs/otel/src/events/session_telemetry.rs index 74381ece77..f6099b84df 100644 --- a/codex-rs/otel/src/events/session_telemetry.rs +++ b/codex-rs/otel/src/events/session_telemetry.rs @@ -48,11 +48,11 @@ use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::ToolResultLogConfig; use codex_protocol::error::CodexErr; use codex_protocol::error::CodexErrKind; +use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; -use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::TokenUsage; use codex_protocol::user_input::UserInput; @@ -62,6 +62,7 @@ use opentelemetry_sdk::metrics::data::ResourceMetrics; use reqwest::Error; use reqwest::Response; use std::future::Future; +use std::path::Path; use std::time::Duration; use std::time::Instant; use tokio::time::error::Elapsed; @@ -693,10 +694,12 @@ impl SessionTelemetry { context_window: Option, auto_compact_token_limit: Option, approval_policy: AskForApproval, - sandbox_policy: SandboxPolicy, + permission_profile: &PermissionProfile, + cwd: &Path, mcp_servers: Vec<&str>, legacy_multi_agent_disabled_sources: &[&str], ) { + let sandbox_policy = permission_profile.legacy_sandbox_policy_label(cwd); log_and_trace_event!( self, common: { diff --git a/codex-rs/otel/tests/suite/otel_export_routing_policy.rs b/codex-rs/otel/tests/suite/otel_export_routing_policy.rs index 3d6052cc13..7105c4d6cf 100644 --- a/codex-rs/otel/tests/suite/otel_export_routing_policy.rs +++ b/codex-rs/otel/tests/suite/otel_export_routing_policy.rs @@ -34,15 +34,22 @@ use codex_protocol::ToolName; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::ToolResultLogConfig; use codex_protocol::models::ImageReference; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::FileSystemSpecialPath; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::GuardianAssessmentEvent; use codex_protocol::protocol::GuardianAssessmentOutcome; use codex_protocol::protocol::GuardianAssessmentStatus; -use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::TokenUsage; use codex_protocol::user_input::UserInput; +use codex_utils_absolute_path::AbsolutePathBuf; use eventsource_stream::Event as StreamEvent; fn log_attributes(record: &SdkLogRecord) -> BTreeMap { @@ -874,17 +881,91 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() { .with_auth_env(auth_env_metadata()); let root_span = tracing::info_span!("root"); let _root_guard = root_span.enter(); - manager.conversation_starts( - "openai", - /*reasoning_effort*/ None, - ReasoningSummary::Auto, - /*context_window*/ None, - /*auto_compact_token_limit*/ None, - AskForApproval::Never, - SandboxPolicy::DangerFullAccess, - Vec::new(), - /*legacy_multi_agent_disabled_sources*/ &[], + let cwd = tempfile::tempdir().expect("telemetry cwd"); + let root_write = FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Write, ); + for permission_profile in [ + PermissionProfile::Disabled, + PermissionProfile::read_only(), + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::unrestricted(), + NetworkSandboxPolicy::Restricted, + ), + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + FileSystemAccessMode::Write, + )]), + NetworkSandboxPolicy::Restricted, + ), + PermissionProfile::workspace_write(), + // Effective workspace roots are explicit paths without a full-root grant. + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + AbsolutePathBuf::from_absolute_path(cwd.path()) + .expect("absolute writable workspace") + .into(), + FileSystemAccessMode::Write, + )]), + NetworkSandboxPolicy::Restricted, + ), + PermissionProfile::External { + network: NetworkSandboxPolicy::Enabled, + }, + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![root_write.clone()]), + NetworkSandboxPolicy::Enabled, + ), + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![root_write.clone()]), + NetworkSandboxPolicy::Restricted, + ), + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![ + root_write.clone(), + FileSystemSandboxEntry::new( + FileSystemPath::Path { + path: "file:///%00/bad/path/YQ" + .parse() + .expect("opaque writable URI"), + }, + FileSystemAccessMode::Write, + ), + ]), + NetworkSandboxPolicy::Enabled, + ), + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![ + root_write, + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + FileSystemAccessMode::Read, + ), + ]), + NetworkSandboxPolicy::Enabled, + ), + ] { + manager.conversation_starts( + "openai", + /*reasoning_effort*/ None, + ReasoningSummary::Auto, + /*context_window*/ None, + /*auto_compact_token_limit*/ None, + AskForApproval::Never, + &permission_profile, + cwd.path(), + Vec::new(), + /*legacy_multi_agent_disabled_sources*/ &[], + ); + } let agent_identity_telemetry = AgentIdentityTelemetry { agent_id: "agent-runtime-otel".to_string(), task_id: "task-run-otel".to_string(), @@ -913,6 +994,31 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() { let logs = log_exporter.get_emitted_logs().expect("log export"); let conversation_log = find_log_by_event_name(&logs, "codex.conversation_starts"); + let expected_policy_labels = vec![ + "danger-full-access", + "read-only", + "external-sandbox", + "workspace-write", + "workspace-write", + "workspace-write", + "external-sandbox", + "danger-full-access", + "external-sandbox", + "workspace-write", + "workspace-write", + ]; + assert_eq!( + logs.iter() + .map(|log| log_attributes(&log.record)) + .filter(|attributes| { + attributes + .get("event.name") + .is_some_and(|name| name == "codex.conversation_starts") + }) + .map(|attributes| attributes["sandbox_policy"].clone()) + .collect::>(), + expected_policy_labels, + ); let conversation_log_attrs = log_attributes(&conversation_log.record); assert_eq!( conversation_log_attrs @@ -991,6 +1097,21 @@ fn otel_export_routing_policy_routes_api_request_auth_observability() { let conversation_trace_event = find_span_event_by_name_attr(&spans[0].events.events, "codex.conversation_starts"); let conversation_trace_attrs = span_event_attributes(conversation_trace_event); + assert_eq!( + spans[0] + .events + .events + .iter() + .map(span_event_attributes) + .filter(|attributes| { + attributes + .get("event.name") + .is_some_and(|name| name == "codex.conversation_starts") + }) + .map(|attributes| attributes["sandbox_policy"].clone()) + .collect::>(), + expected_policy_labels, + ); assert_eq!( conversation_trace_attrs .get("auth.env_provider_key_present") diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index cdeac4a598..d9f2de8178 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -689,6 +689,30 @@ impl PermissionProfile { } } + /// Returns the historical `sandbox_policy` diagnostic label without constructing + /// a legacy policy. Retains native-path compatibility fallbacks rather than + /// evaluating executor authorization. + pub fn legacy_sandbox_policy_label(&self, cwd: &Path) -> &'static str { + match self { + Self::Disabled => "danger-full-access", + Self::External { .. } => "external-sandbox", + Self::Managed { network, .. } => { + let file_system = self.file_system_sandbox_policy(); + if file_system.has_legacy_full_disk_write_access() { + if network.is_enabled() { + "danger-full-access" + } else { + "external-sandbox" + } + } else if file_system.has_legacy_write_intent(cwd) { + "workspace-write" + } else { + "read-only" + } + } + } + } + pub fn to_legacy_sandbox_policy(&self, cwd: &Path) -> io::Result { match self { Self::Managed { diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index d9c31241ef..6cc2839485 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -1494,6 +1494,42 @@ impl FileSystemSandboxPolicy { self.get_writable_roots_with_cwd_impl(cwd, FileSystemPolicyProjection::LinuxMounts) } + /// Reports full-disk writes when every explicit writable path can be projected locally. + /// Preserves the legacy fallback used by startup checks and diagnostic labels, + /// using this host's path convention. + pub fn has_legacy_full_disk_write_access(&self) -> bool { + self.has_full_disk_write_access() + && self + .entries + .iter() + .all(|entry| entry.legacy_writable_path().is_ok()) + } + + /// Reports entry write intent recognized by the legacy adapter and its diagnostic labels. + /// This does not inspect path existence or apply read/deny precedence: missing and + /// unrenderable writable paths still count, while unknown special paths and globs do not. + pub fn has_legacy_write_intent(&self, cwd: &Path) -> bool { + self.entries.iter().any(|entry| { + entry.access.can_write() + && match &entry.path { + FileSystemPath::Path { .. } => true, + FileSystemPath::GlobPattern { .. } => false, + FileSystemPath::Special { value } => match value { + FileSystemSpecialPath::Root + | FileSystemSpecialPath::Tmpdir + | FileSystemSpecialPath::ProjectRoots { subpath: None } => true, + FileSystemSpecialPath::ProjectRoots { subpath: Some(_) } => { + AbsolutePathBuf::from_absolute_path(cwd).is_ok() + } + FileSystemSpecialPath::SlashTmp => cfg!(unix), + FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => { + false + } + }, + } + }) + } + /// Reports configured writable roots for diagnostics without inspecting the filesystem. /// /// Unlike runtime root resolution, this includes configured roots even if they @@ -1888,7 +1924,6 @@ impl FileSystemSandboxPolicy { let mut writable_roots = Vec::new(); let mut tmpdir_writable = false; let mut slash_tmp_writable = false; - let mut unbridgeable_root_write = false; for entry in &self.entries { match &entry.path { @@ -1903,14 +1938,7 @@ impl FileSystemSandboxPolicy { } } FileSystemPath::Special { value } => match value { - FileSystemSpecialPath::Root => match entry.access { - FileSystemAccessMode::Deny => {} - FileSystemAccessMode::Read => {} - FileSystemAccessMode::Write => { - unbridgeable_root_write = true; - } - }, - FileSystemSpecialPath::Minimal => {} + FileSystemSpecialPath::Root | FileSystemSpecialPath::Minimal => {} FileSystemSpecialPath::ProjectRoots { subpath } => { if subpath.is_none() && entry.access.can_write() { workspace_root_writable = true; @@ -1956,11 +1984,7 @@ impl FileSystemSandboxPolicy { exclude_tmpdir_env_var: !tmpdir_writable, exclude_slash_tmp: !slash_tmp_writable, } - } else if unbridgeable_root_write - || !writable_roots.is_empty() - || tmpdir_writable - || (cfg!(unix) && slash_tmp_writable) - { + } else if self.has_legacy_write_intent(cwd) { return Err(io::Error::new( io::ErrorKind::InvalidInput, "permissions profile requests filesystem writes outside the workspace root, which is not supported until the runtime enforces FileSystemSandboxPolicy directly", diff --git a/codex-rs/protocol/src/permissions_tests.rs b/codex-rs/protocol/src/permissions_tests.rs index 68fd36e49b..95930032cb 100644 --- a/codex-rs/protocol/src/permissions_tests.rs +++ b/codex-rs/protocol/src/permissions_tests.rs @@ -113,6 +113,14 @@ fn writable_root_presence_matches_materialized_roots() { )]), ]; + assert_eq!( + policies + .iter() + .map(|policy| policy.has_legacy_write_intent(cwd.path())) + .collect::>(), + vec![false, false, false, true, true, true, false], + ); + for policy in policies { assert_eq!( policy.has_configured_writable_roots_with_cwd(cwd.path()),