From 28a079f0c67dd3cdd08a6feb9865ff8f4976ba1d Mon Sep 17 00:00:00 2001 From: celia-oai Date: Tue, 17 Feb 2026 20:09:21 -0800 Subject: [PATCH] changes --- codex-rs/core/src/codex.rs | 5 ++ codex-rs/core/src/exec_policy.rs | 77 +++++++++++++++++ codex-rs/core/src/skills/permissions.rs | 109 ++++++++++++++++++++++++ 3 files changed, 191 insertions(+) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b96af69f03..179d428b98 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -326,6 +326,11 @@ impl Codex { let exec_policy = ExecPolicyManager::load(&config.config_layer_stack) .await .map_err(|err| CodexErr::Fatal(format!("failed to load rules: {err}")))?; + exec_policy.add_skill_script_allow_rules( + loaded_skills + .skills_with_enabled() + .filter_map(|(skill, enabled)| enabled.then_some(skill)), + ); let config = Arc::new(config); let _ = models_manager diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 62ebdde43d..74a78cb77b 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -9,6 +9,8 @@ use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigLayerStackOrdering; use crate::is_dangerous_command::command_might_be_dangerous; use crate::is_safe_command::is_known_safe_command; +use crate::skills::SkillMetadata; +use crate::skills::permissions::exec_policy_prefixes_for_skill_scripts; use codex_execpolicy::AmendError; use codex_execpolicy::Decision; use codex_execpolicy::Error as ExecPolicyRuleError; @@ -158,6 +160,38 @@ impl ExecPolicyManager { self.policy.load_full() } + pub(crate) fn add_skill_script_allow_rules<'a, I>(&self, skills: I) + where + I: IntoIterator, + { + let mut prefixes = Vec::new(); + for skill in skills { + let Some(skill_dir) = skill.path.parent() else { + continue; + }; + prefixes.extend(exec_policy_prefixes_for_skill_scripts( + skill_dir, + skill.permissions.as_ref(), + )); + } + if prefixes.is_empty() { + return; + } + + prefixes.sort(); + prefixes.dedup(); + let mut updated_policy = self.current().as_ref().clone(); + for prefix in prefixes { + if let Err(error) = updated_policy.add_prefix_rule(&prefix, Decision::Allow) { + tracing::warn!( + ?prefix, + "failed to add skill script execpolicy rule: {error}" + ); + } + } + self.policy.store(Arc::new(updated_policy)); + } + pub(crate) async fn create_exec_approval_requirement_for_command( &self, req: ExecApprovalRequest<'_>, @@ -688,9 +722,13 @@ mod tests { use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; + use crate::skills::SkillMetadata; + use crate::skills::permissions::SkillManifestPermissions; + use crate::skills::permissions::compile_permission_profile; use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; + use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::fs; @@ -1388,6 +1426,45 @@ prefix_rule( )); } + #[test] + fn add_skill_script_allow_rules_includes_script_files_for_permissioned_skills() { + let temp_dir = tempdir().expect("create temp dir"); + let skill_dir = temp_dir.path().join("skill"); + let scripts_dir = skill_dir.join("scripts"); + fs::create_dir_all(&scripts_dir).expect("create scripts dir"); + let script_path = scripts_dir.join("run.sh"); + fs::write(&script_path, "#!/bin/sh\necho hi\n").expect("write script"); + let permission_profile = + compile_permission_profile(&skill_dir, Some(SkillManifestPermissions::default())) + .expect("permission profile"); + let skill = SkillMetadata { + name: "test-skill".to_string(), + description: "skill".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + permissions: Some(permission_profile), + path: skill_dir.join("SKILL.md"), + scope: SkillScope::User, + }; + let manager = ExecPolicyManager::default(); + + manager.add_skill_script_allow_rules([&skill]); + let command = [script_path.to_string_lossy().into_owned()]; + let evaluation = manager.current().check(&command, &|_| Decision::Forbidden); + + assert_eq!(evaluation.decision, Decision::Allow); + assert_eq!( + evaluation.matched_rules, + vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec![script_path.to_string_lossy().into_owned()], + decision: Decision::Allow, + justification: None, + }] + ); + } + #[tokio::test] async fn proposed_execpolicy_amendment_is_present_for_single_command_without_policy_match() { let command = vec!["cargo".to_string(), "build".to_string()]; diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs index 1cf18e1052..4e0a911952 100644 --- a/codex-rs/core/src/skills/permissions.rs +++ b/codex-rs/core/src/skills/permissions.rs @@ -1,4 +1,5 @@ use std::collections::HashSet; +use std::io::ErrorKind; use std::path::Component; use std::path::Path; use std::path::PathBuf; @@ -118,6 +119,70 @@ pub(crate) fn compile_permission_profile( }) } +pub(crate) fn exec_policy_prefixes_for_skill_scripts( + skill_dir: &Path, + permission_profile: Option<&Permissions>, +) -> Vec> { + let Some(permission_profile) = permission_profile else { + return Vec::new(); + }; + match permission_profile.sandbox_policy.get() { + SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } => {} + SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { + return Vec::new(); + } + } + + let scripts_dir = skill_dir.join("scripts"); + let mut pending_dirs = vec![scripts_dir]; + let mut script_files = Vec::new(); + while let Some(dir) = pending_dirs.pop() { + let read_dir = match std::fs::read_dir(&dir) { + Ok(read_dir) => read_dir, + Err(error) if error.kind() == ErrorKind::NotFound => continue, + Err(error) => { + warn!("failed to read scripts dir {}: {error}", dir.display()); + continue; + } + }; + + for entry in read_dir { + let entry = match entry { + Ok(entry) => entry, + Err(error) => { + warn!("failed to read scripts entry in {}: {error}", dir.display()); + continue; + } + }; + let path = entry.path(); + let file_type = match entry.file_type() { + Ok(file_type) => file_type, + Err(error) => { + warn!( + "failed to read scripts entry type for {}: {error}", + path.display() + ); + continue; + } + }; + if file_type.is_dir() { + pending_dirs.push(path); + continue; + } + if file_type.is_file() { + script_files.push(path); + } + } + } + + script_files.sort(); + script_files.dedup(); + script_files + .into_iter() + .map(|path| vec![path.to_string_lossy().into_owned()]) + .collect() +} + fn normalize_permission_paths( skill_dir: &Path, values: &[String], @@ -289,6 +354,7 @@ mod tests { use super::SkillManifestMacOsPermissions; use super::SkillManifestPermissions; use super::compile_permission_profile; + use super::exec_policy_prefixes_for_skill_scripts; use crate::config::Constrained; use crate::config::Permissions; use crate::config::types::ShellEnvironmentPolicy; @@ -370,6 +436,49 @@ mod tests { assert_eq!(profile, None); } + #[test] + fn exec_policy_prefixes_for_skill_scripts_returns_empty_without_permission_profile() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let skill_dir = tempdir.path().join("skill"); + fs::create_dir_all(skill_dir.join("scripts")).expect("scripts dir"); + fs::write( + skill_dir.join("scripts").join("run.sh"), + "#!/bin/sh\necho hi\n", + ) + .expect("script file"); + + let prefixes = exec_policy_prefixes_for_skill_scripts(&skill_dir, None); + + assert_eq!(prefixes, Vec::>::new()); + } + + #[test] + fn exec_policy_prefixes_for_skill_scripts_has_one_rule_per_script_file() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let skill_dir = tempdir.path().join("skill"); + let scripts_dir = skill_dir.join("scripts"); + fs::create_dir_all(scripts_dir.join("nested")).expect("scripts dirs"); + let root_script = scripts_dir.join("run.sh"); + let nested_script = scripts_dir.join("nested").join("build.py"); + fs::write(&root_script, "#!/bin/sh\necho hi\n").expect("root script"); + fs::write(&nested_script, "#!/usr/bin/env python3\nprint('ok')\n").expect("nested script"); + + let permission_profile = + compile_permission_profile(&skill_dir, Some(SkillManifestPermissions::default())) + .expect("permission profile"); + + let prefixes = + exec_policy_prefixes_for_skill_scripts(&skill_dir, Some(&permission_profile)); + + assert_eq!( + prefixes, + vec![ + vec![nested_script.to_string_lossy().into_owned()], + vec![root_script.to_string_lossy().into_owned()], + ] + ); + } + #[test] fn compile_permission_profile_with_network_only_uses_read_only_policy() { let tempdir = tempfile::tempdir().expect("tempdir");