diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 01790fb98f..5d9365460b 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1618,6 +1618,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 4258bf7c2c..bbdba2f129 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -5331,6 +5331,10 @@ ], "description": "Read access granted while running under this policy." }, + "network_access": { + "description": "When set to `true`, outbound network access is allowed. `false` by default.", + "type": "boolean" + }, "type": { "enum": [ "read-only" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 4cf3933e75..70400cafa9 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -11900,6 +11900,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/CommandExecParams.json b/codex-rs/app-server-protocol/schema/json/v2/CommandExecParams.json index 4528b34159..08f1a9a15a 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/CommandExecParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/CommandExecParams.json @@ -89,6 +89,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json index 3e4dca5b1c..41f4c9b7bb 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json @@ -653,6 +653,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json index d9fbc0d02c..416bceb6e0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json @@ -653,6 +653,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json index ee8b384998..25db79cf24 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json @@ -653,6 +653,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json index 0a769c3525..9e749d5b81 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json @@ -214,6 +214,10 @@ "type": "fullAccess" } }, + "networkAccess": { + "default": false, + "type": "boolean" + }, "type": { "enum": [ "readOnly" diff --git a/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts b/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts index 743ad22229..8440fd8043 100644 --- a/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts +++ b/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts @@ -12,7 +12,12 @@ export type SandboxPolicy = { "type": "danger-full-access" } | { "type": "read-o /** * Read access granted while running under this policy. */ -access?: ReadOnlyAccess, } | { "type": "external-sandbox", +access?: ReadOnlyAccess, +/** + * When set to `true`, outbound network access is allowed. `false` by + * default. + */ +network_access?: boolean, } | { "type": "external-sandbox", /** * Whether the external sandbox permits outbound network traffic. */ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/SandboxPolicy.ts b/codex-rs/app-server-protocol/schema/typescript/v2/SandboxPolicy.ts index c81c2642d2..c6780648cf 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/SandboxPolicy.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/SandboxPolicy.ts @@ -5,4 +5,4 @@ import type { AbsolutePathBuf } from "../AbsolutePathBuf"; import type { NetworkAccess } from "./NetworkAccess"; import type { ReadOnlyAccess } from "./ReadOnlyAccess"; -export type SandboxPolicy = { "type": "dangerFullAccess" } | { "type": "readOnly", access: ReadOnlyAccess, } | { "type": "externalSandbox", networkAccess: NetworkAccess, } | { "type": "workspaceWrite", writableRoots: Array, readOnlyAccess: ReadOnlyAccess, networkAccess: boolean, excludeTmpdirEnvVar: boolean, excludeSlashTmp: boolean, }; +export type SandboxPolicy = { "type": "dangerFullAccess" } | { "type": "readOnly", access: ReadOnlyAccess, networkAccess: boolean, } | { "type": "externalSandbox", networkAccess: NetworkAccess, } | { "type": "workspaceWrite", writableRoots: Array, readOnlyAccess: ReadOnlyAccess, networkAccess: boolean, excludeTmpdirEnvVar: boolean, excludeSlashTmp: boolean, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 8f0b37beef..a24dd1a83a 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -950,6 +950,8 @@ pub enum SandboxPolicy { ReadOnly { #[serde(default)] access: ReadOnlyAccess, + #[serde(default)] + network_access: bool, }, #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] @@ -979,11 +981,13 @@ impl SandboxPolicy { SandboxPolicy::DangerFullAccess => { codex_protocol::protocol::SandboxPolicy::DangerFullAccess } - SandboxPolicy::ReadOnly { access } => { - codex_protocol::protocol::SandboxPolicy::ReadOnly { - access: access.to_core(), - } - } + SandboxPolicy::ReadOnly { + access, + network_access, + } => codex_protocol::protocol::SandboxPolicy::ReadOnly { + access: access.to_core(), + network_access: *network_access, + }, SandboxPolicy::ExternalSandbox { network_access } => { codex_protocol::protocol::SandboxPolicy::ExternalSandbox { network_access: match network_access { @@ -1015,11 +1019,13 @@ impl From for SandboxPolicy { codex_protocol::protocol::SandboxPolicy::DangerFullAccess => { SandboxPolicy::DangerFullAccess } - codex_protocol::protocol::SandboxPolicy::ReadOnly { access } => { - SandboxPolicy::ReadOnly { - access: ReadOnlyAccess::from(access), - } - } + codex_protocol::protocol::SandboxPolicy::ReadOnly { + access, + network_access, + } => SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::from(access), + network_access, + }, codex_protocol::protocol::SandboxPolicy::ExternalSandbox { network_access } => { SandboxPolicy::ExternalSandbox { network_access: match network_access { @@ -4278,6 +4284,7 @@ mod tests { include_platform_defaults: false, readable_roots: vec![readable_root.clone()], }, + network_access: true, }; let core_policy = v2_policy.to_core(); @@ -4288,6 +4295,7 @@ mod tests { include_platform_defaults: false, readable_roots: vec![readable_root], }, + network_access: true, } ); @@ -4338,6 +4346,7 @@ mod tests { policy, SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, } ); } diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index d8f06be421..cb1bd136ad 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -609,6 +609,7 @@ fn trigger_zsh_fork_multi_cmd_approval( turn_params.approval_policy = Some(AskForApproval::OnRequest); turn_params.sandbox_policy = Some(SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, }); let turn_response = client.turn_start(turn_params)?; @@ -742,6 +743,7 @@ fn trigger_cmd_approval( Some(AskForApproval::OnRequest), Some(SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, }), dynamic_tools, ) @@ -764,6 +766,7 @@ fn trigger_patch_approval( Some(AskForApproval::OnRequest), Some(SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, }), dynamic_tools, ) diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 9ec4821e8d..93ba92679d 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -573,7 +573,7 @@ Run a standalone command (argv vector) in the server’s sandbox without creatin Notes: - Empty `command` arrays are rejected. -- `sandboxPolicy` accepts the same shape used by `turn/start` (e.g., `dangerFullAccess`, `readOnly`, `workspaceWrite` with flags, `externalSandbox` with `networkAccess` `restricted|enabled`). +- `sandboxPolicy` accepts the same shape used by `turn/start` (e.g., `dangerFullAccess`, `readOnly` with optional `networkAccess`, `workspaceWrite` with flags, `externalSandbox` with `networkAccess` `restricted|enabled`). - When omitted, `timeoutMs` falls back to the server default. ## Events diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index f15bb6ec57..4a1af423f7 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -442,6 +442,7 @@ mod tests { dependencies: Some(SkillDependencies { tools }), policy: None, permission_profile: None, + permissions: None, path_to_skills_md: PathBuf::from("skill"), scope: SkillScope::User, } diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index bbd73749e4..203444eebc 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,15 +218,20 @@ 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, } } - SandboxPolicy::ReadOnly { access } => { + SandboxPolicy::ReadOnly { + access, + network_access, + } => { if extra_writes.is_empty() { SandboxPolicy::ReadOnly { access: merge_read_only_access_with_additional_reads(access, extra_reads), + network_access: *network_access + || additional_permissions.network.unwrap_or(false), } } else { // todo(dylan) - for now, this grants more access than the request. We should restrict this, @@ -238,15 +243,14 @@ fn sandbox_policy_with_additional_permissions( access, extra_reads, ), - network_access: false, + network_access: *network_access + || additional_permissions.network.unwrap_or(false), exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } } } - }; - - Ok(policy) + } } #[derive(Default)] @@ -312,7 +316,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() }; @@ -412,10 +416,13 @@ pub async fn execute_env( #[cfg(test)] mod tests { use super::SandboxManager; + use super::sandbox_policy_with_additional_permissions; use crate::exec::SandboxType; use crate::protocol::SandboxPolicy; use crate::tools::sandboxing::SandboxablePreference; use codex_protocol::config_types::WindowsSandboxLevel; + use codex_protocol::models::PermissionProfile; + use codex_protocol::protocol::ReadOnlyAccess; use pretty_assertions::assert_eq; #[test] @@ -442,4 +449,23 @@ mod tests { ); assert_eq!(sandbox, expected); } + + #[test] + fn read_only_merge_preserves_network_only_permissions() { + let merged = sandbox_policy_with_additional_permissions( + &SandboxPolicy::new_read_only_policy(), + &PermissionProfile { + network: Some(true), + ..Default::default() + }, + ); + + assert_eq!( + merged, + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + network_access: true, + } + ); + } } diff --git a/codex-rs/core/src/skills/injection.rs b/codex-rs/core/src/skills/injection.rs index e293aac59f..9bb3a74781 100644 --- a/codex-rs/core/src/skills/injection.rs +++ b/codex-rs/core/src/skills/injection.rs @@ -483,6 +483,7 @@ mod tests { dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: PathBuf::from(path), scope: codex_protocol::protocol::SkillScope::User, } diff --git a/codex-rs/core/src/skills/invocation_utils.rs b/codex-rs/core/src/skills/invocation_utils.rs index 36bb0c3caa..d2831c4dfb 100644 --- a/codex-rs/core/src/skills/invocation_utils.rs +++ b/codex-rs/core/src/skills/invocation_utils.rs @@ -253,6 +253,7 @@ mod tests { dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: skill_doc_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 bfa22cf01b..c2f4d59a2f 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -1,3 +1,4 @@ +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::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; @@ -58,7 +61,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)] @@ -67,6 +86,7 @@ struct LoadedSkillMetadata { dependencies: Option, policy: Option, permission_profile: Option, + permissions: Option, } #[derive(Debug, Default, Deserialize)] @@ -525,6 +545,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result Result LoadedSkillMetadata { policy, permissions, } = parsed; + 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(Some(permissions.profile)) + } else { + None + } + }); LoadedSkillMetadata { interface: resolve_interface(interface, skill_dir), dependencies: resolve_dependencies(dependencies), policy: resolve_policy(policy), - permission_profile: permissions.filter(|profile| !profile.is_empty()), + permission_profile, + permissions: exact_permissions, } } @@ -856,7 +890,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; @@ -1090,6 +1126,7 @@ mod tests { dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -1247,6 +1284,7 @@ mod tests { }), policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -1303,6 +1341,7 @@ interface: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(skill_path.as_path()), scope: SkillScope::User, }] @@ -1422,6 +1461,7 @@ permissions: macos: None, }) ); + assert_eq!(outcome.skills[0].permissions, None); } #[tokio::test] @@ -1447,6 +1487,131 @@ permissions: {} ); 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()); + #[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, + allow_login_shell: true, + shell_environment_policy: ShellEnvironmentPolicy::default(), + windows_sandbox_mode: None, + macos_seatbelt_profile_extensions, + }) + ); + } + + #[tokio::test] + 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-exact", "from yaml"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_metadata_at( + skill_dir, + r#" +permissions: + mode: exact +"#, + ); + + 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 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, + allow_login_shell: true, + 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, + allow_login_shell: true, + shell_environment_policy: ShellEnvironmentPolicy::default(), + windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: None, + }); + assert_eq!(outcome.skills[0].permission_profile, None); + assert_eq!(outcome.skills[0].permissions, expected); } #[cfg(target_os = "macos")] @@ -1460,6 +1625,7 @@ permissions: {} skill_dir, r#" permissions: + mode: exact macos: preferences: "readwrite" automations: @@ -1478,21 +1644,24 @@ permissions: outcome.errors ); assert_eq!(outcome.skills.len(), 1); + let profile = outcome.skills[0] + .permissions + .as_ref() + .expect("permission profile"); assert_eq!( - outcome.skills[0].permission_profile, - Some(PermissionProfile { - macos: Some(codex_protocol::models::MacOsPermissions { - preferences: Some(codex_protocol::models::MacOsPreferencesValue::Mode( - "readwrite".to_string(), - ),), - automations: Some(codex_protocol::models::MacOsAutomationValue::BundleIds( - vec!["com.apple.Notes".to_string()], - )), - accessibility: Some(true), - calendar: Some(true), - }), - ..Default::default() - }) + 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, + } + ) ); } @@ -1507,6 +1676,7 @@ permissions: skill_dir, r#" permissions: + mode: exact macos: preferences: "readwrite" automations: @@ -1526,19 +1696,17 @@ permissions: ); assert_eq!(outcome.skills.len(), 1); assert_eq!( - outcome.skills[0].permission_profile, - Some(PermissionProfile { - macos: Some(codex_protocol::models::MacOsPermissions { - preferences: Some(codex_protocol::models::MacOsPreferencesValue::Mode( - "readwrite".to_string(), - )), - automations: Some(codex_protocol::models::MacOsAutomationValue::BundleIds( - vec!["com.apple.Notes".to_string()], - )), - accessibility: Some(true), - calendar: Some(true), - }), - ..Default::default() + 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, + allow_login_shell: true, + shell_environment_policy: ShellEnvironmentPolicy::default(), + windows_sandbox_mode: None, + macos_seatbelt_profile_extensions: None, }) ); } @@ -1588,6 +1756,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -1629,6 +1798,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -1683,6 +1853,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -1725,6 +1896,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -1770,6 +1942,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&shared_skill_path), scope: SkillScope::User, }] @@ -1831,6 +2004,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -1868,6 +2042,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&shared_skill_path), scope: SkillScope::Admin, }] @@ -1909,6 +2084,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&linked_skill_path), scope: SkillScope::Repo, }] @@ -1977,6 +2153,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&within_depth_path), scope: SkillScope::User, }] @@ -2005,6 +2182,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -2038,6 +2216,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -2080,6 +2259,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -2112,6 +2292,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, }] @@ -2225,6 +2406,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -2261,6 +2443,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -2315,6 +2498,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&nested_skill_path), scope: SkillScope::Repo, }, @@ -2326,6 +2510,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&root_skill_path), scope: SkillScope::Repo, }, @@ -2366,6 +2551,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -2404,6 +2590,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -2446,6 +2633,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&repo_skill_path), scope: SkillScope::Repo, }, @@ -2457,6 +2645,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&user_skill_path), scope: SkillScope::User, }, @@ -2522,6 +2711,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: first_path, scope: SkillScope::Repo, }, @@ -2533,6 +2723,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: second_path, scope: SkillScope::Repo, }, @@ -2605,6 +2796,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -2664,6 +2856,7 @@ permissions: dependencies: None, policy: None, permission_profile: None, + permissions: None, path_to_skills_md: normalized(&skill_path), scope: SkillScope::System, }] diff --git a/codex-rs/core/src/skills/model.rs b/codex-rs/core/src/skills/model.rs index 525ea28da3..ddb1dd2638 100644 --- a/codex-rs/core/src/skills/model.rs +++ b/codex-rs/core/src/skills/model.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; use std::path::PathBuf; use std::sync::Arc; +use crate::config::Permissions; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::SkillScope; @@ -15,6 +16,8 @@ pub struct SkillMetadata { pub dependencies: Option, pub policy: Option, pub permission_profile: Option, + // This is an experimental field. + pub permissions: Option, /// Path to the SKILLS.md file that declares this skill. pub path_to_skills_md: PathBuf, pub scope: SkillScope, diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs index 1d0e6c82ad..d0d08a63b8 100644 --- a/codex-rs/core/src/skills/permissions.rs +++ b/codex-rs/core/src/skills/permissions.rs @@ -1,21 +1,22 @@ -#[cfg(any(unix, test))] +#[cfg(target_os = "macos")] +use std::collections::BTreeSet; use std::collections::HashSet; +#[cfg(target_os = "macos")] +use codex_protocol::models::MacOsAutomationPermission; #[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::MacOsPreferencesPermission; +#[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))] @@ -39,11 +40,12 @@ use crate::protocol::SandboxPolicy; pub(crate) fn compile_permission_profile( 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(), @@ -68,16 +70,18 @@ pub(crate) fn compile_permission_profile( 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() + SandboxPolicy::ReadOnly { + access: if fs_read.is_empty() { + ReadOnlyAccess::FullAccess + } else { + ReadOnlyAccess::Restricted { + include_platform_defaults: true, + readable_roots: fs_read, + } + }, + network_access: network.unwrap_or_default(), + } }; let macos_permissions = macos.unwrap_or_default(); let macos_seatbelt_profile_extensions = @@ -94,7 +98,35 @@ pub(crate) fn compile_permission_profile( }) } -#[cfg(any(unix, test))] +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(); @@ -111,7 +143,6 @@ fn normalize_permission_paths(values: &[AbsolutePathBuf], field: &str) -> Vec Option { let canonicalized = canonicalize_path(value.as_path()).unwrap_or_else(|_| value.to_path_buf()); match AbsolutePathBuf::from_absolute_path(&canonicalized) { @@ -146,6 +177,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>, @@ -209,6 +351,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; @@ -320,7 +470,10 @@ mod tests { profile, Permissions { approval_policy: Constrained::allow_any(AskForApproval::Never), - sandbox_policy: Constrained::allow_any(SandboxPolicy::new_read_only_policy()), + sandbox_policy: Constrained::allow_any(SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + network_access: true, + }), network: None, allow_login_shell: true, shell_environment_policy: ShellEnvironmentPolicy::default(), @@ -366,6 +519,7 @@ mod tests { .expect("absolute read path") ], }, + network_access: true, }), network: None, allow_login_shell: true, 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 36f63be4fa..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,9 +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::compile_permission_profile; +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; @@ -227,8 +228,14 @@ impl CoreShellActionProvider { } } - fn skill_escalation_execution(skill: &SkillMetadata) -> EscalationExecution { - compile_permission_profile(skill.permission_profile.clone()) + fn skill_escalation_execution( + skill: &SkillMetadata, + turn_sandbox_policy: &SandboxPolicy, + turn_macos_seatbelt_profile_extensions: Option<&MacOsSeatbeltProfileExtensions>, + ) -> EscalationExecution { + skill + .permissions + .as_ref() .map(|permissions| { EscalationExecution::Permissions(EscalationPermissions::Permissions( EscalatedPermissions { @@ -239,6 +246,23 @@ impl CoreShellActionProvider { }, )) }) + .or_else(|| { + 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) } @@ -257,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![ @@ -280,7 +311,7 @@ impl CoreShellActionProvider { approval_id, command, workdir, - None, + reason, None, None, additional_permissions, @@ -462,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)); @@ -487,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 d81cbb5c66..aa9667a737 100644 --- a/codex-rs/core/tests/suite/skill_approval.rs +++ b/codex-rs/core/tests/suite/skill_approval.rs @@ -543,10 +543,10 @@ async fn shell_zsh_fork_skill_with_empty_permissions_inherits_turn_sandbox() -> /// 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(()); }; @@ -654,12 +654,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"), @@ -702,12 +702,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"), @@ -717,6 +717,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/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 67a8b8e604..56c4ce70d1 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -478,6 +478,7 @@ mod tests { .expect("absolute readable root"), ], }, + network_access: false, }; let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); @@ -503,6 +504,7 @@ mod tests { include_platform_defaults: true, readable_roots: Vec::new(), }, + network_access: false, }; // `ReadOnlyAccess::Restricted` always includes `cwd` as a readable diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 3f1c392719..a1181d3b79 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -623,6 +623,11 @@ pub enum SandboxPolicy { skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access" )] access: ReadOnlyAccess, + + /// When set to `true`, outbound network access is allowed. `false` by + /// default. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + network_access: bool, }, /// Indicates the process is already in an external sandbox. Allows full @@ -712,6 +717,7 @@ impl SandboxPolicy { pub fn new_read_only_policy() -> Self { SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, + network_access: false, } } @@ -732,7 +738,7 @@ impl SandboxPolicy { match self { SandboxPolicy::DangerFullAccess => true, SandboxPolicy::ExternalSandbox { .. } => true, - SandboxPolicy::ReadOnly { access } => access.has_full_disk_read_access(), + SandboxPolicy::ReadOnly { access, .. } => access.has_full_disk_read_access(), SandboxPolicy::WorkspaceWrite { read_only_access, .. } => read_only_access.has_full_disk_read_access(), @@ -752,7 +758,7 @@ impl SandboxPolicy { match self { SandboxPolicy::DangerFullAccess => true, SandboxPolicy::ExternalSandbox { network_access } => network_access.is_enabled(), - SandboxPolicy::ReadOnly { .. } => false, + SandboxPolicy::ReadOnly { network_access, .. } => *network_access, SandboxPolicy::WorkspaceWrite { network_access, .. } => *network_access, } } @@ -763,7 +769,7 @@ impl SandboxPolicy { return false; } match self { - SandboxPolicy::ReadOnly { access } => access.include_platform_defaults(), + SandboxPolicy::ReadOnly { access, .. } => access.include_platform_defaults(), SandboxPolicy::WorkspaceWrite { read_only_access, .. } => read_only_access.include_platform_defaults(), @@ -779,7 +785,7 @@ impl SandboxPolicy { pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec { let mut roots = match self { SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => Vec::new(), - SandboxPolicy::ReadOnly { access } => access.get_readable_roots_with_cwd(cwd), + SandboxPolicy::ReadOnly { access, .. } => access.get_readable_roots_with_cwd(cwd), SandboxPolicy::WorkspaceWrite { read_only_access, .. } => { @@ -3502,6 +3508,23 @@ mod tests { Ok(()) } + #[test] + fn serialize_read_only_policy_with_network_access() -> Result<()> { + let policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + network_access: true, + }; + + assert_eq!( + serde_json::to_value(&policy)?, + json!({ + "type": "read-only", + "network_access": true, + }) + ); + Ok(()) + } + #[test] fn vec_u8_as_base64_serialization_and_deserialization() -> Result<()> { let event = ExecCommandOutputDeltaEvent { 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. diff --git a/codex-rs/tui/src/status/card.rs b/codex-rs/tui/src/status/card.rs index a7546230bf..953bc434a3 100644 --- a/codex-rs/tui/src/status/card.rs +++ b/codex-rs/tui/src/status/card.rs @@ -199,6 +199,10 @@ impl StatusHistoryCell { .unwrap_or_else(|| "".to_string()); let sandbox = match config.permissions.sandbox_policy.get() { SandboxPolicy::DangerFullAccess => "danger-full-access".to_string(), + SandboxPolicy::ReadOnly { + network_access: true, + .. + } => "read-only with network access".to_string(), SandboxPolicy::ReadOnly { .. } => "read-only".to_string(), SandboxPolicy::WorkspaceWrite { network_access: true, diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_mentions_read_only_network_access.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_mentions_read_only_network_access.snap new file mode 100644 index 0000000000..7c46a806dd --- /dev/null +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_mentions_read_only_network_access.snap @@ -0,0 +1,20 @@ +--- +source: tui/src/status/tests.rs +expression: sanitized +--- +/status + +╭────────────────────────────────────────────────────────────────────╮ +│ >_ OpenAI Codex (v0.0.0) │ +│ │ +│ Visit https://chatgpt.com/codex/settings/usage for up-to-date │ +│ information on rate limits and credits │ +│ │ +│ Model: gpt-5.3-codex (reasoning none, summaries auto) │ +│ Directory: [[workspace]] │ +│ Permissions: Custom (read-only with network access, on-request) │ +│ Agents.md: │ +│ │ +│ Token usage: 0 total (0 input + 0 output) │ +│ Limits: data not available yet │ +╰────────────────────────────────────────────────────────────────────╯ diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index 16ab60bd72..a381596391 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -236,6 +236,58 @@ async fn status_permissions_non_default_workspace_write_is_custom() { ); } +#[tokio::test] +async fn status_snapshot_mentions_read_only_network_access() { + let temp_home = TempDir::new().expect("temp home"); + let mut config = test_config(&temp_home).await; + config + .permissions + .approval_policy + .set(AskForApproval::OnRequest) + .expect("set approval policy"); + config + .permissions + .sandbox_policy + .set(SandboxPolicy::ReadOnly { + access: Default::default(), + network_access: true, + }) + .expect("set sandbox policy"); + config.cwd = PathBuf::from("/workspace/tests"); + + let auth_manager = test_auth_manager(&config); + let usage = TokenUsage::default(); + let captured_at = chrono::Local + .with_ymd_and_hms(2024, 1, 2, 3, 4, 5) + .single() + .expect("timestamp"); + let model_slug = codex_core::test_support::get_model_offline(config.model.as_deref()); + + let composite = new_status_output( + &config, + &auth_manager, + None, + &usage, + &None, + None, + None, + None, + None, + captured_at, + &model_slug, + None, + None, + ); + let mut rendered_lines = render_lines(&composite.display_lines(80)); + if cfg!(windows) { + for line in &mut rendered_lines { + *line = line.replace('\\', "/"); + } + } + let sanitized = sanitize_directory(rendered_lines).join("\n"); + assert_snapshot!(sanitized); +} + #[tokio::test] async fn status_snapshot_includes_forked_from() { let temp_home = TempDir::new().expect("temp home"); diff --git a/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs b/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs index a5c9b31fdd..8f57a011f5 100644 --- a/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs +++ b/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs @@ -4,7 +4,13 @@ use codex_protocol::protocol::SandboxPolicy; pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { match sandbox_policy { SandboxPolicy::DangerFullAccess => "danger-full-access".to_string(), - SandboxPolicy::ReadOnly { .. } => "read-only".to_string(), + SandboxPolicy::ReadOnly { network_access, .. } => { + let mut summary = "read-only".to_string(); + if *network_access { + summary.push_str(" (network access enabled)"); + } + summary + } SandboxPolicy::ExternalSandbox { network_access } => { let mut summary = "external-sandbox".to_string(); if matches!(network_access, NetworkAccess::Enabled) { @@ -66,6 +72,15 @@ mod tests { assert_eq!(summary, "external-sandbox (network access enabled)"); } + #[test] + fn read_only_summary_includes_network_access() { + let summary = summarize_sandbox_policy(&SandboxPolicy::ReadOnly { + access: Default::default(), + network_access: true, + }); + assert_eq!(summary, "read-only (network access enabled)"); + } + #[test] fn workspace_write_summary_still_includes_network_access() { let root = if cfg!(windows) { "C:\\repo" } else { "/repo" };