From 5b6911cb1beef5da474e6c55e9304e42e10febcf Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Fri, 13 Feb 2026 17:43:44 -0800 Subject: [PATCH] feat(skills): add permission profiles from openai.yaml metadata (#11658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This PR adds support for skill-level permissions in .codex/openai.yaml and wires that through the skill loading pipeline. ## What’s included 1. Added a new permissions section for skills (network, filesystem, and macOS-related access). 2. Implemented permission parsing/normalization and translation into runtime permission profiles. 3. Threaded the new permission profile through SkillMetadata and loader flow. ## Follow-up A follow-up PR will connect these permission profiles to actual sandbox enforcement and add user approval prompts for executing binaries/scripts from skill directories. ## Example `openai.yaml` snippet: ``` permissions: network: true fs_read: - "./data" - "./data" fs_write: - "./output" macos_preferences: "readwrite" macos_automation: - "com.apple.Notes" macos_accessibility: true macos_calendar: true ``` compiled skill permission profile metadata (macOS): ``` SkillPermissionProfile { sandbox_policy: SandboxPolicy::WorkspaceWrite { writable_roots: vec![ AbsolutePathBuf::try_from("/ABS/PATH/TO/SKILL/output").unwrap(), ], read_only_access: ReadOnlyAccess::Restricted { include_platform_defaults: true, readable_roots: vec![ AbsolutePathBuf::try_from("/ABS/PATH/TO/SKILL/data").unwrap(), ], }, network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, }, // Truncated for readability; actual generated profile is longer. macos_seatbelt_permission_file: r#" (allow user-preference-write) (allow appleevent-send (appleevent-destination "com.apple.Notes")) (allow mach-lookup (global-name "com.apple.axserver")) (allow mach-lookup (global-name "com.apple.CalendarAgent")) ... "#.to_string(), ``` --- codex-rs/core/src/config/mod.rs | 12 + codex-rs/core/src/mcp/skill_dependencies.rs | 1 + codex-rs/core/src/seatbelt.rs | 10 +- codex-rs/core/src/seatbelt_permissions.rs | 10 +- codex-rs/core/src/skills/injection.rs | 1 + codex-rs/core/src/skills/loader.rs | 260 +++++++++- codex-rs/core/src/skills/mod.rs | 1 + codex-rs/core/src/skills/model.rs | 5 +- codex-rs/core/src/skills/permissions.rs | 510 ++++++++++++++++++++ codex-rs/tui/src/bottom_pane/mod.rs | 1 + codex-rs/tui/src/chatwidget/skills.rs | 1 + codex-rs/tui/src/chatwidget/tests.rs | 2 + 12 files changed, 797 insertions(+), 17 deletions(-) create mode 100644 codex-rs/core/src/skills/permissions.rs diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 5cfdcf044..91ab7e5b7 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -49,6 +49,8 @@ use crate::project_doc::LOCAL_PROJECT_DOC_FILENAME; use crate::protocol::AskForApproval; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; +#[cfg(target_os = "macos")] +use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions; use crate::windows_sandbox::WindowsSandboxLevelExt; use crate::windows_sandbox::resolve_windows_sandbox_mode; use codex_app_server_protocol::Tools; @@ -78,6 +80,8 @@ use std::path::Path; use std::path::PathBuf; #[cfg(test)] use tempfile::tempdir; +#[cfg(not(target_os = "macos"))] +type MacOsSeatbeltProfileExtensions = (); use crate::config::profile::ConfigProfile; use toml::Value as TomlValue; @@ -133,6 +137,9 @@ pub struct Permissions { /// Effective Windows sandbox mode derived from `[windows].sandbox` or /// legacy feature keys. pub windows_sandbox_mode: Option, + /// Optional macOS seatbelt extension profile used to extend default + /// seatbelt permissions when running under seatbelt. + pub macos_seatbelt_profile_extensions: Option, } /// Application configuration loaded from disk and merged with overrides. @@ -1745,6 +1752,7 @@ impl Config { network, shell_environment_policy, windows_sandbox_mode, + macos_seatbelt_profile_extensions: None, }, enforce_residency: enforce_residency.value, did_user_set_custom_approval_policy_or_sandbox_mode, @@ -4084,6 +4092,7 @@ model_verbosity = "high" network: None, shell_environment_policy: ShellEnvironmentPolicy::default(), windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: None, }, enforce_residency: Constrained::allow_any(None), did_user_set_custom_approval_policy_or_sandbox_mode: true, @@ -4194,6 +4203,7 @@ model_verbosity = "high" network: None, shell_environment_policy: ShellEnvironmentPolicy::default(), windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: None, }, enforce_residency: Constrained::allow_any(None), did_user_set_custom_approval_policy_or_sandbox_mode: true, @@ -4302,6 +4312,7 @@ model_verbosity = "high" network: None, shell_environment_policy: ShellEnvironmentPolicy::default(), windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: None, }, enforce_residency: Constrained::allow_any(None), did_user_set_custom_approval_policy_or_sandbox_mode: true, @@ -4396,6 +4407,7 @@ model_verbosity = "high" network: None, shell_environment_policy: ShellEnvironmentPolicy::default(), windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: None, }, enforce_residency: Constrained::allow_any(None), did_user_set_custom_approval_policy_or_sandbox_mode: true, diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index e2817d5a4..0e6d61055 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -432,6 +432,7 @@ mod tests { interface: None, dependencies: Some(SkillDependencies { tools }), policy: None, + permissions: None, path: PathBuf::from("skill"), scope: SkillScope::User, } diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 29c05e018..7f738797b 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -14,7 +14,6 @@ use tokio::process::Child; use url::Url; use crate::protocol::SandboxPolicy; -use crate::seatbelt_permissions::MacOsPreferencesPermission; use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions; use crate::seatbelt_permissions::build_seatbelt_extensions; use crate::spawn::CODEX_SANDBOX_ENV_VAR; @@ -299,10 +298,7 @@ pub(crate) fn create_seatbelt_command_args_with_extensions( let seatbelt_extensions = extensions.map_or_else( || { // Backward-compatibility default when no extension profile is provided. - build_seatbelt_extensions(&MacOsSeatbeltProfileExtensions { - macos_preferences: MacOsPreferencesPermission::ReadOnly, - ..Default::default() - }) + build_seatbelt_extensions(&MacOsSeatbeltProfileExtensions::default()) }, build_seatbelt_extensions, ); @@ -480,7 +476,7 @@ mod tests { } #[test] - fn seatbelt_args_omit_macos_extensions_when_profile_is_empty() { + fn seatbelt_args_default_extension_profile_keeps_preferences_read_access() { let cwd = std::env::temp_dir(); let args = create_seatbelt_command_args_with_extensions( vec!["echo".to_string(), "ok".to_string()], @@ -494,7 +490,7 @@ mod tests { assert!(!policy.contains("appleevent-send")); assert!(!policy.contains("com.apple.axserver")); assert!(!policy.contains("com.apple.CalendarAgent")); - assert!(!policy.contains("user-preference-read")); + assert!(policy.contains("(allow user-preference-read)")); assert!(!policy.contains("user-preference-write")); } diff --git a/codex-rs/core/src/seatbelt_permissions.rs b/codex-rs/core/src/seatbelt_permissions.rs index 29313d7a3..aacc3a778 100644 --- a/codex-rs/core/src/seatbelt_permissions.rs +++ b/codex-rs/core/src/seatbelt_permissions.rs @@ -6,10 +6,12 @@ use std::path::PathBuf; #[allow(dead_code)] #[derive(Debug, Clone, PartialEq, Eq, Default)] pub enum MacOsPreferencesPermission { + // IMPORTANT: ReadOnly needs to be the default because it's the security-sensitive default. + // it's important for allowing cf prefs to work. #[default] - None, ReadOnly, ReadWrite, + None, } #[allow(dead_code)] @@ -164,7 +166,6 @@ mod tests { use super::MacOsPreferencesPermission; use super::MacOsSeatbeltProfileExtensions; use super::build_seatbelt_extensions; - use pretty_assertions::assert_eq; #[test] fn preferences_read_only_emits_read_clauses_only() { @@ -239,8 +240,9 @@ mod tests { } #[test] - fn empty_extensions_emit_empty_policy() { + fn default_extensions_emit_preferences_read_only_policy() { let policy = build_seatbelt_extensions(&MacOsSeatbeltProfileExtensions::default()); - assert_eq!(policy.policy, ""); + assert!(policy.policy.contains("(allow user-preference-read)")); + assert!(!policy.policy.contains("(allow user-preference-write)")); } } diff --git a/codex-rs/core/src/skills/injection.rs b/codex-rs/core/src/skills/injection.rs index 660785d71..4dec3fe39 100644 --- a/codex-rs/core/src/skills/injection.rs +++ b/codex-rs/core/src/skills/injection.rs @@ -476,6 +476,7 @@ mod tests { interface: None, dependencies: None, policy: None, + permissions: None, path: PathBuf::from(path), scope: codex_protocol::protocol::SkillScope::User, } diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index 6821d1b8f..e00bd4297 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -1,4 +1,5 @@ use crate::config::Config; +use crate::config::Permissions; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigLayerStackOrdering; use crate::config_loader::default_project_root_markers; @@ -11,6 +12,8 @@ use crate::skills::model::SkillLoadOutcome; use crate::skills::model::SkillMetadata; use crate::skills::model::SkillPolicy; use crate::skills::model::SkillToolDependency; +use crate::skills::permissions::SkillManifestPermissions; +use crate::skills::permissions::compile_permission_profile; use crate::skills::system::system_cache_root_dir; use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::protocol::SkillScope; @@ -50,6 +53,8 @@ struct SkillMetadataFile { dependencies: Option, #[serde(default)] policy: Option, + #[serde(default)] + permissions: Option, } #[derive(Debug, Default, Deserialize)] @@ -490,7 +495,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result Result, Option, Option, + Option, ) { // Fail open: optional metadata should not block loading SKILL.md. let Some(skill_dir) = skill_path.parent() else { - return (None, None, None); + return (None, None, None, None); }; let metadata_path = skill_dir .join(SKILLS_METADATA_DIR) .join(SKILLS_METADATA_FILENAME); if !metadata_path.exists() { - return (None, None, None); + return (None, None, None, None); } let contents = match fs::read_to_string(&metadata_path) { @@ -542,7 +549,7 @@ fn load_skill_metadata( path = metadata_path.display(), label = SKILLS_METADATA_FILENAME ); - return (None, None, None); + return (None, None, None, None); } }; @@ -554,7 +561,7 @@ fn load_skill_metadata( path = metadata_path.display(), label = SKILLS_METADATA_FILENAME ); - return (None, None, None); + return (None, None, None, None); } }; @@ -562,12 +569,14 @@ fn load_skill_metadata( interface, dependencies, policy, + permissions, } = parsed; ( resolve_interface(interface, skill_dir), resolve_dependencies(dependencies), resolve_policy(policy), + compile_permission_profile(skill_dir, permissions), ) } @@ -801,7 +810,9 @@ mod tests { use crate::config::ConfigBuilder; use crate::config::ConfigOverrides; use crate::config::ConfigToml; + use crate::config::Constrained; use crate::config::ProjectConfig; + use crate::config::types::ShellEnvironmentPolicy; use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; @@ -1021,6 +1032,7 @@ mod tests { interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1168,6 +1180,7 @@ mod tests { ], }), policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1223,6 +1236,7 @@ interface: }), dependencies: None, policy: None, + permissions: None, path: normalized(skill_path.as_path()), scope: SkillScope::User, }] @@ -1295,6 +1309,219 @@ policy: {} ); } + #[tokio::test] + async fn loads_skill_permissions_from_yaml() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "permissions-skill", "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: + network: true + file_system: + read: + - "./data" + - "./data" + write: + - "./output" +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&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()); + #[cfg(not(target_os = "macos"))] + let macos_seatbelt_profile_extensions = None; + assert_eq!( + outcome.skills[0].permissions, + Some(Permissions { + approval_policy: Constrained::allow_any(crate::protocol::AskForApproval::Never), + sandbox_policy: Constrained::allow_any( + crate::protocol::SandboxPolicy::WorkspaceWrite { + writable_roots: vec![ + AbsolutePathBuf::try_from(normalized( + skill_dir.join("output").as_path(), + )) + .expect("absolute output path") + ], + read_only_access: crate::protocol::ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: vec![ + AbsolutePathBuf::try_from(normalized( + skill_dir.join("data").as_path(), + )) + .expect("absolute data path") + ], + }, + network_access: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + } + ), + network: None, + shell_environment_policy: ShellEnvironmentPolicy::default(), + windows_sandbox_mode: None, + macos_seatbelt_profile_extensions, + }) + ); + } + + #[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(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); + #[cfg(target_os = "macos")] + let expected = Some(Permissions { + approval_policy: Constrained::allow_any(crate::protocol::AskForApproval::Never), + sandbox_policy: Constrained::allow_any( + crate::protocol::SandboxPolicy::new_read_only_policy(), + ), + network: None, + shell_environment_policy: ShellEnvironmentPolicy::default(), + windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: Some( + crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default(), + ), + }); + #[cfg(not(target_os = "macos"))] + let expected = Some(Permissions { + approval_policy: Constrained::allow_any(crate::protocol::AskForApproval::Never), + sandbox_policy: Constrained::allow_any( + crate::protocol::SandboxPolicy::new_read_only_policy(), + ), + network: None, + shell_environment_policy: ShellEnvironmentPolicy::default(), + windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: None, + }); + assert_eq!(outcome.skills[0].permissions, expected); + } + + #[cfg(target_os = "macos")] + #[tokio::test] + async fn loads_skill_macos_permissions_from_yaml() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "permissions-macos", "from yaml"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_metadata_at( + skill_dir, + r#" +permissions: + macos: + preferences: "readwrite" + automations: + - "com.apple.Notes" + accessibility: true + calendar: true +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); + let profile = outcome.skills[0] + .permissions + .as_ref() + .expect("permission 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(not(target_os = "macos"))] + #[tokio::test] + async fn loads_skill_macos_permissions_from_yaml_non_macos_does_not_create_profile() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "permissions-macos", "from yaml"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_metadata_at( + skill_dir, + r#" +permissions: + macos: + preferences: "readwrite" + automations: + - "com.apple.Notes" + accessibility: true + calendar: true +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); + assert_eq!( + outcome.skills[0].permissions, + Some(Permissions { + approval_policy: Constrained::allow_any(crate::protocol::AskForApproval::Never), + sandbox_policy: Constrained::allow_any( + crate::protocol::SandboxPolicy::new_read_only_policy(), + ), + network: None, + shell_environment_policy: ShellEnvironmentPolicy::default(), + windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: None, + }) + ); + } + #[tokio::test] async fn accepts_icon_paths_under_assets_dir() { let codex_home = tempfile::tempdir().expect("tempdir"); @@ -1339,6 +1566,7 @@ policy: {} }), dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1379,6 +1607,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1432,6 +1661,7 @@ policy: {} }), dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1473,6 +1703,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1517,6 +1748,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&shared_skill_path), scope: SkillScope::User, }] @@ -1577,6 +1809,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1613,6 +1846,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&shared_skill_path), scope: SkillScope::Admin, }] @@ -1653,6 +1887,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&linked_skill_path), scope: SkillScope::Repo, }] @@ -1720,6 +1955,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&within_depth_path), scope: SkillScope::User, }] @@ -1747,6 +1983,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1778,6 +2015,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1890,6 +2128,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1925,6 +2164,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1978,6 +2218,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&nested_skill_path), scope: SkillScope::Repo, }, @@ -1988,6 +2229,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&root_skill_path), scope: SkillScope::Repo, }, @@ -2027,6 +2269,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -2064,6 +2307,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -2105,6 +2349,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&repo_skill_path), scope: SkillScope::Repo, }, @@ -2115,6 +2360,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&user_skill_path), scope: SkillScope::User, }, @@ -2179,6 +2425,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: first_path, scope: SkillScope::Repo, }, @@ -2189,6 +2436,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: second_path, scope: SkillScope::Repo, }, @@ -2260,6 +2508,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -2318,6 +2567,7 @@ policy: {} interface: None, dependencies: None, policy: None, + permissions: None, path: normalized(&skill_path), scope: SkillScope::System, }] diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core/src/skills/mod.rs index 17405129e..18f7180f9 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core/src/skills/mod.rs @@ -3,6 +3,7 @@ pub mod injection; 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/model.rs b/codex-rs/core/src/skills/model.rs index bc75b7e36..ac642db43 100644 --- a/codex-rs/core/src/skills/model.rs +++ b/codex-rs/core/src/skills/model.rs @@ -1,9 +1,10 @@ use std::collections::HashSet; use std::path::PathBuf; +use crate::config::Permissions; use codex_protocol::protocol::SkillScope; -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq)] pub struct SkillMetadata { pub name: String, pub description: String, @@ -11,6 +12,8 @@ pub struct SkillMetadata { pub interface: Option, pub dependencies: Option, pub policy: Option, + // This is an experimental field. + pub permissions: Option, pub path: PathBuf, pub scope: SkillScope, } diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs new file mode 100644 index 000000000..1cf18e105 --- /dev/null +++ b/codex-rs/core/src/skills/permissions.rs @@ -0,0 +1,510 @@ +use std::collections::HashSet; +use std::path::Component; +use std::path::Path; +use std::path::PathBuf; + +use codex_utils_absolute_path::AbsolutePathBuf; +use dirs::home_dir; +use dunce::canonicalize as canonicalize_path; +use serde::Deserialize; +use tracing::warn; + +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; +#[cfg(target_os = "macos")] +use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions; +#[cfg(not(target_os = "macos"))] +type MacOsSeatbeltProfileExtensions = (); + +#[derive(Debug, Clone, PartialEq, Eq, Default, Deserialize)] +pub(crate) struct SkillManifestPermissions { + #[serde(default)] + pub(crate) network: bool, + #[serde(default)] + pub(crate) file_system: SkillManifestFileSystemPermissions, + #[serde(default)] + pub(crate) macos: SkillManifestMacOsPermissions, +} + +#[derive(Debug, Clone, PartialEq, Eq, Default, Deserialize)] +pub(crate) struct SkillManifestFileSystemPermissions { + #[serde(default)] + pub(crate) read: Vec, + #[serde(default)] + pub(crate) write: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Default, Deserialize)] +pub(crate) struct SkillManifestMacOsPermissions { + #[serde(default)] + pub(crate) preferences: Option, + #[serde(default)] + pub(crate) automations: Option, + #[serde(default)] + pub(crate) accessibility: bool, + #[serde(default)] + pub(crate) calendar: bool, +} + +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(untagged)] +pub(crate) enum MacOsPreferencesValue { + Bool(bool), + Mode(String), +} + +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(untagged)] +pub(crate) enum MacOsAutomationValue { + Bool(bool), + BundleIds(Vec), +} + +pub(crate) fn compile_permission_profile( + skill_dir: &Path, + permissions: Option, +) -> Option { + let permissions = permissions?; + let fs_read = normalize_permission_paths( + skill_dir, + &permissions.file_system.read, + "permissions.file_system.read", + ); + let fs_write = normalize_permission_paths( + skill_dir, + &permissions.file_system.write, + "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: permissions.network, + 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, + }, + } + } else { + // Default sandbox policy + SandboxPolicy::new_read_only_policy() + }; + let macos_seatbelt_profile_extensions = + build_macos_seatbelt_profile_extensions(&permissions.macos); + + Some(Permissions { + approval_policy: Constrained::allow_any(AskForApproval::Never), + sandbox_policy: Constrained::allow_any(sandbox_policy), + network: None, + shell_environment_policy: ShellEnvironmentPolicy::default(), + windows_sandbox_mode: None, + macos_seatbelt_profile_extensions, + }) +} + +fn normalize_permission_paths( + skill_dir: &Path, + values: &[String], + field: &str, +) -> Vec { + let mut paths = Vec::new(); + let mut seen = HashSet::new(); + + for value in values { + let Some(path) = normalize_permission_path(skill_dir, value, field) else { + continue; + }; + if seen.insert(path.clone()) { + paths.push(path); + } + } + + paths +} + +fn normalize_permission_path( + skill_dir: &Path, + value: &str, + field: &str, +) -> Option { + let trimmed = value.trim(); + if trimmed.is_empty() { + warn!("ignoring {field}: value is empty"); + return None; + } + + let expanded = expand_home(trimmed); + let path = PathBuf::from(expanded); + let absolute = if path.is_absolute() { + path + } else { + skill_dir.join(path) + }; + let normalized = normalize_lexically(&absolute); + let canonicalized = canonicalize_path(&normalized).unwrap_or(normalized); + match AbsolutePathBuf::from_absolute_path(&canonicalized) { + Ok(path) => Some(path), + Err(error) => { + warn!("ignoring {field}: expected absolute path, got {canonicalized:?}: {error}"); + None + } + } +} + +fn expand_home(path: &str) -> String { + if path == "~" { + if let Some(home) = home_dir() { + return home.to_string_lossy().to_string(); + } + return path.to_string(); + } + if let Some(rest) = path.strip_prefix("~/") + && let Some(home) = home_dir() + { + return home.join(rest).to_string_lossy().to_string(); + } + path.to_string() +} + +#[cfg(target_os = "macos")] +fn build_macos_seatbelt_profile_extensions( + permissions: &SkillManifestMacOsPermissions, +) -> 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, + macos_calendar: permissions.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(not(target_os = "macos"))] +fn build_macos_seatbelt_profile_extensions( + _: &SkillManifestMacOsPermissions, +) -> Option { + None +} + +fn normalize_lexically(path: &Path) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + normalized.pop(); + } + Component::RootDir | Component::Prefix(_) | Component::Normal(_) => { + normalized.push(component.as_os_str()); + } + } + } + normalized +} + +#[cfg(test)] +mod tests { + use super::SkillManifestFileSystemPermissions; + #[cfg(target_os = "macos")] + use super::SkillManifestMacOsPermissions; + use super::SkillManifestPermissions; + 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_utils_absolute_path::AbsolutePathBuf; + use pretty_assertions::assert_eq; + use std::fs; + + #[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( + &skill_dir, + Some(SkillManifestPermissions { + network: true, + file_system: SkillManifestFileSystemPermissions { + read: vec![ + "./data".to_string(), + "./data".to_string(), + "scripts/../data".to_string(), + ], + write: vec!["./output".to_string()], + }, + ..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, + 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(&skill_dir, 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( + &skill_dir, + Some(SkillManifestPermissions { + network: true, + ..Default::default() + }), + ) + .expect("profile"); + + assert_eq!( + profile, + Permissions { + approval_policy: Constrained::allow_any(AskForApproval::Never), + sandbox_policy: Constrained::allow_any(SandboxPolicy::new_read_only_policy()), + network: None, + 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( + &skill_dir, + Some(SkillManifestPermissions { + network: true, + file_system: SkillManifestFileSystemPermissions { + read: vec!["./data".to_string()], + write: 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: None, + 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( + &skill_dir, + Some(SkillManifestPermissions { + macos: SkillManifestMacOsPermissions { + preferences: Some(super::MacOsPreferencesValue::Mode("readwrite".to_string())), + automations: Some(super::MacOsAutomationValue::BundleIds(vec![ + "com.apple.Notes".to_string(), + ])), + accessibility: true, + calendar: 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(&skill_dir, Some(SkillManifestPermissions::default())) + .expect("profile"); + + assert_eq!( + profile.macos_seatbelt_profile_extensions, + Some(crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default()) + ); + } +} diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 4e4dabe5c..e36c2f3c4 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -1402,6 +1402,7 @@ mod tests { interface: None, dependencies: None, policy: None, + permissions: None, path: PathBuf::from("test-skill"), scope: SkillScope::User, }]), diff --git a/codex-rs/tui/src/chatwidget/skills.rs b/codex-rs/tui/src/chatwidget/skills.rs index 6ec4aefa6..e7fadde1a 100644 --- a/codex-rs/tui/src/chatwidget/skills.rs +++ b/codex-rs/tui/src/chatwidget/skills.rs @@ -190,6 +190,7 @@ fn protocol_skill_to_core(skill: &ProtocolSkillMetadata) -> SkillMetadata { .collect(), }), policy: None, + permissions: None, path: skill.path.clone(), scope: skill.scope, } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index cd03b9899..5435913bb 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -874,6 +874,7 @@ async fn submission_prefers_selected_duplicate_skill_path() { interface: None, dependencies: None, policy: None, + permissions: None, path: repo_skill_path, scope: SkillScope::Repo, }, @@ -884,6 +885,7 @@ async fn submission_prefers_selected_duplicate_skill_path() { interface: None, dependencies: None, policy: None, + permissions: None, path: user_skill_path.clone(), scope: SkillScope::User, },