diff --git a/codex-rs/core/src/skills/invocation_utils.rs b/codex-rs/core/src/skills/invocation_utils.rs index de386bb92f..4c83462cf0 100644 --- a/codex-rs/core/src/skills/invocation_utils.rs +++ b/codex-rs/core/src/skills/invocation_utils.rs @@ -273,7 +273,10 @@ fn detect_skill_script_run( detect_skill_for_path_under_scripts(outcome, script_path.as_path()) } -fn detect_skill_for_path_under_scripts(outcome: &SkillLoadOutcome, path: &Path) -> Option { +fn detect_skill_for_path_under_scripts( + outcome: &SkillLoadOutcome, + path: &Path, +) -> Option { for ancestor in path.ancestors() { if let Some(candidate) = outcome.implicit_skills_by_scripts_dir.get(ancestor) { return Some(candidate.clone()); diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 1cbba1d568..3356550f96 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -310,10 +310,8 @@ impl ShellHandler { shell: &Shell, command: &[String], shell_runtime_backend: ShellRuntimeBackend, - skill_shell_command_enabled: bool, ) -> Option { - if !skill_shell_command_enabled - || shell_runtime_backend != ShellRuntimeBackend::ShellCommandClassic + if shell_runtime_backend != ShellRuntimeBackend::ShellCommandClassic || shell.shell_type != ShellType::Zsh { return None; @@ -428,37 +426,54 @@ impl ShellHandler { let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None); emitter.begin(event_ctx).await; - let user_shell = session.user_shell(); - let matched_skill = Self::detect_skill_shell_command( - turn.turn_skills.outcome.as_ref(), - user_shell.as_ref(), - &exec_params.command, - shell_runtime_backend, - turn.features.enabled(Feature::SkillShellCommandSandbox), - ); - let effective_sandbox_policy = matched_skill - .as_ref() - .and_then(|skill| skill.permissions.as_ref()) - .map(|permissions| permissions.sandbox_policy.get().clone()) - .unwrap_or_else(|| turn.sandbox_policy.get().clone()); - let exec_approval_requirement = if matched_skill.is_some() { - ExecApprovalRequirement::NeedsApproval { - reason: Some(SKILL_SHELL_COMMAND_APPROVAL_REASON.to_string()), - proposed_execpolicy_amendment: None, - } - } else { - session - .services - .exec_policy - .create_exec_approval_requirement_for_command(ExecApprovalRequest { - command: &exec_params.command, - approval_policy: turn.approval_policy.value(), - sandbox_policy: &effective_sandbox_policy, - sandbox_permissions: exec_params.sandbox_permissions, - prefix_rule, - }) - .await - }; + let (effective_sandbox_policy, exec_approval_requirement) = + if turn.features.enabled(Feature::SkillShellCommandSandbox) { + let user_shell = session.user_shell(); + let matched_skill = Self::detect_skill_shell_command( + turn.turn_skills.outcome.as_ref(), + user_shell.as_ref(), + &exec_params.command, + shell_runtime_backend, + ); + let effective_sandbox_policy = matched_skill + .as_ref() + .and_then(|skill| skill.permissions.as_ref()) + .map(|permissions| permissions.sandbox_policy.get().clone()) + .unwrap_or_else(|| turn.sandbox_policy.get().clone()); + let exec_approval_requirement = if matched_skill.is_some() { + ExecApprovalRequirement::NeedsApproval { + reason: Some(SKILL_SHELL_COMMAND_APPROVAL_REASON.to_string()), + proposed_execpolicy_amendment: None, + } + } else { + session + .services + .exec_policy + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &exec_params.command, + approval_policy: turn.approval_policy.value(), + sandbox_policy: &effective_sandbox_policy, + sandbox_permissions: exec_params.sandbox_permissions, + prefix_rule, + }) + .await + }; + (effective_sandbox_policy, exec_approval_requirement) + } else { + let effective_sandbox_policy = turn.sandbox_policy.get().clone(); + let exec_approval_requirement = session + .services + .exec_policy + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &exec_params.command, + approval_policy: turn.approval_policy.value(), + sandbox_policy: &effective_sandbox_policy, + sandbox_permissions: exec_params.sandbox_permissions, + prefix_rule, + }) + .await; + (effective_sandbox_policy, exec_approval_requirement) + }; let req = ShellRequest { command: exec_params.command.clone(), @@ -519,8 +534,6 @@ mod tests { use pretty_assertions::assert_eq; use crate::codex::make_session_and_context; - use crate::skills::SkillLoadOutcome; - use crate::skills::SkillMetadata; use crate::exec_env::create_env; use crate::is_safe_command::is_known_safe_command; use crate::powershell::try_find_powershell_executable_blocking; @@ -529,6 +542,8 @@ mod tests { use crate::shell::Shell; use crate::shell::ShellType; use crate::shell_snapshot::ShellSnapshot; + use crate::skills::SkillLoadOutcome; + use crate::skills::SkillMetadata; use crate::tools::handlers::ShellCommandHandler; use crate::tools::handlers::ShellHandler; use crate::tools::runtimes::shell::ShellRuntimeBackend; @@ -733,10 +748,12 @@ mod tests { &shell, &command, ShellRuntimeBackend::ShellCommandClassic, - true, ); - assert_eq!(found.map(|skill| skill.name), Some("test-skill".to_string())); + assert_eq!( + found.map(|skill| skill.name), + Some("test-skill".to_string()) + ); } #[test] @@ -754,17 +771,13 @@ mod tests { )])), ..Default::default() }; - let command = shell.derive_exec_args( - "/tmp/skill-test/scripts/run-tool && echo done", - true, - ); + let command = shell.derive_exec_args("/tmp/skill-test/scripts/run-tool && echo done", true); let found = ShellHandler::detect_skill_shell_command( &outcome, &shell, &command, ShellRuntimeBackend::ShellCommandClassic, - true, ); assert_eq!(found, None); @@ -792,7 +805,6 @@ mod tests { &shell, &command, ShellRuntimeBackend::ShellCommandZshFork, - true, ); assert_eq!(found, None); diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 2841e914bf..6ea1e43591 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -11,10 +11,10 @@ use crate::command_canonicalization::canonicalize_command_for_approval; use crate::exec::ExecToolCallOutput; use crate::features::Feature; use crate::powershell::prefix_powershell_script_with_utf8; +use crate::protocol::SandboxPolicy; use crate::sandboxing::SandboxPermissions; use crate::sandboxing::execute_env; use crate::shell::ShellType; -use crate::protocol::SandboxPolicy; use crate::tools::network_approval::NetworkApprovalMode; use crate::tools::network_approval::NetworkApprovalSpec; use crate::tools::runtimes::build_command_spec; diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index f779e1ff89..aa6b7bfc0d 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -303,11 +303,7 @@ pub(crate) enum ToolError { } pub(crate) trait ToolRuntime: Approvable + Sandboxable { - fn sandbox_policy<'a>( - &self, - _req: &'a Req, - turn_ctx: &'a TurnContext, - ) -> &'a SandboxPolicy { + fn sandbox_policy<'a>(&self, _req: &'a Req, turn_ctx: &'a TurnContext) -> &'a SandboxPolicy { turn_ctx.sandbox_policy.get() }