From c622cbef3506fd2a6f29f896384d635f7cd3db37 Mon Sep 17 00:00:00 2001 From: celia-oai Date: Tue, 17 Feb 2026 12:01:16 -0800 Subject: [PATCH] changes --- codex-rs/core/src/skills/command_policy.rs | 379 ++++++++++++++++++ codex-rs/core/src/skills/mod.rs | 2 + codex-rs/core/src/skills/permissions.rs | 231 +++++++++++ codex-rs/core/src/tools/handlers/shell.rs | 30 +- codex-rs/core/src/tools/orchestrator.rs | 40 +- .../core/src/unified_exec/process_manager.rs | 21 +- 6 files changed, 689 insertions(+), 14 deletions(-) create mode 100644 codex-rs/core/src/skills/command_policy.rs diff --git a/codex-rs/core/src/skills/command_policy.rs b/codex-rs/core/src/skills/command_policy.rs new file mode 100644 index 0000000000..7a70868aa7 --- /dev/null +++ b/codex-rs/core/src/skills/command_policy.rs @@ -0,0 +1,379 @@ +use std::collections::HashSet; +use std::path::Component; +use std::path::Path; +use std::path::PathBuf; + +use codex_protocol::protocol::SandboxPolicy; +use dirs::home_dir; +use dunce::canonicalize as canonicalize_path; + +use crate::bash::parse_shell_lc_plain_commands; +use crate::bash::parse_shell_lc_single_command_prefix; +use crate::config::Permissions; +use crate::path_utils::normalize_for_path_comparison; +use crate::skills::SkillLoadOutcome; +use crate::skills::SkillMetadata; + +pub(crate) fn resolve_skill_sandbox_extension_for_command( + skills_outcome: &SkillLoadOutcome, + command: &[String], + command_cwd: &Path, +) -> Option { + let segments = command_segments_for_matching(command); + let mut best_match: Option<(usize, SandboxPolicy)> = None; + + for segment in segments { + for candidate in candidate_paths_for_segment(&segment, command_cwd) { + let Some((depth, _, permissions)) = + match_skill_for_candidate(skills_outcome, candidate.as_path()) + else { + continue; + }; + + let should_replace = match &best_match { + Some((best_depth, _)) => depth > *best_depth, + None => true, + }; + if should_replace { + best_match = Some((depth, permissions.sandbox_policy.get().clone())); + } + } + } + + best_match.map(|(_, sandbox_policy)| sandbox_policy) +} + +fn command_segments_for_matching(command: &[String]) -> Vec> { + if let Some(commands) = parse_shell_lc_plain_commands(command) + && !commands.is_empty() + { + return commands; + } + + if let Some(command) = parse_shell_lc_single_command_prefix(command) { + return vec![command]; + } + + vec![command.to_vec()] +} + +fn candidate_paths_for_segment(segment: &[String], command_cwd: &Path) -> Vec { + let mut candidates = Vec::new(); + let mut seen = HashSet::new(); + + if let Some(path) = normalize_candidate_path(command_cwd) + && seen.insert(path.clone()) + { + candidates.push(path); + } + + for token in segment { + let Some(path) = candidate_path_from_token(token, command_cwd) else { + continue; + }; + if seen.insert(path.clone()) { + candidates.push(path); + } + } + + candidates +} + +fn candidate_path_from_token(token: &str, command_cwd: &Path) -> Option { + if token.is_empty() || token.contains("://") || token.starts_with('-') { + return None; + } + + let is_path_like = token == "~" + || token.starts_with("~/") + || token.starts_with("./") + || token.starts_with("../") + || token.contains('/') + || token.contains('\\') + || Path::new(token).is_absolute() + || command_cwd.join(token).exists(); + if !is_path_like { + return None; + } + + let expanded = expand_home(token); + let path = PathBuf::from(expanded); + let absolute = if path.is_absolute() { + path + } else { + command_cwd.join(path) + }; + normalize_candidate_path(&absolute) +} + +fn normalize_candidate_path(path: &Path) -> Option { + let normalized = normalize_lexically(path); + let canonicalized = canonicalize_path(&normalized).unwrap_or(normalized); + let comparison_path = + normalize_for_path_comparison(canonicalized.as_path()).unwrap_or(canonicalized); + if comparison_path.is_absolute() { + Some(comparison_path) + } else { + None + } +} + +fn match_skill_for_candidate<'a>( + skills_outcome: &'a SkillLoadOutcome, + candidate: &Path, +) -> Option<(usize, &'a SkillMetadata, &'a Permissions)> { + let mut best_match: Option<(usize, &SkillMetadata, &Permissions)> = None; + + for skill in &skills_outcome.skills { + if skills_outcome.disabled_paths.contains(&skill.path) { + continue; + } + let Some(permissions) = skill.permissions.as_ref() else { + continue; + }; + let Some(skill_dir) = skill.path.parent() else { + continue; + }; + let Some(skill_dir) = normalize_candidate_path(skill_dir) else { + continue; + }; + if !candidate.starts_with(&skill_dir) { + continue; + } + + let depth = skill_dir.components().count(); + let should_replace = match &best_match { + Some((best_depth, _, _)) => depth > *best_depth, + None => true, + }; + if should_replace { + best_match = Some((depth, skill, permissions)); + } + } + + best_match +} + +fn expand_home(path: &str) -> String { + if path == "~" { + if let Some(home) = home_dir() { + return home.to_string_lossy().to_string(); + } + return path.to_string(); + } + if let Some(rest) = path.strip_prefix("~/") + && let Some(home) = home_dir() + { + return home.join(rest).to_string_lossy().to_string(); + } + path.to_string() +} + +fn normalize_lexically(path: &Path) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + normalized.pop(); + } + Component::RootDir | Component::Prefix(_) | Component::Normal(_) => { + normalized.push(component.as_os_str()); + } + } + } + normalized +} + +#[cfg(test)] +mod tests { + use super::resolve_skill_sandbox_extension_for_command; + use crate::config::Constrained; + use crate::config::Permissions; + use crate::config::types::ShellEnvironmentPolicy; + use crate::protocol::AskForApproval; + use crate::protocol::ReadOnlyAccess; + use crate::protocol::SandboxPolicy; + use crate::skills::SkillLoadOutcome; + use crate::skills::model::SkillMetadata; + use codex_protocol::protocol::SkillScope; + use codex_utils_absolute_path::AbsolutePathBuf; + use pretty_assertions::assert_eq; + use std::collections::HashSet; + use std::path::Path; + use std::path::PathBuf; + + fn skill_with_policy(skill_path: PathBuf, sandbox_policy: SandboxPolicy) -> SkillMetadata { + SkillMetadata { + name: "skill".to_string(), + description: "skill".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + permissions: Some(Permissions { + approval_policy: Constrained::allow_any(AskForApproval::Never), + sandbox_policy: Constrained::allow_any(sandbox_policy), + network: None, + shell_environment_policy: ShellEnvironmentPolicy::default(), + windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: None, + }), + path: skill_path, + scope: SkillScope::User, + } + } + + fn outcome_with_skills(skills: Vec) -> SkillLoadOutcome { + SkillLoadOutcome { + skills, + errors: Vec::new(), + disabled_paths: HashSet::new(), + } + } + + fn canonical(path: &Path) -> PathBuf { + dunce::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()) + } + + #[test] + fn resolves_skill_policy_for_executable_inside_skill_directory() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let skill_dir = tempdir.path().join("skills/demo"); + let scripts_dir = skill_dir.join("scripts"); + std::fs::create_dir_all(&scripts_dir).expect("create scripts"); + let executable = scripts_dir.join("run.sh"); + std::fs::write(&executable, "#!/bin/sh\necho ok\n").expect("write script"); + let skill_path = skill_dir.join("SKILL.md"); + std::fs::write(&skill_path, "skill").expect("write SKILL.md"); + + let write_root = AbsolutePathBuf::try_from(skill_dir.join("output")).expect("absolute"); + let skill_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![write_root], + read_only_access: ReadOnlyAccess::FullAccess, + network_access: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + }; + let outcome = outcome_with_skills(vec![skill_with_policy( + canonical(&skill_path), + skill_policy.clone(), + )]); + + let resolved = resolve_skill_sandbox_extension_for_command( + &outcome, + &[canonical(&executable).to_string_lossy().to_string()], + tempdir.path(), + ); + + assert_eq!(resolved, Some(skill_policy)); + } + + #[test] + fn resolves_skill_policy_for_shell_wrapped_relative_script_command() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let skill_dir = tempdir.path().join("skills/demo"); + let scripts_dir = skill_dir.join("scripts"); + std::fs::create_dir_all(&scripts_dir).expect("create scripts"); + std::fs::write(scripts_dir.join("run.sh"), "#!/bin/sh\necho ok\n").expect("write script"); + let skill_path = skill_dir.join("SKILL.md"); + std::fs::write(&skill_path, "skill").expect("write SKILL.md"); + + let skill_policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: vec![ + AbsolutePathBuf::try_from(skill_dir.join("data")).expect("absolute"), + ], + }, + }; + let outcome = outcome_with_skills(vec![skill_with_policy( + canonical(&skill_path), + skill_policy.clone(), + )]); + + let resolved = resolve_skill_sandbox_extension_for_command( + &outcome, + &[ + "bash".to_string(), + "-lc".to_string(), + "./scripts/run.sh --flag".to_string(), + ], + &skill_dir, + ); + + assert_eq!(resolved, Some(skill_policy)); + } + + #[test] + fn ignores_disabled_skill_when_resolving_command_policy() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let skill_dir = tempdir.path().join("skills/demo"); + std::fs::create_dir_all(&skill_dir).expect("create skill dir"); + let executable = skill_dir.join("tool.sh"); + std::fs::write(&executable, "#!/bin/sh\necho ok\n").expect("write script"); + let skill_path = skill_dir.join("SKILL.md"); + std::fs::write(&skill_path, "skill").expect("write SKILL.md"); + let skill_path = canonical(&skill_path); + + let mut outcome = outcome_with_skills(vec![skill_with_policy( + skill_path.clone(), + SandboxPolicy::new_workspace_write_policy(), + )]); + outcome.disabled_paths.insert(skill_path); + + let resolved = resolve_skill_sandbox_extension_for_command( + &outcome, + &[canonical(&executable).to_string_lossy().to_string()], + tempdir.path(), + ); + + assert_eq!(resolved, None); + } + + #[test] + fn prefers_most_specific_skill_directory_for_nested_match() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let parent_skill_dir = tempdir.path().join("skills/parent"); + let nested_skill_dir = parent_skill_dir.join("nested"); + std::fs::create_dir_all(nested_skill_dir.join("scripts")).expect("create scripts"); + + let executable = nested_skill_dir.join("scripts/run.sh"); + std::fs::write(&executable, "#!/bin/sh\necho ok\n").expect("write script"); + + let parent_skill_path = parent_skill_dir.join("SKILL.md"); + let nested_skill_path = nested_skill_dir.join("SKILL.md"); + std::fs::write(&parent_skill_path, "parent").expect("write parent skill"); + std::fs::write(&nested_skill_path, "nested").expect("write nested skill"); + + let parent_policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![ + AbsolutePathBuf::try_from(parent_skill_dir.join("data")).expect("absolute"), + ], + }, + }; + let nested_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![ + AbsolutePathBuf::try_from(nested_skill_dir.join("output")).expect("absolute"), + ], + read_only_access: ReadOnlyAccess::FullAccess, + network_access: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + }; + let outcome = outcome_with_skills(vec![ + skill_with_policy(canonical(&parent_skill_path), parent_policy), + skill_with_policy(canonical(&nested_skill_path), nested_policy.clone()), + ]); + + let resolved = resolve_skill_sandbox_extension_for_command( + &outcome, + &[canonical(&executable).to_string_lossy().to_string()], + tempdir.path(), + ); + + assert_eq!(resolved, Some(nested_policy)); + } +} diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core/src/skills/mod.rs index 18f7180f99..f9d07b7edf 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core/src/skills/mod.rs @@ -1,3 +1,4 @@ +mod command_policy; mod env_var_dependencies; pub mod injection; pub mod loader; @@ -8,6 +9,7 @@ pub mod remote; pub mod render; pub mod system; +pub(crate) use command_policy::resolve_skill_sandbox_extension_for_command; pub(crate) use env_var_dependencies::collect_env_var_dependencies; pub(crate) use env_var_dependencies::resolve_skill_dependencies_for_turn; pub(crate) use injection::SkillInjections; diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs index 1cf18e1052..44b862a792 100644 --- a/codex-rs/core/src/skills/permissions.rs +++ b/codex-rs/core/src/skills/permissions.rs @@ -13,6 +13,7 @@ use crate::config::Constrained; use crate::config::Permissions; use crate::config::types::ShellEnvironmentPolicy; use crate::protocol::AskForApproval; +use crate::protocol::NetworkAccess; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; #[cfg(target_os = "macos")] @@ -118,6 +119,150 @@ pub(crate) fn compile_permission_profile( }) } +pub(crate) fn extend_sandbox_policy( + base: &SandboxPolicy, + extension: &SandboxPolicy, +) -> SandboxPolicy { + match (base, extension) { + (SandboxPolicy::DangerFullAccess, _) | (_, SandboxPolicy::DangerFullAccess) => { + SandboxPolicy::DangerFullAccess + } + ( + SandboxPolicy::ExternalSandbox { + network_access: base_network, + }, + SandboxPolicy::ExternalSandbox { + network_access: extension_network, + }, + ) => SandboxPolicy::ExternalSandbox { + network_access: if base_network.is_enabled() || extension_network.is_enabled() { + NetworkAccess::Enabled + } else { + NetworkAccess::Restricted + }, + }, + (SandboxPolicy::ExternalSandbox { .. }, _) | (_, SandboxPolicy::ExternalSandbox { .. }) => { + SandboxPolicy::ExternalSandbox { + network_access: if base.has_full_network_access() + || extension.has_full_network_access() + { + NetworkAccess::Enabled + } else { + NetworkAccess::Restricted + }, + } + } + ( + SandboxPolicy::ReadOnly { + access: base_access, + }, + SandboxPolicy::ReadOnly { + access: extension_access, + }, + ) => SandboxPolicy::ReadOnly { + access: extend_read_only_access(base_access, extension_access), + }, + ( + SandboxPolicy::ReadOnly { + access: base_access, + }, + SandboxPolicy::WorkspaceWrite { + writable_roots, + read_only_access, + network_access, + exclude_tmpdir_env_var, + exclude_slash_tmp, + }, + ) => SandboxPolicy::WorkspaceWrite { + writable_roots: writable_roots.clone(), + read_only_access: extend_read_only_access(base_access, read_only_access), + network_access: *network_access, + exclude_tmpdir_env_var: *exclude_tmpdir_env_var, + exclude_slash_tmp: *exclude_slash_tmp, + }, + ( + SandboxPolicy::WorkspaceWrite { + writable_roots, + read_only_access, + network_access, + exclude_tmpdir_env_var, + exclude_slash_tmp, + }, + SandboxPolicy::ReadOnly { + access: extension_access, + }, + ) => SandboxPolicy::WorkspaceWrite { + writable_roots: writable_roots.clone(), + read_only_access: extend_read_only_access(read_only_access, extension_access), + network_access: *network_access, + exclude_tmpdir_env_var: *exclude_tmpdir_env_var, + exclude_slash_tmp: *exclude_slash_tmp, + }, + ( + SandboxPolicy::WorkspaceWrite { + writable_roots: base_writable_roots, + read_only_access: base_read_only_access, + network_access: base_network_access, + exclude_tmpdir_env_var: base_exclude_tmpdir_env_var, + exclude_slash_tmp: base_exclude_slash_tmp, + }, + SandboxPolicy::WorkspaceWrite { + writable_roots: extension_writable_roots, + read_only_access: extension_read_only_access, + network_access: extension_network_access, + exclude_tmpdir_env_var: extension_exclude_tmpdir_env_var, + exclude_slash_tmp: extension_exclude_slash_tmp, + }, + ) => SandboxPolicy::WorkspaceWrite { + writable_roots: extend_absolute_roots(base_writable_roots, extension_writable_roots), + read_only_access: extend_read_only_access( + base_read_only_access, + extension_read_only_access, + ), + network_access: *base_network_access || *extension_network_access, + exclude_tmpdir_env_var: *base_exclude_tmpdir_env_var + && *extension_exclude_tmpdir_env_var, + exclude_slash_tmp: *base_exclude_slash_tmp && *extension_exclude_slash_tmp, + }, + } +} + +fn extend_read_only_access(base: &ReadOnlyAccess, extension: &ReadOnlyAccess) -> ReadOnlyAccess { + match (base, extension) { + (ReadOnlyAccess::FullAccess, _) | (_, ReadOnlyAccess::FullAccess) => { + ReadOnlyAccess::FullAccess + } + ( + ReadOnlyAccess::Restricted { + include_platform_defaults: base_include_platform_defaults, + readable_roots: base_readable_roots, + }, + ReadOnlyAccess::Restricted { + include_platform_defaults: extension_include_platform_defaults, + readable_roots: extension_readable_roots, + }, + ) => ReadOnlyAccess::Restricted { + include_platform_defaults: *base_include_platform_defaults + || *extension_include_platform_defaults, + readable_roots: extend_absolute_roots(base_readable_roots, extension_readable_roots), + }, + } +} + +fn extend_absolute_roots( + base_roots: &[AbsolutePathBuf], + extension_roots: &[AbsolutePathBuf], +) -> Vec { + let mut roots = Vec::new(); + let mut seen = HashSet::new(); + for root in base_roots.iter().chain(extension_roots) { + if seen.insert(root.to_path_buf()) { + roots.push(root.clone()); + } + } + roots +} + fn normalize_permission_paths( skill_dir: &Path, values: &[String], @@ -289,6 +434,7 @@ mod tests { use super::SkillManifestMacOsPermissions; use super::SkillManifestPermissions; use super::compile_permission_profile; + use super::extend_sandbox_policy; use crate::config::Constrained; use crate::config::Permissions; use crate::config::types::ShellEnvironmentPolicy; @@ -451,6 +597,91 @@ mod tests { ); } + #[test] + fn extend_sandbox_policy_merges_read_only_into_workspace_write() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let base_read_root = + AbsolutePathBuf::try_from(tempdir.path().join("base-read")).expect("absolute path"); + let extension_write_root = + AbsolutePathBuf::try_from(tempdir.path().join("extension-write")) + .expect("absolute path"); + + let merged = extend_sandbox_policy( + &SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![base_read_root.clone()], + }, + }, + &SandboxPolicy::WorkspaceWrite { + writable_roots: vec![extension_write_root.clone()], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: Vec::new(), + }, + network_access: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + }, + ); + + assert_eq!( + merged, + SandboxPolicy::WorkspaceWrite { + writable_roots: vec![extension_write_root], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: vec![base_read_root], + }, + network_access: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + } + ); + } + + #[test] + fn extend_sandbox_policy_unions_workspace_roots_and_network_access() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let shared_root = + AbsolutePathBuf::try_from(tempdir.path().join("shared")).expect("absolute path"); + let base_root = + AbsolutePathBuf::try_from(tempdir.path().join("base")).expect("absolute path"); + let extension_root = + AbsolutePathBuf::try_from(tempdir.path().join("extension")).expect("absolute path"); + + let merged = extend_sandbox_policy( + &SandboxPolicy::WorkspaceWrite { + writable_roots: vec![shared_root.clone(), base_root.clone()], + read_only_access: ReadOnlyAccess::FullAccess, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }, + &SandboxPolicy::WorkspaceWrite { + writable_roots: vec![shared_root.clone(), extension_root.clone()], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + }, + ); + + assert_eq!( + merged, + SandboxPolicy::WorkspaceWrite { + writable_roots: vec![shared_root, base_root, extension_root], + read_only_access: ReadOnlyAccess::FullAccess, + network_access: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + } + ); + } + #[cfg(target_os = "macos")] #[test] fn compile_permission_profile_builds_macos_permission_file() { diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 31bf3f1294..62cc83abe6 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -13,6 +13,8 @@ use crate::function_tool::FunctionCallError; use crate::is_safe_command::is_known_safe_command; use crate::protocol::ExecCommandSource; use crate::shell::Shell; +use crate::skills::permissions::extend_sandbox_policy; +use crate::skills::resolve_skill_sandbox_extension_for_command; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -297,13 +299,30 @@ impl ShellHandler { let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None); emitter.begin(event_ctx).await; + let skills_outcome = session + .services + .skills_manager + .skills_for_cwd(&turn.cwd, false) + .await; + let effective_sandbox_policy = resolve_skill_sandbox_extension_for_command( + &skills_outcome, + &exec_params.command, + &exec_params.cwd, + ) + .map_or_else( + || turn.sandbox_policy.clone(), + |skill_sandbox_policy| { + extend_sandbox_policy(&turn.sandbox_policy, &skill_sandbox_policy) + }, + ); + let exec_approval_requirement = session .services .exec_policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &exec_params.command, approval_policy: turn.approval_policy, - sandbox_policy: &turn.sandbox_policy, + sandbox_policy: &effective_sandbox_policy, sandbox_permissions: exec_params.sandbox_permissions, prefix_rule, }) @@ -330,7 +349,14 @@ impl ShellHandler { network_attempt_id: None, }; let out = orchestrator - .run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy) + .run_with_sandbox_policy( + &mut runtime, + &req, + &tool_ctx, + &turn, + turn.approval_policy, + &effective_sandbox_policy, + ) .await .map(|result| result.output); let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None); diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index cdbaac0254..3ebf40a3ec 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -107,6 +107,29 @@ impl ToolOrchestrator { turn_ctx: &crate::codex::TurnContext, approval_policy: AskForApproval, ) -> Result, ToolError> + where + T: ToolRuntime, + { + self.run_with_sandbox_policy( + tool, + req, + tool_ctx, + turn_ctx, + approval_policy, + &turn_ctx.sandbox_policy, + ) + .await + } + + pub async fn run_with_sandbox_policy( + &mut self, + tool: &mut T, + req: &Rq, + tool_ctx: &ToolCtx<'_>, + turn_ctx: &crate::codex::TurnContext, + approval_policy: AskForApproval, + sandbox_policy: &crate::protocol::SandboxPolicy, + ) -> Result, ToolError> where T: ToolRuntime, { @@ -119,9 +142,9 @@ impl ToolOrchestrator { // 1) Approval let mut already_approved = false; - let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| { - default_exec_approval_requirement(approval_policy, &turn_ctx.sandbox_policy) - }); + let requirement = tool + .exec_approval_requirement(req) + .unwrap_or_else(|| default_exec_approval_requirement(approval_policy, sandbox_policy)); match requirement { ExecApprovalRequirement::Skip { .. } => { otel.tool_decision(otel_tn, otel_ci, &ReviewDecision::Approved, otel_cfg); @@ -163,7 +186,7 @@ impl ToolOrchestrator { let initial_sandbox = match tool.sandbox_mode_for_first_attempt(req) { SandboxOverride::BypassSandboxFirstAttempt => crate::exec::SandboxType::None, SandboxOverride::NoOverride => self.sandbox.select_initial( - &turn_ctx.sandbox_policy, + sandbox_policy, tool.sandbox_preference(), turn_ctx.windows_sandbox_level, has_managed_network_requirements, @@ -175,7 +198,7 @@ impl ToolOrchestrator { let use_linux_sandbox_bwrap = turn_ctx.features.enabled(Feature::UseLinuxSandboxBwrap); let initial_attempt = SandboxAttempt { sandbox: initial_sandbox, - policy: &turn_ctx.sandbox_policy, + policy: sandbox_policy, enforce_managed_network: has_managed_network_requirements, manager: &self.sandbox, sandbox_cwd: &turn_ctx.cwd, @@ -230,10 +253,7 @@ impl ToolOrchestrator { matches!(approval_policy, AskForApproval::OnRequest) && network_approval_context.is_some() && matches!( - default_exec_approval_requirement( - approval_policy, - &turn_ctx.sandbox_policy - ), + default_exec_approval_requirement(approval_policy, sandbox_policy), ExecApprovalRequirement::NeedsApproval { .. } ); if !allow_on_request_network_prompt { @@ -281,7 +301,7 @@ impl ToolOrchestrator { let escalated_attempt = SandboxAttempt { sandbox: crate::exec::SandboxType::None, - policy: &turn_ctx.sandbox_policy, + policy: sandbox_policy, enforce_managed_network: has_managed_network_requirements, manager: &self.sandbox, sandbox_cwd: &turn_ctx.cwd, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 6896df14b4..8b96495aa2 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -16,6 +16,8 @@ use crate::exec_env::create_env; use crate::exec_policy::ExecApprovalRequest; use crate::protocol::ExecCommandSource; use crate::sandboxing::ExecRequest; +use crate::skills::permissions::extend_sandbox_policy; +use crate::skills::resolve_skill_sandbox_extension_for_command; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::events::ToolEventStage; @@ -606,6 +608,20 @@ impl UnifiedExecProcessManager { )); let mut orchestrator = ToolOrchestrator::new(); let mut runtime = UnifiedExecRuntime::new(self); + let skills_outcome = context + .session + .services + .skills_manager + .skills_for_cwd(&context.turn.cwd, false) + .await; + let effective_sandbox_policy = + resolve_skill_sandbox_extension_for_command(&skills_outcome, &request.command, &cwd) + .map_or_else( + || context.turn.sandbox_policy.clone(), + |skill_sandbox_policy| { + extend_sandbox_policy(&context.turn.sandbox_policy, &skill_sandbox_policy) + }, + ); let exec_approval_requirement = context .session .services @@ -613,7 +629,7 @@ impl UnifiedExecProcessManager { .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &request.command, approval_policy: context.turn.approval_policy, - sandbox_policy: &context.turn.sandbox_policy, + sandbox_policy: &effective_sandbox_policy, sandbox_permissions: request.sandbox_permissions, prefix_rule: request.prefix_rule.clone(), }) @@ -637,12 +653,13 @@ impl UnifiedExecProcessManager { network_attempt_id: None, }; orchestrator - .run( + .run_with_sandbox_policy( &mut runtime, &req, &tool_ctx, context.turn.as_ref(), context.turn.approval_policy, + &effective_sandbox_policy, ) .await .map(|result| (result.output, result.deferred_network_approval))