From 2ab139d57bb5eb89ce04febc487bd44d926a4265 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 28 Feb 2026 09:01:07 -0800 Subject: [PATCH] fix: merge skill permissions into escalated sandbox by default --- codex-rs/core/src/sandboxing/mod.rs | 18 +- codex-rs/core/src/skills/loader.rs | 102 +++++++++- codex-rs/core/src/skills/permissions.rs | 158 ++++++++++++++- .../tools/runtimes/shell/unix_escalation.rs | 59 +++++- codex-rs/core/tests/suite/skill_approval.rs | 191 +++++++++++++++++- .../skill-creator/references/openai_yaml.md | 13 ++ 6 files changed, 509 insertions(+), 32 deletions(-) diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index bbd73749e4..e041734412 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -189,17 +189,17 @@ fn merge_read_only_access_with_additional_reads( } } -fn sandbox_policy_with_additional_permissions( +pub(crate) fn sandbox_policy_with_additional_permissions( sandbox_policy: &SandboxPolicy, additional_permissions: &PermissionProfile, -) -> Result { +) -> SandboxPolicy { if additional_permissions.is_empty() { - return Ok(sandbox_policy.clone()); + return sandbox_policy.clone(); } let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions); - let policy = match sandbox_policy { + match sandbox_policy { SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { sandbox_policy.clone() } @@ -218,7 +218,7 @@ fn sandbox_policy_with_additional_permissions( read_only_access, extra_reads, ), - network_access: *network_access, + network_access: *network_access || additional_permissions.network.unwrap_or(false), exclude_tmpdir_env_var: *exclude_tmpdir_env_var, exclude_slash_tmp: *exclude_slash_tmp, } @@ -238,15 +238,13 @@ fn sandbox_policy_with_additional_permissions( access, extra_reads, ), - network_access: false, + network_access: additional_permissions.network.unwrap_or(false), exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } } } - }; - - Ok(policy) + } } #[derive(Default)] @@ -312,7 +310,7 @@ impl SandboxManager { } = request; let effective_policy = if let Some(additional_permissions) = spec.additional_permissions.take() { - sandbox_policy_with_additional_permissions(policy, &additional_permissions)? + sandbox_policy_with_additional_permissions(policy, &additional_permissions) } else { policy.clone() }; diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index 83db7a9078..d43785e9f7 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -13,6 +13,7 @@ use crate::skills::model::SkillMetadata; use crate::skills::model::SkillPolicy; use crate::skills::model::SkillToolDependency; use crate::skills::permissions::compile_permission_profile; +use crate::skills::permissions::normalize_permission_profile; use crate::skills::system::system_cache_root_dir; use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::models::PermissionProfile; @@ -55,7 +56,23 @@ struct SkillMetadataFile { #[serde(default)] policy: Option, #[serde(default)] - permissions: Option, + permissions: Option, +} + +#[derive(Debug, Clone, Default, Deserialize)] +#[serde(rename_all = "snake_case")] +enum SkillPermissionsMode { + #[default] + Merge, + Exact, +} + +#[derive(Debug, Clone, Default, Deserialize)] +struct SkillPermissionsConfig { + #[serde(default)] + mode: SkillPermissionsMode, + #[serde(flatten)] + profile: PermissionProfile, } #[derive(Default)] @@ -595,14 +612,24 @@ fn load_skill_metadata(skill_path: &Path) -> LoadedSkillMetadata { policy, permissions, } = parsed; - let permission_profile = permissions.clone().filter(|profile| !profile.is_empty()); + let permission_profile = permissions + .as_ref() + .map(|permissions| normalize_permission_profile(permissions.profile.clone())) + .filter(|profile| !profile.is_empty()); + let exact_permissions = permissions.and_then(|permissions| { + if matches!(permissions.mode, SkillPermissionsMode::Exact) { + compile_permission_profile(skill_dir, Some(permissions.profile)) + } else { + None + } + }); LoadedSkillMetadata { interface: resolve_interface(interface, skill_dir), dependencies: resolve_dependencies(dependencies), policy: resolve_policy(policy), permission_profile, - permissions: compile_permission_profile(skill_dir, permissions), + permissions: exact_permissions, } } @@ -1392,6 +1419,66 @@ permissions: macos: None, }) ); + assert_eq!(outcome.skills[0].permissions, None); + } + + #[tokio::test] + async fn empty_skill_permissions_do_not_create_profile() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "permissions-empty", "from yaml"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_metadata_at( + skill_dir, + r#" +permissions: {} +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills_for_test(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); + assert_eq!(outcome.skills[0].permission_profile, None); + assert_eq!(outcome.skills[0].permissions, None); + } + + #[tokio::test] + async fn loads_exact_skill_permissions_from_yaml() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "permissions-skill-exact", "from yaml"); + let skill_dir = skill_path.parent().expect("skill dir"); + fs::create_dir_all(skill_dir.join("data")).expect("create read path"); + fs::create_dir_all(skill_dir.join("output")).expect("create write path"); + + write_skill_metadata_at( + skill_dir, + r#" +permissions: + mode: exact + network: true + file_system: + read: + - "./data" + write: + - "./output" +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills_for_test(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); #[cfg(target_os = "macos")] let macos_seatbelt_profile_extensions = Some(crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default()); @@ -1433,15 +1520,16 @@ permissions: } #[tokio::test] - async fn empty_skill_permissions_do_not_create_profile() { + async fn empty_exact_skill_permissions_compile_default_sandbox() { let codex_home = tempfile::tempdir().expect("tempdir"); - let skill_path = write_skill(&codex_home, "demo", "permissions-empty", "from yaml"); + let skill_path = write_skill(&codex_home, "demo", "permissions-empty-exact", "from yaml"); let skill_dir = skill_path.parent().expect("skill dir"); write_skill_metadata_at( skill_dir, r#" -permissions: {} +permissions: + mode: exact "#, ); @@ -1495,6 +1583,7 @@ permissions: {} skill_dir, r#" permissions: + mode: exact macos: preferences: "readwrite" automations: @@ -1545,6 +1634,7 @@ permissions: skill_dir, r#" permissions: + mode: exact macos: preferences: "readwrite" automations: diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs index 38d02ee0b6..e7b634c9cb 100644 --- a/codex-rs/core/src/skills/permissions.rs +++ b/codex-rs/core/src/skills/permissions.rs @@ -1,10 +1,16 @@ +#[cfg(target_os = "macos")] +use std::collections::BTreeSet; use std::collections::HashSet; use std::path::Path; +#[cfg(target_os = "macos")] +use codex_protocol::models::MacOsAutomationPermission; #[cfg(target_os = "macos")] use codex_protocol::models::MacOsAutomationValue; use codex_protocol::models::MacOsPermissions; #[cfg(target_os = "macos")] +use codex_protocol::models::MacOsPreferencesPermission; +#[cfg(target_os = "macos")] use codex_protocol::models::MacOsPreferencesValue; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::PermissionProfile; @@ -23,11 +29,12 @@ pub(crate) fn compile_permission_profile( _skill_dir: &Path, permissions: Option, ) -> Option { + let permissions = normalize_permission_profile(permissions?); let PermissionProfile { network, file_system, macos, - } = permissions?; + } = permissions; let file_system = file_system.unwrap_or_default(); let fs_read = normalize_permission_paths( file_system.read.as_deref().unwrap_or_default(), @@ -78,6 +85,36 @@ pub(crate) fn compile_permission_profile( }) } +pub(crate) fn normalize_permission_profile(permissions: PermissionProfile) -> PermissionProfile { + let PermissionProfile { + network, + file_system, + macos, + } = permissions; + let file_system = file_system.and_then(|file_system| { + let read = file_system + .read + .map(|paths| normalize_permission_paths(&paths, "permissions.file_system.read")) + .filter(|paths| !paths.is_empty()); + let write = file_system + .write + .map(|paths| normalize_permission_paths(&paths, "permissions.file_system.write")) + .filter(|paths| !paths.is_empty()); + + if read.is_none() && write.is_none() { + None + } else { + Some(codex_protocol::models::FileSystemPermissions { read, write }) + } + }); + + PermissionProfile { + network, + file_system, + macos, + } +} + fn normalize_permission_paths(values: &[AbsolutePathBuf], field: &str) -> Vec { let mut paths = Vec::new(); let mut seen = HashSet::new(); @@ -128,6 +165,117 @@ fn build_macos_seatbelt_profile_extensions( Some(extensions) } +#[cfg(target_os = "macos")] +pub(crate) fn merge_macos_seatbelt_profile_extensions( + current: Option<&MacOsSeatbeltProfileExtensions>, + permissions: Option<&MacOsPermissions>, +) -> Option { + let Some(permissions) = permissions else { + return current.cloned(); + }; + + let defaults = current.cloned().unwrap_or_default(); + let merged = MacOsSeatbeltProfileExtensions { + macos_preferences: merge_macos_preferences_permission( + permissions.preferences.as_ref(), + defaults.macos_preferences, + ), + macos_automation: merge_macos_automation_permission( + permissions.automations.as_ref(), + defaults.macos_automation, + ), + macos_accessibility: defaults.macos_accessibility + || permissions.accessibility.unwrap_or(false), + macos_calendar: defaults.macos_calendar || permissions.calendar.unwrap_or(false), + }; + Some(merged) +} + +#[cfg(target_os = "macos")] +fn merge_macos_preferences_permission( + value: Option<&MacOsPreferencesValue>, + current: MacOsPreferencesPermission, +) -> MacOsPreferencesPermission { + let requested = match value { + Some(MacOsPreferencesValue::Bool(true)) => Some(MacOsPreferencesPermission::ReadOnly), + Some(MacOsPreferencesValue::Bool(false)) | None => None, + Some(MacOsPreferencesValue::Mode(mode)) => { + let mode = mode.trim(); + if mode.eq_ignore_ascii_case("readonly") || mode.eq_ignore_ascii_case("read-only") { + Some(MacOsPreferencesPermission::ReadOnly) + } else if mode.eq_ignore_ascii_case("readwrite") + || mode.eq_ignore_ascii_case("read-write") + { + Some(MacOsPreferencesPermission::ReadWrite) + } else { + warn!( + "ignoring permissions.macos.preferences: expected true/false, readonly, or readwrite" + ); + None + } + } + }; + + match (current, requested) { + (MacOsPreferencesPermission::ReadWrite, _) => MacOsPreferencesPermission::ReadWrite, + (MacOsPreferencesPermission::ReadOnly, Some(MacOsPreferencesPermission::ReadWrite)) => { + MacOsPreferencesPermission::ReadWrite + } + (MacOsPreferencesPermission::None, Some(permission)) => permission, + (existing, Some(_)) | (existing, None) => existing, + } +} + +#[cfg(target_os = "macos")] +fn merge_macos_automation_permission( + value: Option<&MacOsAutomationValue>, + current: MacOsAutomationPermission, +) -> MacOsAutomationPermission { + let requested = match value { + Some(MacOsAutomationValue::Bool(true)) => Some(MacOsAutomationPermission::All), + Some(MacOsAutomationValue::Bool(false)) | None => 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() { + None + } else { + Some(MacOsAutomationPermission::BundleIds(bundle_ids)) + } + } + }; + + match (current, requested) { + (MacOsAutomationPermission::All, _) | (_, Some(MacOsAutomationPermission::All)) => { + MacOsAutomationPermission::All + } + (MacOsAutomationPermission::None, Some(permission)) => permission, + ( + MacOsAutomationPermission::BundleIds(existing), + Some(MacOsAutomationPermission::BundleIds(requested)), + ) => { + let merged = existing + .into_iter() + .chain(requested) + .map(|bundle_id| bundle_id.trim().to_string()) + .filter(|bundle_id| !bundle_id.is_empty()) + .collect::>() + .into_iter() + .collect::>(); + if merged.is_empty() { + MacOsAutomationPermission::None + } else { + MacOsAutomationPermission::BundleIds(merged) + } + } + (existing, Some(_)) | (existing, None) => existing, + } +} + #[cfg(target_os = "macos")] fn resolve_macos_preferences_permission( value: Option<&MacOsPreferencesValue>, @@ -191,6 +339,14 @@ fn build_macos_seatbelt_profile_extensions( None } +#[cfg(all(unix, not(target_os = "macos")))] +pub(crate) fn merge_macos_seatbelt_profile_extensions( + current: Option<&MacOsSeatbeltProfileExtensions>, + _: Option<&MacOsPermissions>, +) -> Option { + current.cloned() +} + #[cfg(test)] mod tests { use super::compile_permission_profile; 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 5288fd8e82..05c59e915e 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -7,8 +7,10 @@ use crate::exec::SandboxType; use crate::exec::is_likely_sandbox_denied; use crate::features::Feature; use crate::sandboxing::SandboxPermissions; +use crate::sandboxing::sandbox_policy_with_additional_permissions; use crate::shell::ShellType; use crate::skills::SkillMetadata; +use crate::skills::permissions::merge_macos_seatbelt_profile_extensions; use crate::tools::runtimes::ExecveSessionApproval; use crate::tools::runtimes::build_command_spec; use crate::tools::sandboxing::SandboxAttempt; @@ -226,7 +228,11 @@ impl CoreShellActionProvider { } } - fn skill_escalation_execution(skill: &SkillMetadata) -> EscalationExecution { + fn skill_escalation_execution( + skill: &SkillMetadata, + turn_sandbox_policy: &SandboxPolicy, + turn_macos_seatbelt_profile_extensions: Option<&MacOsSeatbeltProfileExtensions>, + ) -> EscalationExecution { skill .permissions .as_ref() @@ -241,11 +247,21 @@ impl CoreShellActionProvider { )) }) .or_else(|| { - skill - .permission_profile - .clone() - .map(EscalationPermissions::PermissionProfile) - .map(EscalationExecution::Permissions) + skill.permission_profile.as_ref().map(|permission_profile| { + EscalationExecution::Permissions(EscalationPermissions::Permissions( + EscalatedPermissions { + sandbox_policy: sandbox_policy_with_additional_permissions( + turn_sandbox_policy, + permission_profile, + ), + macos_seatbelt_profile_extensions: + merge_macos_seatbelt_profile_extensions( + turn_macos_seatbelt_profile_extensions, + permission_profile.macos.as_ref(), + ), + }, + )) + }) }) .unwrap_or(EscalationExecution::TurnDefault) } @@ -265,6 +281,13 @@ impl CoreShellActionProvider { let turn = self.turn.clone(); let call_id = self.call_id.clone(); let approval_id = Some(Uuid::new_v4().to_string()); + let reason = match decision_source { + DecisionSource::SkillScript { skill } if skill.permissions.is_some() => Some( + "Use the skill's declared sandbox exactly; do not merge it with the current sandbox." + .to_string(), + ), + _ => None, + }; Ok(stopwatch .pause_for(async move { let available_decisions = vec![ @@ -288,7 +311,7 @@ impl CoreShellActionProvider { approval_id, command, workdir, - None, + reason, None, None, additional_permissions, @@ -470,7 +493,17 @@ impl EscalationPolicy for CoreShellActionProvider { let execution = approval .skill .as_ref() - .map(Self::skill_escalation_execution) + .map(|skill| { + Self::skill_escalation_execution( + skill, + &self.sandbox_policy, + self.turn + .config + .permissions + .macos_seatbelt_profile_extensions + .as_ref(), + ) + }) .unwrap_or(EscalationExecution::TurnDefault); return Ok(EscalationDecision::escalate(execution)); @@ -495,7 +528,15 @@ impl EscalationPolicy for CoreShellActionProvider { argv, workdir, skill.permission_profile.clone(), - Self::skill_escalation_execution(&skill), + Self::skill_escalation_execution( + &skill, + &self.sandbox_policy, + self.turn + .config + .permissions + .macos_seatbelt_profile_extensions + .as_ref(), + ), decision_source, ) .await; diff --git a/codex-rs/core/tests/suite/skill_approval.rs b/codex-rs/core/tests/suite/skill_approval.rs index 84e1d24085..198ff0a5e3 100644 --- a/codex-rs/core/tests/suite/skill_approval.rs +++ b/codex-rs/core/tests/suite/skill_approval.rs @@ -408,10 +408,10 @@ async fn shell_zsh_fork_skill_without_permissions_inherits_turn_sandbox() -> Res /// and writes to an unrelated folder fail, both before and after cached approval. #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> Result<()> { +async fn shell_zsh_fork_skill_session_approval_merges_skill_permissions() -> Result<()> { skip_if_no_network!(Ok(())); - let Some(runtime) = zsh_fork_runtime("zsh-fork explicit skill sandbox test")? else { + let Some(runtime) = zsh_fork_runtime("zsh-fork merged skill permissions test")? else { return Ok(()); }; @@ -519,12 +519,12 @@ async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> R .to_string(); assert!( first_output.contains("allowed"), - "expected skill sandbox to permit writes to the approved folder, got output: {first_output:?}" + "expected merged skill permissions to permit writes to the approved folder, got output: {first_output:?}" ); assert_eq!(fs::read_to_string(&allowed_path)?, "allowed"); assert!( !blocked_path.exists(), - "first run should not write outside the explicit skill sandbox" + "first run should not write outside the merged skill and turn sandbox" ); assert!( !first_output.contains("blocked-created"), @@ -567,12 +567,12 @@ async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> R .to_string(); assert!( second_output.contains("allowed"), - "expected cached skill approval to retain the explicit skill sandbox, got output: {second_output:?}" + "expected cached skill approval to retain the merged skill permissions, got output: {second_output:?}" ); assert_eq!(fs::read_to_string(&allowed_path)?, "allowed"); assert!( !blocked_path.exists(), - "cached session approval should not widen skill execution beyond the explicit skill sandbox" + "cached session approval should not widen skill execution beyond the merged skill and turn sandbox" ); assert!( !second_output.contains("blocked-created"), @@ -582,6 +582,185 @@ async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> R Ok(()) } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn shell_zsh_fork_skill_exact_mode_replaces_turn_sandbox() -> Result<()> { + skip_if_no_network!(Ok(())); + + let Some(runtime) = zsh_fork_runtime("zsh-fork exact skill sandbox test")? else { + return Ok(()); + }; + + let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?; + let allowed_dir = outside_dir.path().join("allowed-output"); + let blocked_dir = outside_dir.path().join("blocked-output"); + fs::create_dir_all(&allowed_dir)?; + fs::create_dir_all(&blocked_dir)?; + + let allowed_path = allowed_dir.join("allowed.txt"); + let blocked_path = blocked_dir.join("blocked.txt"); + let allowed_path_quoted = shlex::try_join([allowed_path.to_string_lossy().as_ref()])?; + let blocked_path_quoted = shlex::try_join([blocked_path.to_string_lossy().as_ref()])?; + let script_contents = format!( + "#!/bin/sh\nprintf '%s' allowed > {allowed_path_quoted}\ncat {allowed_path_quoted}\nprintf '%s' forbidden > {blocked_path_quoted}\nif [ -f {blocked_path_quoted} ]; then echo blocked-created; fi\n" + ); + let allowed_dir_for_hook = allowed_dir.clone(); + let allowed_path_for_hook = allowed_path.clone(); + let blocked_path_for_hook = blocked_path.clone(); + let script_contents_for_hook = script_contents.clone(); + + let permissions_yaml = format!( + "permissions:\n mode: exact\n file_system:\n write:\n - \"{}\"\n", + allowed_dir.display() + ); + + let server = start_mock_server().await; + let test = build_zsh_fork_test( + &server, + runtime, + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + move |home| { + let _ = fs::remove_file(&allowed_path_for_hook); + let _ = fs::remove_file(&blocked_path_for_hook); + fs::create_dir_all(&allowed_dir_for_hook).unwrap(); + fs::create_dir_all(blocked_path_for_hook.parent().unwrap()).unwrap(); + write_skill_with_shell_script_contents( + home, + "mbolin-test-skill", + "sandboxed.sh", + &script_contents_for_hook, + ) + .unwrap(); + write_skill_metadata(home, "mbolin-test-skill", &permissions_yaml).unwrap(); + }, + ) + .await?; + + let (script_path_str, command) = skill_script_command(&test, "sandboxed.sh")?; + + let first_call_id = "zsh-fork-skill-exact-1"; + let first_arguments = shell_command_arguments(&command)?; + let first_mocks = mount_function_call_agent_response( + &server, + first_call_id, + &first_arguments, + "shell_command", + ) + .await; + + submit_turn_with_policies( + &test, + "use $mbolin-test-skill", + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + ) + .await?; + + let maybe_approval = wait_for_exec_approval_request(&test).await; + let approval = match maybe_approval { + Some(approval) => approval, + None => panic!("expected exec approval request before completion"), + }; + assert_eq!(approval.call_id, first_call_id); + assert_eq!(approval.command, vec![script_path_str.clone()]); + assert_eq!( + approval.reason.as_deref(), + Some("Use the skill's declared sandbox exactly; do not merge it with the current sandbox.") + ); + assert_eq!( + approval.additional_permissions, + Some(PermissionProfile { + file_system: Some(FileSystemPermissions { + read: None, + write: Some(vec![absolute_path(&allowed_dir)]), + }), + ..Default::default() + }) + ); + + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::ApprovedForSession, + }) + .await?; + + wait_for_turn_complete(&test).await; + + let first_output = first_mocks + .completion + .single_request() + .function_call_output(first_call_id)["output"] + .as_str() + .unwrap_or_default() + .to_string(); + assert!( + first_output.contains("allowed"), + "expected exact skill sandbox to permit writes to the approved folder, got output: {first_output:?}" + ); + assert_eq!(fs::read_to_string(&allowed_path)?, "allowed"); + assert!( + !blocked_path.exists(), + "exact mode should replace the full-access turn sandbox instead of merging it" + ); + assert!( + !first_output.contains("blocked-created"), + "blocked path should not have been created in exact mode: {first_output:?}" + ); + + let second_call_id = "zsh-fork-skill-exact-2"; + let second_arguments = shell_command_arguments(&command)?; + let second_mocks = mount_function_call_agent_response( + &server, + second_call_id, + &second_arguments, + "shell_command", + ) + .await; + + let _ = fs::remove_file(&allowed_path); + let _ = fs::remove_file(&blocked_path); + + submit_turn_with_policies( + &test, + "use $mbolin-test-skill", + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + ) + .await?; + + let cached_approval = wait_for_exec_approval_request(&test).await; + assert!( + cached_approval.is_none(), + "expected second run to reuse the cached session approval" + ); + + let second_output = second_mocks + .completion + .single_request() + .function_call_output(second_call_id)["output"] + .as_str() + .unwrap_or_default() + .to_string(); + assert!( + second_output.contains("allowed"), + "expected cached exact skill approval to retain the exact skill sandbox, got output: {second_output:?}" + ); + assert_eq!(fs::read_to_string(&allowed_path)?, "allowed"); + assert!( + !blocked_path.exists(), + "cached session approval should continue to replace the turn sandbox in exact mode" + ); + assert!( + !second_output.contains("blocked-created"), + "blocked path should not have been created after cached exact approval: {second_output:?}" + ); + + Ok(()) +} + /// This stays narrow on purpose: the important check is that `WorkspaceWrite` /// continues to deny writes outside the workspace even under `zsh-fork`. #[cfg(unix)] diff --git a/codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md b/codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md index 90f9e8e863..193e829367 100644 --- a/codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md +++ b/codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md @@ -23,6 +23,12 @@ dependencies: policy: allow_implicit_invocation: true + +permissions: + mode: "merge" + file_system: + write: + - "/absolute/path/for/skill-output" ``` ## Field descriptions and constraints @@ -47,3 +53,10 @@ Top-level constraints: - `policy.allow_implicit_invocation`: When false, the skill is not injected into the model context by default, but can still be invoked explicitly via `$skill`. Defaults to true. +- `permissions.mode`: Optional sandbox behavior for skill script approvals. + `merge` is the default and adds the declared permissions to the current turn sandbox. + `exact` uses only the sandbox compiled from the declared permissions instead of merging. +- `permissions.network`: Optional network access hint for the skill permission profile. +- `permissions.file_system.read`: Optional absolute paths the skill may read. +- `permissions.file_system.write`: Optional absolute paths the skill may write. +- `permissions.macos`: Optional macOS-specific permission hints for skill execution.