From 400a1e1e82bc558bd52df2c603829e894385bf67 Mon Sep 17 00:00:00 2001 From: ioanbudea-OAI Date: Fri, 13 Nov 2026 17:41:25 +0000 Subject: [PATCH] Use native permission profiles for diagnostics and sandbox checks (#61369) ## What changed - Derive sampling diagnostic and rollout trace labels directly from permission profiles instead of constructing legacy sandbox policies. Rename the trace metadata field from `sandbox_policy` to `permissions_profile`. - Extract `with_workspace_write_cwd` to share default workspace metadata protections between legacy policy conversion and the TUI's sandbox-mode compatibility check. Build the check's workspace-write profile from filesystem and network permissions directly. ## Testing Add a regression test confirming that the sandbox-mode compatibility check accepts standard workspace metadata protections and rejects a profile with an additional writable `.git` root. GitOrigin-RevId: f20d23afd80dff6347865df3d6a94d6f5e90e1e0 --- codex-rs/core/src/session/session.rs | 19 ++------- codex-rs/core/src/session/tests.rs | 13 +------ codex-rs/core/src/session/turn.rs | 8 +++- codex-rs/core/src/session/turn_context.rs | 8 ---- .../src/tools/tool_dispatch_trace_tests.rs | 2 +- codex-rs/protocol/src/permissions.rs | 26 ++++++++----- codex-rs/rollout-trace/src/thread.rs | 2 +- codex-rs/rollout-trace/src/thread_tests.rs | 6 +-- codex-rs/tui/src/app_server_session.rs | 39 +++++++++++++++++-- 9 files changed, 70 insertions(+), 53 deletions(-) diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index a146a524de..017d5124da 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -265,17 +265,6 @@ impl SessionConfiguration { .set_legacy_permission_profile(permission_profile) } - pub(super) fn sandbox_policy( - &self, - environments: &[TurnEnvironmentSelection], - ) -> SandboxPolicy { - let permission_profile = self.materialized_permission_profile(environments); - codex_sandboxing::compatibility_sandbox_policy_for_permission_profile( - &permission_profile, - self.cwd(), - ) - } - pub(super) fn file_system_sandbox_policy( &self, environments: &[TurnEnvironmentSelection], @@ -1334,10 +1323,10 @@ impl Session { .approval_policy .value() .to_string(), - sandbox_policy: format!( - "{:?}", - session_configuration.sandbox_policy(environment_selections) - ), + permissions_profile: session_configuration + .materialized_permission_profile(environment_selections) + .legacy_sandbox_policy_label(session_configuration.cwd()) + .to_string(), }; let mut post_session_configured_events = Vec::::new(); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index b2fcaf1733..f3a3ad7d56 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5636,7 +5636,7 @@ async fn session_configuration_apply_permission_profile_accepts_direct_write_roo let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: external_write_path.clone().into(), + path: external_write_path.into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -5661,15 +5661,6 @@ async fn session_configuration_apply_permission_profile_accepts_direct_write_roo updated.file_system_sandbox_policy(&[]), file_system_sandbox_policy ); - assert_eq!( - updated.sandbox_policy(&[]), - SandboxPolicy::WorkspaceWrite { - writable_roots: vec![external_write_path], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - } - ); } #[test_case::test_case(false; "ordinary_proxy")] @@ -8877,7 +8868,7 @@ async fn submission_loop_channel_close_runs_full_thread_teardown() -> anyhow::Re model: turn_context.model_info().slug.clone(), provider_name: config.model_provider_id.clone(), approval_policy: "never".to_string(), - sandbox_policy: "danger-full-access".to_string(), + permissions_profile: "danger-full-access".to_string(), }, )?; let live_thread = LiveThread::create( diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 0dd75a878d..fb40b7e763 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -2909,10 +2909,16 @@ async fn try_run_sampling_request( draining_for_mailbox: &mut bool, ) -> CodexResult { let turn_context = Arc::clone(&step_context.turn); + #[expect(deprecated, reason = "Classify feedback using the initial turn cwd.")] + let diagnostic_cwd = &turn_context.cwd; feedback_tags!( model = step_context.settings.model_info.slug.clone(), approval_policy = turn_context.approval_policy(), - sandbox_policy = &turn_context.sandbox_policy(), + sandbox_policy = tracing::field::display( + turn_context + .permission_profile() + .legacy_sandbox_policy_label(diagnostic_cwd) + ), effort = step_context.settings.reasoning_effort(), auth_mode = sess.services.auth_manager.auth_mode(), tags_json = tracing::field::display(serde_json::json!(crate::feedback_config::usage_tags( diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 794fa420c0..ebf12d21dd 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -796,14 +796,6 @@ impl TurnContext { self.permission_profile().network_sandbox_policy() } - pub(crate) fn sandbox_policy(&self) -> SandboxPolicy { - #[allow(deprecated)] - codex_sandboxing::compatibility_sandbox_policy_for_permission_profile( - &self.permission_profile(), - &self.cwd, - ) - } - /// Legacy: returns the frozen initial-turn reasoning effort, including the initial model default. /// Step-scoped consumers should use their captured `StepContext::settings`. pub(crate) fn effective_reasoning_effort(&self) -> Option { diff --git a/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs b/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs index 2006b8681d..380248bc06 100644 --- a/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs +++ b/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs @@ -421,7 +421,7 @@ fn attach_test_trace(session: &mut Session, turn: &TurnContext, root: &Path) -> model: "gpt-test".to_string(), provider_name: "test-provider".to_string(), approval_policy: "never".to_string(), - sandbox_policy: "danger-full-access".to_string(), + permissions_profile: "danger-full-access".to_string(), }, )?; rollout_thread_trace.record_codex_turn_started(turn.sub_id.as_str()); diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 50650627e9..3409237a71 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -892,16 +892,7 @@ impl FileSystemSandboxPolicy { pub fn from_legacy_sandbox_policy_for_cwd(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self { let mut file_system_policy = Self::from(sandbox_policy); if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = sandbox_policy { - if let Ok(cwd_root) = AbsolutePathBuf::from_absolute_path(cwd) { - for protected_path in default_read_only_subpaths_for_writable_root( - &cwd_root, /*protect_missing_dot_codex*/ true, - ) { - append_default_read_only_path_if_no_explicit_rule( - &mut file_system_policy.entries, - protected_path, - ); - } - } + file_system_policy = file_system_policy.with_workspace_write_cwd(cwd); for writable_root in writable_roots { for protected_path in default_read_only_subpaths_for_writable_root( writable_root, @@ -918,6 +909,21 @@ impl FileSystemSandboxPolicy { file_system_policy } + /// Binds the default workspace metadata protections for a native working directory. + pub fn with_workspace_write_cwd(mut self, cwd: &Path) -> Self { + if let Ok(cwd_root) = AbsolutePathBuf::from_absolute_path(cwd) { + for protected_path in default_read_only_subpaths_for_writable_root( + &cwd_root, /*protect_missing_dot_codex*/ true, + ) { + append_default_read_only_path_if_no_explicit_rule( + &mut self.entries, + protected_path, + ); + } + } + self + } + /// Returns true when filesystem reads are unrestricted on this host. pub fn has_full_disk_read_access(&self) -> bool { // TODO(anp) Migrate callers to select the executor's path convention explicitly. diff --git a/codex-rs/rollout-trace/src/thread.rs b/codex-rs/rollout-trace/src/thread.rs index ae31d17d77..1ed714cb16 100644 --- a/codex-rs/rollout-trace/src/thread.rs +++ b/codex-rs/rollout-trace/src/thread.rs @@ -62,7 +62,7 @@ pub struct ThreadStartedTraceMetadata { pub model: String, pub provider_name: String, pub approval_policy: String, - pub sandbox_policy: String, + pub permissions_profile: String, } /// Trace-only payload for a child completion notification delivered to its parent. diff --git a/codex-rs/rollout-trace/src/thread_tests.rs b/codex-rs/rollout-trace/src/thread_tests.rs index c14f960139..285cfc3216 100644 --- a/codex-rs/rollout-trace/src/thread_tests.rs +++ b/codex-rs/rollout-trace/src/thread_tests.rs @@ -49,7 +49,7 @@ fn create_in_root_writes_replayable_lifecycle_events() -> anyhow::Result<()> { model: "gpt-test".to_string(), provider_name: "test-provider".to_string(), approval_policy: "never".to_string(), - sandbox_policy: "DangerFullAccess".to_string(), + permissions_profile: "danger-full-access".to_string(), }, )?; @@ -162,7 +162,7 @@ fn spawned_thread_start_appends_to_root_bundle() -> anyhow::Result<()> { model: "gpt-test".to_string(), provider_name: "test-provider".to_string(), approval_policy: "never".to_string(), - sandbox_policy: "DangerFullAccess".to_string(), + permissions_profile: "danger-full-access".to_string(), })?; child_trace.record_ended(RolloutStatus::Completed); let bundle_dir = single_bundle_dir(temp.path())?; @@ -458,7 +458,7 @@ fn minimal_metadata(thread_id: ThreadId) -> ThreadStartedTraceMetadata { model: "gpt-test".to_string(), provider_name: "test-provider".to_string(), approval_policy: "never".to_string(), - sandbox_policy: "danger-full-access".to_string(), + permissions_profile: "danger-full-access".to_string(), } } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 272a4d286c..a63291bf9b 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -162,6 +162,8 @@ use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ModelServiceTier; use codex_protocol::openai_models::ModelUpgrade; use codex_protocol::openai_models::ReasoningEffortPreset; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SubAgentSource; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; @@ -2603,9 +2605,14 @@ pub(crate) fn permission_profile_is_safely_represented_by_sandbox_mode( let projected_profile = match sandbox_mode { codex_app_server_protocol::SandboxMode::ReadOnly => PermissionProfile::read_only(), codex_app_server_protocol::SandboxMode::WorkspaceWrite => { - PermissionProfile::from_legacy_sandbox_policy_for_cwd( - &codex_protocol::protocol::SandboxPolicy::new_workspace_write_policy(), - cwd, + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::workspace_write( + &[], + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ) + .with_workspace_write_cwd(cwd), + NetworkSandboxPolicy::Restricted, ) } codex_app_server_protocol::SandboxMode::DangerFullAccess => PermissionProfile::Disabled, @@ -4240,6 +4247,32 @@ mod tests { assert_eq!(resume.runtime_workspace_roots, None); } + #[test] + fn sandbox_mode_preserves_workspace_metadata_at_the_destination() { + let workspace = tempfile::tempdir().expect("workspace"); + let cwd = workspace.path(); + std::fs::create_dir(cwd.join(".git")).expect("git metadata"); + let profile = PermissionProfile::from_legacy_sandbox_policy_for_cwd( + &codex_protocol::protocol::SandboxPolicy::new_workspace_write_policy(), + cwd, + ); + assert!(permission_profile_is_safely_represented_by_sandbox_mode( + &profile, cwd + )); + let policy = profile + .file_system_sandbox_policy() + .with_additional_writable_roots( + cwd, + &[AbsolutePathBuf::from_absolute_path(cwd.join(".git")) + .expect("absolute metadata")], + ); + let profile = + PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted); + assert!(!permission_profile_is_safely_represented_by_sandbox_mode( + &profile, cwd + )); + } + #[test] fn sandbox_mode_does_not_project_non_cwd_write_roots_for_remote_sessions() { let cwd = test_path_buf("/workspace/project").abs();