From 514566f4c3099720294bb7736d867a18ecbc62e3 Mon Sep 17 00:00:00 2001 From: celia-oai Date: Tue, 17 Feb 2026 16:25:47 -0800 Subject: [PATCH] changes --- codex-rs/core/src/skills/command_policy.rs | 93 +++++++++++++++++-- codex-rs/core/src/tools/handlers/shell.rs | 21 +++-- .../core/src/unified_exec/process_manager.rs | 15 +-- 3 files changed, 105 insertions(+), 24 deletions(-) diff --git a/codex-rs/core/src/skills/command_policy.rs b/codex-rs/core/src/skills/command_policy.rs index abf1b548f7..c27016dd66 100644 --- a/codex-rs/core/src/skills/command_policy.rs +++ b/codex-rs/core/src/skills/command_policy.rs @@ -2,6 +2,7 @@ use std::path::Component; use std::path::Path; use std::path::PathBuf; +use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::SandboxPolicy; use dunce::canonicalize as canonicalize_path; @@ -16,17 +17,51 @@ use crate::skills::SkillLoadOutcome; /// 1. `command_cwd` reflects the effective command target location. /// 2. If `command_cwd` is contained by multiple skill directories, the first /// enabled skill in `skills_outcome.skills` wins. -/// 3. Command tokens are not used for matching. +/// 3. If `command_cwd` does not match, each command action path is checked. /// /// Returns `None` when no enabled skill with permissions matches -/// `command_cwd`. +/// `command_cwd` or command action paths. pub(crate) fn resolve_skill_sandbox_extension_for_command( skills_outcome: &SkillLoadOutcome, command_cwd: &Path, + command_actions: &[ParsedCommand], ) -> Option { - let candidate = normalize_candidate_path(command_cwd)?; - let permissions = match_skill_for_candidate(skills_outcome, candidate.as_path())?; - Some(permissions.sandbox_policy.get().clone()) + let command_cwd = normalize_candidate_path(command_cwd)?; + if let Some(permissions) = match_skill_for_candidate(skills_outcome, command_cwd.as_path()) { + return Some(permissions.sandbox_policy.get().clone()); + } + + for command_action in command_actions { + let Some(action_candidate) = + command_action_candidate_path(command_action, command_cwd.as_path()) + else { + continue; + }; + if let Some(permissions) = match_skill_for_candidate(skills_outcome, &action_candidate) { + return Some(permissions.sandbox_policy.get().clone()); + } + } + + None +} + +fn command_action_candidate_path( + command_action: &ParsedCommand, + command_cwd: &Path, +) -> Option { + let action_path = match command_action { + ParsedCommand::Read { path, .. } => Some(path.as_path()), + ParsedCommand::ListFiles { path, .. } | ParsedCommand::Search { path, .. } => { + path.as_deref().map(Path::new) + } + ParsedCommand::Unknown { .. } => None, + }?; + let action_path = if action_path.is_absolute() { + action_path.to_path_buf() + } else { + command_cwd.join(action_path) + }; + normalize_candidate_path(action_path.as_path()) } fn normalize_candidate_path(path: &Path) -> Option { @@ -99,6 +134,7 @@ mod tests { use crate::protocol::SandboxPolicy; use crate::skills::SkillLoadOutcome; use crate::skills::model::SkillMetadata; + use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; @@ -161,13 +197,13 @@ mod tests { skill_policy.clone(), )]); - let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &scripts_dir); + let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &scripts_dir, &[]); assert_eq!(resolved, Some(skill_policy)); } #[test] - fn does_not_resolve_policy_when_only_command_path_matches() { + fn does_not_resolve_policy_when_neither_cwd_nor_command_action_paths_match() { let tempdir = tempfile::tempdir().expect("tempdir"); let skill_dir = tempdir.path().join("skills/demo"); let outside_dir = tempdir.path().join("outside"); @@ -191,11 +227,47 @@ mod tests { skill_policy.clone(), )]); - let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &outside_dir); + let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &outside_dir, &[]); assert_eq!(resolved, None); } + #[test] + fn resolves_policy_when_command_action_path_is_inside_skill_directory() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let skill_dir = tempdir.path().join("skills/demo"); + let outside_dir = tempdir.path().join("outside"); + std::fs::create_dir_all(&outside_dir).expect("create outside"); + let scripts_dir = skill_dir.join("scripts"); + std::fs::create_dir_all(&scripts_dir).expect("create scripts"); + 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 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( + skill_path.clone(), + skill_policy.clone(), + )]); + + let command_actions = vec![ParsedCommand::Read { + cmd: format!("cat {}", skill_path.display()), + name: "SKILL.md".to_string(), + path: skill_path, + }]; + let resolved = + resolve_skill_sandbox_extension_for_command(&outcome, &outside_dir, &command_actions); + + assert_eq!(resolved, Some(skill_policy)); + } + #[test] fn ignores_disabled_skill_when_resolving_command_policy() { let tempdir = tempfile::tempdir().expect("tempdir"); @@ -212,7 +284,7 @@ mod tests { )]); outcome.disabled_paths.insert(skill_path); - let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &skill_dir); + let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &skill_dir, &[]); assert_eq!(resolved, None); } @@ -257,7 +329,8 @@ mod tests { skill_with_policy(canonical(&nested_skill_path), nested_policy.clone()), ]); - let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &nested_skill_dir); + let resolved = + resolve_skill_sandbox_extension_for_command(&outcome, &nested_skill_dir, &[]); assert_eq!(resolved, Some(parent_policy)); } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index dca6e36ca8..60fbf93ea3 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -11,6 +11,7 @@ use crate::exec_env::create_env; use crate::exec_policy::ExecApprovalRequest; use crate::function_tool::FunctionCallError; use crate::is_safe_command::is_known_safe_command; +use crate::parse_command::parse_command; use crate::protocol::ExecCommandSource; use crate::sandboxing::extend_sandbox_policy; use crate::shell::Shell; @@ -304,14 +305,18 @@ impl ShellHandler { .skills_manager .skills_for_cwd(&turn.cwd, false) .await; - let effective_sandbox_policy = - resolve_skill_sandbox_extension_for_command(&skills_outcome, &exec_params.cwd) - .map_or_else( - || turn.sandbox_policy.clone(), - |skill_sandbox_policy| { - extend_sandbox_policy(&turn.sandbox_policy, &skill_sandbox_policy) - }, - ); + let command_actions = parse_command(&exec_params.command); + let effective_sandbox_policy = resolve_skill_sandbox_extension_for_command( + &skills_outcome, + &exec_params.cwd, + &command_actions, + ) + .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 diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 6c6ff032b2..a3b83a9fd0 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -14,6 +14,7 @@ use tokio_util::sync::CancellationToken; use crate::exec_env::create_env; use crate::exec_policy::ExecApprovalRequest; +use crate::parse_command::parse_command; use crate::protocol::ExecCommandSource; use crate::sandboxing::ExecRequest; use crate::sandboxing::extend_sandbox_policy; @@ -614,13 +615,15 @@ impl UnifiedExecProcessManager { .skills_manager .skills_for_cwd(&context.turn.cwd, false) .await; + let command_actions = parse_command(&request.command); let effective_sandbox_policy = - resolve_skill_sandbox_extension_for_command(&skills_outcome, &cwd).map_or_else( - || context.turn.sandbox_policy.clone(), - |skill_sandbox_policy| { - extend_sandbox_policy(&context.turn.sandbox_policy, &skill_sandbox_policy) - }, - ); + resolve_skill_sandbox_extension_for_command(&skills_outcome, &cwd, &command_actions) + .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