From a63624a61a2fa50d11f8863f9225b9f861062f3f Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Thu, 5 Mar 2026 12:05:35 -0800 Subject: [PATCH] feat: merge skill permission profiles into the turn sandbox for zsh-fork execs (#13496) ## Summary This changes the Unix shell escalation path for skill-matched executables to apply a skill's `PermissionProfile` as additive permissions on top of the existing turn/request sandbox policy. Previously, skill-matched executables compiled the skill permission profile into a standalone sandbox policy and executed against that replacement policy. Now they go through the same `additional_permissions` merge path used elsewhere in shell sandbox preparation. ## What Changed - Changed `skill_escalation_execution()` to return `EscalationPermissions::PermissionProfile(...)` for non-empty skill permission profiles. - Kept empty or missing skill permission profiles on the `TurnDefault` path. - Added tests covering the new additive skill-permission behavior. - Added inline comments in `prepare_escalated_exec()` clarifying the difference between additive permission merging and fully specified replacement sandbox policies. - Removed the now-unused skill permission compiler module after switching this path away from standalone compiled skill sandbox policies. ## Testing - Ran `just fmt` in `codex-rs` - Ran `cargo test -p codex-core` `cargo test -p codex-core` still hits an unrelated existing failure: `shell_snapshot::tests::snapshot_shell_does_not_inherit_stdin` ## Follow-up This change intentionally does not merge skill-specific macOS seatbelt profile extensions through the `additional_permissions` path yet. Filesystem and network permissions now follow the additive merge path, but seatbelt extension permissions still need separate handling in a follow-up PR. --- codex-rs/core/src/skills/mod.rs | 1 - codex-rs/core/src/skills/permissions.rs | 454 ------------------ .../tools/runtimes/shell/unix_escalation.rs | 114 +++-- .../runtimes/shell/unix_escalation_tests.rs | 52 ++ 4 files changed, 118 insertions(+), 503 deletions(-) delete mode 100644 codex-rs/core/src/skills/permissions.rs diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core/src/skills/mod.rs index 2dc7e11c3..8c311c5d3 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core/src/skills/mod.rs @@ -4,7 +4,6 @@ pub(crate) mod invocation_utils; pub mod loader; pub mod manager; pub mod model; -pub mod permissions; pub mod remote; pub mod render; pub mod system; diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs deleted file mode 100644 index 53b1f7bd9..000000000 --- a/codex-rs/core/src/skills/permissions.rs +++ /dev/null @@ -1,454 +0,0 @@ -#[cfg(any(unix, test))] -use std::collections::HashSet; - -#[cfg(target_os = "macos")] -use codex_protocol::models::MacOsAutomationValue; -#[cfg(any(unix, test))] -use codex_protocol::models::MacOsPermissions; -#[cfg(target_os = "macos")] -use codex_protocol::models::MacOsPreferencesValue; -#[cfg(any(unix, test))] -use codex_protocol::models::MacOsSeatbeltProfileExtensions; -#[cfg(any(unix, test))] -use codex_protocol::models::PermissionProfile; -#[cfg(any(unix, test))] -use codex_utils_absolute_path::AbsolutePathBuf; -#[cfg(any(unix, test))] -use dunce::canonicalize as canonicalize_path; -#[cfg(any(unix, test))] -use tracing::warn; - -#[cfg(any(unix, test))] -use crate::config::Constrained; -#[cfg(any(unix, test))] -use crate::config::Permissions; -#[cfg(any(unix, test))] -use crate::config::types::ShellEnvironmentPolicy; -#[cfg(any(unix, test))] -use crate::protocol::AskForApproval; -#[cfg(any(unix, test))] -use crate::protocol::ReadOnlyAccess; -#[cfg(any(unix, test))] -use crate::protocol::SandboxPolicy; - -/// Compiles a skill `PermissionProfile` for the Unix shell escalation path. -/// -/// Normal Windows builds do not currently call this helper, so it is only -/// compiled on Unix and in tests. -#[cfg(any(unix, test))] -pub(crate) fn compile_permission_profile( - permissions: Option, -) -> Option { - let PermissionProfile { - network, - file_system, - macos, - } = permissions?; - let network_access = network.and_then(|value| value.enabled).unwrap_or_default(); - let file_system = file_system.unwrap_or_default(); - let fs_read = normalize_permission_paths( - file_system.read.as_deref().unwrap_or_default(), - "permissions.file_system.read", - ); - let fs_write = normalize_permission_paths( - file_system.write.as_deref().unwrap_or_default(), - "permissions.file_system.write", - ); - let sandbox_policy = if !fs_write.is_empty() { - SandboxPolicy::WorkspaceWrite { - writable_roots: fs_write, - read_only_access: if fs_read.is_empty() { - ReadOnlyAccess::FullAccess - } else { - ReadOnlyAccess::Restricted { - include_platform_defaults: true, - readable_roots: fs_read, - } - }, - network_access, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - } - } else if !fs_read.is_empty() { - SandboxPolicy::ReadOnly { - access: ReadOnlyAccess::Restricted { - include_platform_defaults: true, - readable_roots: fs_read, - }, - network_access, - } - } else if network_access { - SandboxPolicy::ReadOnly { - access: ReadOnlyAccess::FullAccess, - network_access: true, - } - } else { - // Default sandbox policy - SandboxPolicy::new_read_only_policy() - }; - let macos_permissions = macos.unwrap_or_default(); - let macos_seatbelt_profile_extensions = - build_macos_seatbelt_profile_extensions(&macos_permissions); - - Some(Permissions { - approval_policy: Constrained::allow_any(AskForApproval::Never), - sandbox_policy: Constrained::allow_any(sandbox_policy), - network: None, - allow_login_shell: true, - shell_environment_policy: ShellEnvironmentPolicy::default(), - windows_sandbox_mode: None, - macos_seatbelt_profile_extensions, - }) -} - -#[cfg(any(unix, test))] -fn normalize_permission_paths(values: &[AbsolutePathBuf], field: &str) -> Vec { - let mut paths = Vec::new(); - let mut seen = HashSet::new(); - - for value in values { - let Some(path) = normalize_permission_path(value, field) else { - continue; - }; - if seen.insert(path.clone()) { - paths.push(path); - } - } - - paths -} - -#[cfg(any(unix, test))] -fn normalize_permission_path(value: &AbsolutePathBuf, field: &str) -> Option { - let canonicalized = canonicalize_path(value.as_path()).unwrap_or_else(|_| value.to_path_buf()); - match AbsolutePathBuf::from_absolute_path(&canonicalized) { - Ok(path) => Some(path), - Err(error) => { - warn!("ignoring {field}: expected absolute path, got {canonicalized:?}: {error}"); - None - } - } -} - -#[cfg(target_os = "macos")] -fn build_macos_seatbelt_profile_extensions( - permissions: &MacOsPermissions, -) -> Option { - let defaults = MacOsSeatbeltProfileExtensions::default(); - - let extensions = MacOsSeatbeltProfileExtensions { - macos_preferences: resolve_macos_preferences_permission( - permissions.preferences.as_ref(), - defaults.macos_preferences, - ), - macos_automation: resolve_macos_automation_permission( - permissions.automations.as_ref(), - defaults.macos_automation, - ), - macos_accessibility: permissions - .accessibility - .unwrap_or(defaults.macos_accessibility), - macos_calendar: permissions.calendar.unwrap_or(defaults.macos_calendar), - }; - Some(extensions) -} - -#[cfg(target_os = "macos")] -fn resolve_macos_preferences_permission( - value: Option<&MacOsPreferencesValue>, - default: crate::seatbelt_permissions::MacOsPreferencesPermission, -) -> crate::seatbelt_permissions::MacOsPreferencesPermission { - use crate::seatbelt_permissions::MacOsPreferencesPermission; - - match value { - Some(MacOsPreferencesValue::Bool(true)) => MacOsPreferencesPermission::ReadOnly, - Some(MacOsPreferencesValue::Bool(false)) => MacOsPreferencesPermission::None, - Some(MacOsPreferencesValue::Mode(mode)) => { - let mode = mode.trim(); - if mode.eq_ignore_ascii_case("readonly") || mode.eq_ignore_ascii_case("read-only") { - MacOsPreferencesPermission::ReadOnly - } else if mode.eq_ignore_ascii_case("readwrite") - || mode.eq_ignore_ascii_case("read-write") - { - MacOsPreferencesPermission::ReadWrite - } else { - warn!( - "ignoring permissions.macos.preferences: expected true/false, readonly, or readwrite" - ); - default - } - } - None => default, - } -} - -#[cfg(target_os = "macos")] -fn resolve_macos_automation_permission( - value: Option<&MacOsAutomationValue>, - default: crate::seatbelt_permissions::MacOsAutomationPermission, -) -> crate::seatbelt_permissions::MacOsAutomationPermission { - use crate::seatbelt_permissions::MacOsAutomationPermission; - - match value { - Some(MacOsAutomationValue::Bool(true)) => MacOsAutomationPermission::All, - Some(MacOsAutomationValue::Bool(false)) => MacOsAutomationPermission::None, - Some(MacOsAutomationValue::BundleIds(bundle_ids)) => { - let bundle_ids = bundle_ids - .iter() - .map(|bundle_id| bundle_id.trim()) - .filter(|bundle_id| !bundle_id.is_empty()) - .map(ToOwned::to_owned) - .collect::>(); - if bundle_ids.is_empty() { - MacOsAutomationPermission::None - } else { - MacOsAutomationPermission::BundleIds(bundle_ids) - } - } - None => default, - } -} - -#[cfg(all(not(target_os = "macos"), any(unix, test)))] -fn build_macos_seatbelt_profile_extensions( - _: &MacOsPermissions, -) -> Option { - None -} - -#[cfg(test)] -mod tests { - use super::compile_permission_profile; - 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 codex_protocol::models::FileSystemPermissions; - #[cfg(target_os = "macos")] - use codex_protocol::models::MacOsAutomationValue; - #[cfg(target_os = "macos")] - use codex_protocol::models::MacOsPermissions; - #[cfg(target_os = "macos")] - use codex_protocol::models::MacOsPreferencesValue; - use codex_protocol::models::NetworkPermissions; - use codex_protocol::models::PermissionProfile; - use codex_utils_absolute_path::AbsolutePathBuf; - use pretty_assertions::assert_eq; - use std::fs; - use std::path::Path; - - fn absolute_path(path: &Path) -> AbsolutePathBuf { - AbsolutePathBuf::try_from(path).expect("absolute path") - } - - #[test] - fn compile_permission_profile_normalizes_paths() { - let tempdir = tempfile::tempdir().expect("tempdir"); - let skill_dir = tempdir.path().join("skill"); - fs::create_dir_all(skill_dir.join("scripts")).expect("skill dir"); - let read_dir = skill_dir.join("data"); - fs::create_dir_all(&read_dir).expect("read dir"); - - let profile = compile_permission_profile(Some(PermissionProfile { - network: Some(NetworkPermissions { - enabled: Some(true), - }), - file_system: Some(FileSystemPermissions { - read: Some(vec![ - absolute_path(&skill_dir.join("data")), - absolute_path(&skill_dir.join("data")), - absolute_path(&skill_dir.join("scripts/../data")), - ]), - write: Some(vec![absolute_path(&skill_dir.join("output"))]), - }), - ..Default::default() - })) - .expect("profile"); - - assert_eq!( - profile, - Permissions { - approval_policy: Constrained::allow_any(AskForApproval::Never), - sandbox_policy: Constrained::allow_any(SandboxPolicy::WorkspaceWrite { - writable_roots: vec![ - AbsolutePathBuf::try_from(skill_dir.join("output")) - .expect("absolute output path") - ], - read_only_access: ReadOnlyAccess::Restricted { - include_platform_defaults: true, - readable_roots: vec![ - AbsolutePathBuf::try_from( - dunce::canonicalize(&read_dir).unwrap_or(read_dir) - ) - .expect("absolute read path") - ], - }, - network_access: true, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - }), - network: None, - allow_login_shell: true, - shell_environment_policy: ShellEnvironmentPolicy::default(), - windows_sandbox_mode: None, - #[cfg(target_os = "macos")] - macos_seatbelt_profile_extensions: Some( - crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default(), - ), - #[cfg(not(target_os = "macos"))] - macos_seatbelt_profile_extensions: None, - } - ); - } - - #[test] - fn compile_permission_profile_without_permissions_has_empty_profile() { - let tempdir = tempfile::tempdir().expect("tempdir"); - let skill_dir = tempdir.path().join("skill"); - fs::create_dir_all(&skill_dir).expect("skill dir"); - - let profile = compile_permission_profile(None); - - assert_eq!(profile, None); - } - - #[test] - fn compile_permission_profile_with_network_only_uses_read_only_policy() { - let tempdir = tempfile::tempdir().expect("tempdir"); - let skill_dir = tempdir.path().join("skill"); - fs::create_dir_all(&skill_dir).expect("skill dir"); - - let profile = compile_permission_profile(Some(PermissionProfile { - network: Some(NetworkPermissions { - enabled: Some(true), - }), - ..Default::default() - })) - .expect("profile"); - - assert_eq!( - profile, - Permissions { - approval_policy: Constrained::allow_any(AskForApproval::Never), - sandbox_policy: Constrained::allow_any(SandboxPolicy::ReadOnly { - access: ReadOnlyAccess::FullAccess, - network_access: true, - }), - network: None, - allow_login_shell: true, - shell_environment_policy: ShellEnvironmentPolicy::default(), - windows_sandbox_mode: None, - #[cfg(target_os = "macos")] - macos_seatbelt_profile_extensions: Some( - crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default(), - ), - #[cfg(not(target_os = "macos"))] - macos_seatbelt_profile_extensions: None, - } - ); - } - - #[test] - fn compile_permission_profile_with_network_and_read_only_paths_uses_read_only_policy() { - let tempdir = tempfile::tempdir().expect("tempdir"); - let skill_dir = tempdir.path().join("skill"); - let read_dir = skill_dir.join("data"); - fs::create_dir_all(&read_dir).expect("read dir"); - - let profile = compile_permission_profile(Some(PermissionProfile { - network: Some(NetworkPermissions { - enabled: Some(true), - }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path(&skill_dir.join("data"))]), - write: Some(Vec::new()), - }), - ..Default::default() - })) - .expect("profile"); - - assert_eq!( - profile, - Permissions { - approval_policy: Constrained::allow_any(AskForApproval::Never), - sandbox_policy: Constrained::allow_any(SandboxPolicy::ReadOnly { - access: ReadOnlyAccess::Restricted { - include_platform_defaults: true, - readable_roots: vec![ - AbsolutePathBuf::try_from( - dunce::canonicalize(&read_dir).unwrap_or(read_dir) - ) - .expect("absolute read path") - ], - }, - network_access: true, - }), - network: None, - allow_login_shell: true, - shell_environment_policy: ShellEnvironmentPolicy::default(), - windows_sandbox_mode: None, - #[cfg(target_os = "macos")] - macos_seatbelt_profile_extensions: Some( - crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default(), - ), - #[cfg(not(target_os = "macos"))] - macos_seatbelt_profile_extensions: None, - } - ); - } - - #[cfg(target_os = "macos")] - #[test] - fn compile_permission_profile_builds_macos_permission_file() { - let tempdir = tempfile::tempdir().expect("tempdir"); - let skill_dir = tempdir.path().join("skill"); - fs::create_dir_all(&skill_dir).expect("skill dir"); - - let profile = compile_permission_profile(Some(PermissionProfile { - macos: Some(MacOsPermissions { - preferences: Some(MacOsPreferencesValue::Mode("readwrite".to_string())), - automations: Some(MacOsAutomationValue::BundleIds(vec![ - "com.apple.Notes".to_string(), - ])), - accessibility: Some(true), - calendar: Some(true), - }), - ..Default::default() - })) - .expect("profile"); - - assert_eq!( - profile.macos_seatbelt_profile_extensions, - Some( - crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions { - macos_preferences: - crate::seatbelt_permissions::MacOsPreferencesPermission::ReadWrite, - macos_automation: - crate::seatbelt_permissions::MacOsAutomationPermission::BundleIds(vec![ - "com.apple.Notes".to_string() - ],), - macos_accessibility: true, - macos_calendar: true, - } - ) - ); - } - - #[cfg(target_os = "macos")] - #[test] - fn compile_permission_profile_uses_macos_defaults_when_values_missing() { - let tempdir = tempfile::tempdir().expect("tempdir"); - let skill_dir = tempdir.path().join("skill"); - fs::create_dir_all(&skill_dir).expect("skill dir"); - - let profile = - compile_permission_profile(Some(PermissionProfile::default())).expect("profile"); - - assert_eq!( - profile.macos_seatbelt_profile_extensions, - Some(crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default()) - ); - } -} diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 1be2654ce..7cc8d47f3 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -10,7 +10,6 @@ use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; use crate::skills::SkillMetadata; -use crate::skills::permissions::compile_permission_profile; use crate::tools::runtimes::ExecveSessionApproval; use crate::tools::runtimes::build_command_spec; use crate::tools::sandboxing::SandboxAttempt; @@ -332,18 +331,14 @@ impl CoreShellActionProvider { } fn skill_escalation_execution(skill: &SkillMetadata) -> EscalationExecution { - compile_permission_profile(skill.permission_profile.clone()) - .map(|permissions| { - EscalationExecution::Permissions(EscalationPermissions::Permissions( - EscalatedPermissions { - sandbox_policy: permissions.sandbox_policy.get().clone(), - macos_seatbelt_profile_extensions: permissions - .macos_seatbelt_profile_extensions - .clone(), - }, - )) - }) - .unwrap_or(EscalationExecution::TurnDefault) + let permission_profile = skill.permission_profile.clone().unwrap_or_default(); + if permission_profile.is_empty() { + EscalationExecution::TurnDefault + } else { + EscalationExecution::Permissions(EscalationPermissions::PermissionProfile( + permission_profile, + )) + } } async fn prompt( @@ -741,11 +736,22 @@ struct CoreShellCommandExecutor { justification: Option, arg0: Option, sandbox_policy_cwd: PathBuf, + #[cfg_attr(not(target_os = "macos"), allow(dead_code))] macos_seatbelt_profile_extensions: Option, codex_linux_sandbox_exe: Option, use_linux_sandbox_bwrap: bool, } +struct PrepareSandboxedExecParams<'a> { + command: Vec, + workdir: &'a AbsolutePathBuf, + env: HashMap, + sandbox_policy: &'a SandboxPolicy, + additional_permissions: Option, + #[cfg(target_os = "macos")] + macos_seatbelt_profile_extensions: Option<&'a MacOsSeatbeltProfileExtensions>, +} + #[async_trait::async_trait] impl ShellCommandExecutor for CoreShellCommandExecutor { async fn run( @@ -816,33 +822,46 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { env, arg0: Some(first_arg.clone()), }, - EscalationExecution::TurnDefault => self.prepare_sandboxed_exec( - command, - workdir, - env, - &self.sandbox_policy, - None, - self.macos_seatbelt_profile_extensions.as_ref(), - )?, - EscalationExecution::Permissions(EscalationPermissions::PermissionProfile( - permission_profile, - )) => self.prepare_sandboxed_exec( - command, - workdir, - env, - &self.sandbox_policy, - Some(permission_profile), - None, - )?, - EscalationExecution::Permissions(EscalationPermissions::Permissions(permissions)) => { - self.prepare_sandboxed_exec( + EscalationExecution::TurnDefault => { + self.prepare_sandboxed_exec(PrepareSandboxedExecParams { command, workdir, env, - &permissions.sandbox_policy, - None, - permissions.macos_seatbelt_profile_extensions.as_ref(), - )? + sandbox_policy: &self.sandbox_policy, + additional_permissions: None, + #[cfg(target_os = "macos")] + macos_seatbelt_profile_extensions: self + .macos_seatbelt_profile_extensions + .as_ref(), + })? + } + EscalationExecution::Permissions(EscalationPermissions::PermissionProfile( + permission_profile, + )) => { + // Merge additive permissions into the existing turn/request sandbox policy. + self.prepare_sandboxed_exec(PrepareSandboxedExecParams { + command, + workdir, + env, + sandbox_policy: &self.sandbox_policy, + additional_permissions: Some(permission_profile), + #[cfg(target_os = "macos")] + macos_seatbelt_profile_extensions: None, + })? + } + EscalationExecution::Permissions(EscalationPermissions::Permissions(permissions)) => { + // Use a fully specified sandbox policy instead of merging into the turn policy. + self.prepare_sandboxed_exec(PrepareSandboxedExecParams { + command, + workdir, + env, + sandbox_policy: &permissions.sandbox_policy, + additional_permissions: None, + #[cfg(target_os = "macos")] + macos_seatbelt_profile_extensions: permissions + .macos_seatbelt_profile_extensions + .as_ref(), + })? } }; @@ -853,18 +872,17 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { impl CoreShellCommandExecutor { fn prepare_sandboxed_exec( &self, - command: Vec, - workdir: &AbsolutePathBuf, - env: HashMap, - sandbox_policy: &SandboxPolicy, - additional_permissions: Option, - #[cfg(target_os = "macos")] macos_seatbelt_profile_extensions: Option< - &MacOsSeatbeltProfileExtensions, - >, - #[cfg(not(target_os = "macos"))] _macos_seatbelt_profile_extensions: Option< - &MacOsSeatbeltProfileExtensions, - >, + params: PrepareSandboxedExecParams<'_>, ) -> anyhow::Result { + let PrepareSandboxedExecParams { + command, + workdir, + env, + sandbox_policy, + additional_permissions, + #[cfg(target_os = "macos")] + macos_seatbelt_profile_extensions, + } = params; let (program, args) = command .split_first() .ok_or_else(|| anyhow::anyhow!("prepared command must not be empty"))?; diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index ad663a3fe..e83992b09 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -20,6 +20,7 @@ use crate::protocol::SandboxPolicy; use crate::sandboxing::SandboxPermissions; #[cfg(target_os = "macos")] use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; +use crate::skills::SkillMetadata; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; use codex_execpolicy::PolicyParser; @@ -30,6 +31,7 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::MacOsPreferencesPermission; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::SkillScope; use codex_shell_escalation::EscalationExecution; use codex_shell_escalation::EscalationPermissions; use codex_shell_escalation::ExecResult; @@ -59,6 +61,20 @@ fn starlark_string(value: &str) -> String { value.replace('\\', "\\\\").replace('"', "\\\"") } +fn test_skill_metadata(permission_profile: Option) -> SkillMetadata { + SkillMetadata { + name: "skill".to_string(), + description: "description".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + permission_profile, + path_to_skills_md: PathBuf::from("/tmp/skill/SKILL.md"), + scope: SkillScope::User, + } +} + #[test] fn extract_shell_script_preserves_login_flag() { assert_eq!( @@ -246,6 +262,42 @@ fn shell_request_escalation_execution_is_explicit() { ); } +#[test] +fn skill_escalation_execution_uses_additional_permissions() { + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: None, + write: Some(vec![ + AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(), + ]), + }), + ..Default::default() + }; + + assert_eq!( + CoreShellActionProvider::skill_escalation_execution(&test_skill_metadata(Some( + requested_permissions.clone(), + ))), + EscalationExecution::Permissions(EscalationPermissions::PermissionProfile( + requested_permissions, + )), + ); +} + +#[test] +fn skill_escalation_execution_ignores_empty_permissions() { + assert_eq!( + CoreShellActionProvider::skill_escalation_execution(&test_skill_metadata(Some( + PermissionProfile::default(), + ))), + EscalationExecution::TurnDefault, + ); + assert_eq!( + CoreShellActionProvider::skill_escalation_execution(&test_skill_metadata(None)), + EscalationExecution::TurnDefault, + ); +} + #[test] fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_parsing_disabled() { let policy_src = r#"prefix_rule(pattern = ["npm", "publish"], decision = "prompt")"#;