From 16ca527c80ea3d863be5dcac64ba37b1f8c97e47 Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Tue, 24 Feb 2026 19:35:28 -0800 Subject: [PATCH] chore: migrate additional permissions to PermissionProfile (#12731) This PR replaces the old `additional_permissions.fs_read/fs_write` shape with a shared `PermissionProfile` model and wires it through the command approval, sandboxing, protocol, and TUI layers. The schema is adopted from the `SkillManifestPermissions`, which is also refactored to use this unified struct. This helps us easily expose permission profiles in app server/core as a follow-up. --- .../schema/json/EventMsg.json | 135 ++++++++++++-- .../codex_app_server_protocol.schemas.json | 133 ++++++++++++-- .../typescript/ExecApprovalRequestEvent.ts | 4 +- .../typescript/FileSystemPermissions.ts | 5 + ...Permissions.ts => MacOsAutomationValue.ts} | 2 +- .../schema/typescript/MacOsPermissions.ts | 7 + .../typescript/MacOsPreferencesValue.ts | 5 + .../schema/typescript/PermissionProfile.ts | 7 + .../schema/typescript/index.ts | 6 +- codex-rs/core/src/codex.rs | 4 +- codex-rs/core/src/sandboxing/mod.rs | 49 +++-- codex-rs/core/src/skills/loader.rs | 4 +- codex-rs/core/src/skills/permissions.rs | 169 ++++++++---------- codex-rs/core/src/tools/handlers/mod.rs | 10 +- codex-rs/core/src/tools/handlers/shell.rs | 4 +- .../core/src/tools/handlers/unified_exec.rs | 4 +- codex-rs/core/src/tools/runtimes/mod.rs | 4 +- codex-rs/core/src/tools/runtimes/shell.rs | 6 +- .../core/src/tools/runtimes/unified_exec.rs | 6 +- codex-rs/core/src/tools/spec.rs | 49 ++--- codex-rs/core/src/unified_exec/mod.rs | 4 +- .../core/tests/suite/request_permissions.rs | 68 ++++--- codex-rs/protocol/src/approvals.rs | 4 +- codex-rs/protocol/src/models.rs | 70 +++++++- .../on_request_rule_request_permission.md | 4 +- .../tui/src/bottom_pane/approval_overlay.rs | 72 ++++---- 26 files changed, 572 insertions(+), 263 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/FileSystemPermissions.ts rename codex-rs/app-server-protocol/schema/typescript/{AdditionalPermissions.ts => MacOsAutomationValue.ts} (62%) create mode 100644 codex-rs/app-server-protocol/schema/typescript/MacOsPermissions.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/MacOsPreferencesValue.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/PermissionProfile.ts diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index c81f69d10..5139e5da6 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -5,23 +5,6 @@ "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", "type": "string" }, - "AdditionalPermissions": { - "properties": { - "fs_read": { - "items": { - "type": "string" - }, - "type": "array" - }, - "fs_write": { - "items": { - "type": "string" - }, - "type": "array" - } - }, - "type": "object" - }, "AgentMessageContent": { "oneOf": [ { @@ -1633,7 +1616,7 @@ "additional_permissions": { "anyOf": [ { - "$ref": "#/definitions/AdditionalPermissions" + "$ref": "#/definitions/PermissionProfile" }, { "type": "null" @@ -3330,6 +3313,29 @@ } ] }, + "FileSystemPermissions": { + "properties": { + "read": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + }, + "write": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + } + }, + "type": "object" + }, "FunctionCallOutputBody": { "anyOf": [ { @@ -3523,6 +3529,66 @@ ], "type": "string" }, + "MacOsAutomationValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "items": { + "type": "string" + }, + "type": "array" + } + ] + }, + "MacOsPermissions": { + "properties": { + "accessibility": { + "type": [ + "boolean", + "null" + ] + }, + "automations": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsAutomationValue" + }, + { + "type": "null" + } + ] + }, + "calendar": { + "type": [ + "boolean", + "null" + ] + }, + "preferences": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPreferencesValue" + }, + { + "type": "null" + } + ] + } + }, + "type": "object" + }, + "MacOsPreferencesValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "string" + } + ] + }, "McpAuthStatus": { "enum": [ "unsupported", @@ -3840,6 +3906,37 @@ ], "type": "string" }, + "PermissionProfile": { + "properties": { + "file_system": { + "anyOf": [ + { + "$ref": "#/definitions/FileSystemPermissions" + }, + { + "type": "null" + } + ] + }, + "macos": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPermissions" + }, + { + "type": "null" + } + ] + }, + "network": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, "PlanItemArg": { "additionalProperties": false, "properties": { @@ -6945,7 +7042,7 @@ "additional_permissions": { "anyOf": [ { - "$ref": "#/definitions/AdditionalPermissions" + "$ref": "#/definitions/PermissionProfile" }, { "type": "null" 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 859970ef6..11dcf4192 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 @@ -1,23 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { - "AdditionalPermissions": { - "properties": { - "fs_read": { - "items": { - "type": "string" - }, - "type": "array" - }, - "fs_write": { - "items": { - "type": "string" - }, - "type": "array" - } - }, - "type": "object" - }, "AgentMessageContent": { "oneOf": [ { @@ -2694,7 +2677,7 @@ "additional_permissions": { "anyOf": [ { - "$ref": "#/definitions/AdditionalPermissions" + "$ref": "#/definitions/PermissionProfile" }, { "type": "null" @@ -4534,6 +4517,29 @@ "title": "FileChangeRequestApprovalResponse", "type": "object" }, + "FileSystemPermissions": { + "properties": { + "read": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + }, + "write": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + } + }, + "type": "object" + }, "FuzzyFileSearchParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -4834,6 +4840,66 @@ "title": "JSONRPCResponse", "type": "object" }, + "MacOsAutomationValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "items": { + "type": "string" + }, + "type": "array" + } + ] + }, + "MacOsPermissions": { + "properties": { + "accessibility": { + "type": [ + "boolean", + "null" + ] + }, + "automations": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsAutomationValue" + }, + { + "type": "null" + } + ] + }, + "calendar": { + "type": [ + "boolean", + "null" + ] + }, + "preferences": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPreferencesValue" + }, + { + "type": "null" + } + ] + } + }, + "type": "object" + }, + "MacOsPreferencesValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "string" + } + ] + }, "McpInvocation": { "properties": { "arguments": { @@ -5093,6 +5159,37 @@ } ] }, + "PermissionProfile": { + "properties": { + "file_system": { + "anyOf": [ + { + "$ref": "#/definitions/FileSystemPermissions" + }, + { + "type": "null" + } + ] + }, + "macos": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPermissions" + }, + { + "type": "null" + } + ] + }, + "network": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, "PlanItemArg": { "additionalProperties": false, "properties": { diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts index c777831e8..5144dab6e 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts @@ -1,11 +1,11 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { AdditionalPermissions } from "./AdditionalPermissions"; import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; import type { NetworkApprovalContext } from "./NetworkApprovalContext"; import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; import type { ParsedCommand } from "./ParsedCommand"; +import type { PermissionProfile } from "./PermissionProfile"; export type ExecApprovalRequestEvent = { /** @@ -51,4 +51,4 @@ proposed_network_policy_amendments?: Array, /** * Optional additional filesystem permissions requested for this command. */ -additional_permissions?: AdditionalPermissions, parsed_cmd: Array, }; +additional_permissions?: PermissionProfile, parsed_cmd: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemPermissions.ts new file mode 100644 index 000000000..9e03768df --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemPermissions.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type FileSystemPermissions = { read: Array | null, write: Array | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/AdditionalPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/MacOsAutomationValue.ts similarity index 62% rename from codex-rs/app-server-protocol/schema/typescript/AdditionalPermissions.ts rename to codex-rs/app-server-protocol/schema/typescript/MacOsAutomationValue.ts index 363807a2a..e351c319d 100644 --- a/codex-rs/app-server-protocol/schema/typescript/AdditionalPermissions.ts +++ b/codex-rs/app-server-protocol/schema/typescript/MacOsAutomationValue.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type AdditionalPermissions = { fs_read?: Array, fs_write?: Array, }; +export type MacOsAutomationValue = boolean | Array; diff --git a/codex-rs/app-server-protocol/schema/typescript/MacOsPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/MacOsPermissions.ts new file mode 100644 index 000000000..5c0792412 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/MacOsPermissions.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { MacOsAutomationValue } from "./MacOsAutomationValue"; +import type { MacOsPreferencesValue } from "./MacOsPreferencesValue"; + +export type MacOsPermissions = { preferences: MacOsPreferencesValue | null, automations: MacOsAutomationValue | null, accessibility: boolean | null, calendar: boolean | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/MacOsPreferencesValue.ts b/codex-rs/app-server-protocol/schema/typescript/MacOsPreferencesValue.ts new file mode 100644 index 000000000..74a67ca1c --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/MacOsPreferencesValue.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type MacOsPreferencesValue = boolean | string; diff --git a/codex-rs/app-server-protocol/schema/typescript/PermissionProfile.ts b/codex-rs/app-server-protocol/schema/typescript/PermissionProfile.ts new file mode 100644 index 000000000..bfc0de27b --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/PermissionProfile.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { FileSystemPermissions } from "./FileSystemPermissions"; +import type { MacOsPermissions } from "./MacOsPermissions"; + +export type PermissionProfile = { network: boolean | null, file_system: FileSystemPermissions | null, macos: MacOsPermissions | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 3a1281cf2..cc323e963 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -3,7 +3,6 @@ export type { AbsolutePathBuf } from "./AbsolutePathBuf"; export type { AddConversationListenerParams } from "./AddConversationListenerParams"; export type { AddConversationSubscriptionResponse } from "./AddConversationSubscriptionResponse"; -export type { AdditionalPermissions } from "./AdditionalPermissions"; export type { AgentMessageContent } from "./AgentMessageContent"; export type { AgentMessageContentDeltaEvent } from "./AgentMessageContentDeltaEvent"; export type { AgentMessageDeltaEvent } from "./AgentMessageDeltaEvent"; @@ -72,6 +71,7 @@ export type { ExecOutputStream } from "./ExecOutputStream"; export type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; export type { ExitedReviewModeEvent } from "./ExitedReviewModeEvent"; export type { FileChange } from "./FileChange"; +export type { FileSystemPermissions } from "./FileSystemPermissions"; export type { ForcedLoginMethod } from "./ForcedLoginMethod"; export type { ForkConversationParams } from "./ForkConversationParams"; export type { ForkConversationResponse } from "./ForkConversationResponse"; @@ -117,6 +117,9 @@ export type { LoginApiKeyResponse } from "./LoginApiKeyResponse"; export type { LoginChatGptCompleteNotification } from "./LoginChatGptCompleteNotification"; export type { LoginChatGptResponse } from "./LoginChatGptResponse"; export type { LogoutChatGptResponse } from "./LogoutChatGptResponse"; +export type { MacOsAutomationValue } from "./MacOsAutomationValue"; +export type { MacOsPermissions } from "./MacOsPermissions"; +export type { MacOsPreferencesValue } from "./MacOsPreferencesValue"; export type { McpAuthStatus } from "./McpAuthStatus"; export type { McpInvocation } from "./McpInvocation"; export type { McpListToolsResponseEvent } from "./McpListToolsResponseEvent"; @@ -141,6 +144,7 @@ export type { ParsedCommand } from "./ParsedCommand"; export type { PatchApplyBeginEvent } from "./PatchApplyBeginEvent"; export type { PatchApplyEndEvent } from "./PatchApplyEndEvent"; export type { PatchApplyStatus } from "./PatchApplyStatus"; +export type { PermissionProfile } from "./PermissionProfile"; export type { Personality } from "./Personality"; export type { PlanDeltaEvent } from "./PlanDeltaEvent"; export type { PlanItem } from "./PlanItem"; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index d9f383ef6..0fe5f2852 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -70,8 +70,8 @@ use codex_protocol::items::PlanItem; use codex_protocol::items::TurnItem; use codex_protocol::items::UserMessageItem; use codex_protocol::mcp::CallToolResult; -use codex_protocol::models::AdditionalPermissions; use codex_protocol::models::BaseInstructions; +use codex_protocol::models::PermissionProfile; use codex_protocol::models::format_allow_prefixes; use codex_protocol::openai_models::ModelInfo; use codex_protocol::protocol::FileChange; @@ -2571,7 +2571,7 @@ impl Session { reason: Option, network_approval_context: Option, proposed_execpolicy_amendment: Option, - additional_permissions: Option, + additional_permissions: Option, ) -> ReviewDecision { // command-level approvals use `call_id`. // `approval_id` is only present for subcommand callbacks (execve intercept) diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 70ca5588e..56c7bff6a 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -24,7 +24,8 @@ use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use crate::tools::sandboxing::SandboxablePreference; use codex_network_proxy::NetworkProxy; use codex_protocol::config_types::WindowsSandboxLevel; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::FileSystemPermissions; +use codex_protocol::models::PermissionProfile; pub use codex_protocol::models::SandboxPermissions; use codex_protocol::protocol::ReadOnlyAccess; use codex_utils_absolute_path::AbsolutePathBuf; @@ -41,7 +42,7 @@ pub struct CommandSpec { pub env: HashMap, pub expiration: ExecExpiration, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, pub justification: Option, } @@ -95,14 +96,24 @@ pub(crate) enum SandboxTransformError { } pub(crate) fn normalize_additional_permissions( - additional_permissions: AdditionalPermissions, + additional_permissions: PermissionProfile, command_cwd: &Path, -) -> Result { - let fs_read = - normalize_permission_paths(additional_permissions.fs_read, command_cwd, "fs_read")?; - let fs_write = - normalize_permission_paths(additional_permissions.fs_write, command_cwd, "fs_write")?; - Ok(AdditionalPermissions { fs_read, fs_write }) +) -> Result { + let Some(file_system) = additional_permissions.file_system else { + return Ok(PermissionProfile::default()); + }; + let read = file_system + .read + .map(|paths| normalize_permission_paths(paths, command_cwd, "file_system.read")) + .transpose()?; + let write = file_system + .write + .map(|paths| normalize_permission_paths(paths, command_cwd, "file_system.write")) + .transpose()?; + Ok(PermissionProfile { + file_system: Some(FileSystemPermissions { read, write }), + ..Default::default() + }) } fn normalize_permission_paths( @@ -162,7 +173,7 @@ fn dedup_absolute_paths(paths: Vec) -> Vec { } fn additional_permission_roots( - additional_permissions: &AdditionalPermissions, + additional_permissions: &PermissionProfile, ) -> Result<(Vec, Vec), SandboxTransformError> { let to_abs = |paths: &[PathBuf]| { let mut out = Vec::with_capacity(paths.len()); @@ -179,8 +190,20 @@ fn additional_permission_roots( }; Ok(( - to_abs(&additional_permissions.fs_read)?, - to_abs(&additional_permissions.fs_write)?, + to_abs( + additional_permissions + .file_system + .as_ref() + .and_then(|file_system| file_system.read.as_deref()) + .unwrap_or_default(), + )?, + to_abs( + additional_permissions + .file_system + .as_ref() + .and_then(|file_system| file_system.write.as_deref()) + .unwrap_or_default(), + )?, )) } @@ -206,7 +229,7 @@ fn merge_read_only_access_with_additional_reads( fn sandbox_policy_with_additional_permissions( sandbox_policy: &SandboxPolicy, - additional_permissions: &AdditionalPermissions, + additional_permissions: &PermissionProfile, ) -> Result { if additional_permissions.is_empty() { return Ok(sandbox_policy.clone()); diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index 960830649..873f6cc09 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -12,10 +12,10 @@ 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::models::PermissionProfile; use codex_protocol::protocol::SkillScope; use dirs::home_dir; use dunce::canonicalize as canonicalize_path; @@ -54,7 +54,7 @@ struct SkillMetadataFile { #[serde(default)] policy: Option, #[serde(default)] - permissions: Option, + permissions: Option, } #[derive(Debug, Default, Deserialize)] diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs index 24f3c9444..4019a3107 100644 --- a/codex-rs/core/src/skills/permissions.rs +++ b/codex-rs/core/src/skills/permissions.rs @@ -3,10 +3,15 @@ use std::path::Component; use std::path::Path; use std::path::PathBuf; +#[cfg(target_os = "macos")] +use codex_protocol::models::MacOsAutomationValue; +use codex_protocol::models::MacOsPermissions; +#[cfg(target_os = "macos")] +use codex_protocol::models::MacOsPreferencesValue; +use codex_protocol::models::PermissionProfile; 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; @@ -20,63 +25,24 @@ 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, + permissions: Option, ) -> Option { - let permissions = permissions?; + let PermissionProfile { + network, + file_system, + macos, + } = permissions?; + let file_system = file_system.unwrap_or_default(); let fs_read = normalize_permission_paths( skill_dir, - &permissions.file_system.read, + file_system.read.as_deref().unwrap_or_default(), "permissions.file_system.read", ); let fs_write = normalize_permission_paths( skill_dir, - &permissions.file_system.write, + file_system.write.as_deref().unwrap_or_default(), "permissions.file_system.write", ); let sandbox_policy = if !fs_write.is_empty() { @@ -90,7 +56,7 @@ pub(crate) fn compile_permission_profile( readable_roots: fs_read, } }, - network_access: permissions.network, + network_access: network.unwrap_or_default(), exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } @@ -105,8 +71,9 @@ pub(crate) fn compile_permission_profile( // Default sandbox policy SandboxPolicy::new_read_only_policy() }; + let macos_permissions = macos.unwrap_or_default(); let macos_seatbelt_profile_extensions = - build_macos_seatbelt_profile_extensions(&permissions.macos); + build_macos_seatbelt_profile_extensions(&macos_permissions); Some(Permissions { approval_policy: Constrained::allow_any(AskForApproval::Never), @@ -121,7 +88,7 @@ pub(crate) fn compile_permission_profile( fn normalize_permission_paths( skill_dir: &Path, - values: &[String], + values: &[PathBuf], field: &str, ) -> Vec { let mut paths = Vec::new(); @@ -141,9 +108,10 @@ fn normalize_permission_paths( fn normalize_permission_path( skill_dir: &Path, - value: &str, + value: &Path, field: &str, ) -> Option { + let value = value.to_string_lossy(); let trimmed = value.trim(); if trimmed.is_empty() { warn!("ignoring {field}: value is empty"); @@ -151,11 +119,10 @@ fn normalize_permission_path( } let expanded = expand_home(trimmed); - let path = PathBuf::from(expanded); - let absolute = if path.is_absolute() { - path + let absolute = if expanded.is_absolute() { + expanded } else { - skill_dir.join(path) + skill_dir.join(expanded) }; let normalized = normalize_lexically(&absolute); let canonicalized = canonicalize_path(&normalized).unwrap_or(normalized); @@ -168,24 +135,24 @@ fn normalize_permission_path( } } -fn expand_home(path: &str) -> String { +fn expand_home(path: &str) -> PathBuf { if path == "~" { if let Some(home) = home_dir() { - return home.to_string_lossy().to_string(); + return home; } - return path.to_string(); + return PathBuf::from(path); } if let Some(rest) = path.strip_prefix("~/") && let Some(home) = home_dir() { - return home.join(rest).to_string_lossy().to_string(); + return home.join(rest); } - path.to_string() + PathBuf::from(path) } #[cfg(target_os = "macos")] fn build_macos_seatbelt_profile_extensions( - permissions: &SkillManifestMacOsPermissions, + permissions: &MacOsPermissions, ) -> Option { let defaults = MacOsSeatbeltProfileExtensions::default(); @@ -198,8 +165,10 @@ fn build_macos_seatbelt_profile_extensions( permissions.automations.as_ref(), defaults.macos_automation, ), - macos_accessibility: permissions.accessibility, - macos_calendar: permissions.calendar, + macos_accessibility: permissions + .accessibility + .unwrap_or(defaults.macos_accessibility), + macos_calendar: permissions.calendar.unwrap_or(defaults.macos_calendar), }; Some(extensions) } @@ -262,7 +231,7 @@ fn resolve_macos_automation_permission( #[cfg(not(target_os = "macos"))] fn build_macos_seatbelt_profile_extensions( - _: &SkillManifestMacOsPermissions, + _: &MacOsPermissions, ) -> Option { None } @@ -285,10 +254,6 @@ fn normalize_lexically(path: &Path) -> PathBuf { #[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; @@ -296,9 +261,18 @@ mod tests { use crate::protocol::AskForApproval; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; + use codex_protocol::models::FileSystemPermissions; + #[cfg(target_os = "macos")] + use codex_protocol::models::MacOsAutomationValue; + #[cfg(target_os = "macos")] + use codex_protocol::models::MacOsPermissions; + #[cfg(target_os = "macos")] + use codex_protocol::models::MacOsPreferencesValue; + use codex_protocol::models::PermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::fs; + use std::path::PathBuf; #[test] fn compile_permission_profile_normalizes_paths() { @@ -310,16 +284,16 @@ mod tests { 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()], - }, + Some(PermissionProfile { + network: Some(true), + file_system: Some(FileSystemPermissions { + read: Some(vec![ + PathBuf::from("./data"), + PathBuf::from("./data"), + PathBuf::from("scripts/../data"), + ]), + write: Some(vec![PathBuf::from("./output")]), + }), ..Default::default() }), ) @@ -380,8 +354,8 @@ mod tests { let profile = compile_permission_profile( &skill_dir, - Some(SkillManifestPermissions { - network: true, + Some(PermissionProfile { + network: Some(true), ..Default::default() }), ) @@ -415,12 +389,12 @@ mod tests { let profile = compile_permission_profile( &skill_dir, - Some(SkillManifestPermissions { - network: true, - file_system: SkillManifestFileSystemPermissions { - read: vec!["./data".to_string()], - write: Vec::new(), - }, + Some(PermissionProfile { + network: Some(true), + file_system: Some(FileSystemPermissions { + read: Some(vec![PathBuf::from("./data")]), + write: Some(Vec::new()), + }), ..Default::default() }), ) @@ -464,15 +438,15 @@ mod tests { 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![ + Some(PermissionProfile { + macos: Some(MacOsPermissions { + preferences: Some(MacOsPreferencesValue::Mode("readwrite".to_string())), + automations: Some(MacOsAutomationValue::BundleIds(vec![ "com.apple.Notes".to_string(), ])), - accessibility: true, - calendar: true, - }, + accessibility: Some(true), + calendar: Some(true), + }), ..Default::default() }), ) @@ -502,9 +476,8 @@ mod tests { 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"); + let profile = compile_permission_profile(&skill_dir, Some(PermissionProfile::default())) + .expect("profile"); assert_eq!( profile.macos_seatbelt_profile_extensions, diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index bdc47a088..23dc979f2 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -24,7 +24,7 @@ use crate::function_tool::FunctionCallError; use crate::sandboxing::SandboxPermissions; use crate::sandboxing::normalize_additional_permissions; pub use apply_patch::ApplyPatchHandler; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; pub use dynamic::DynamicToolHandler; pub use grep_files::GrepFilesHandler; @@ -62,9 +62,9 @@ pub(super) fn normalize_and_validate_additional_permissions( request_permission_enabled: bool, approval_policy: AskForApproval, sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option, cwd: &Path, -) -> Result, String> { +) -> Result, String> { let uses_additional_permissions = matches!( sandbox_permissions, SandboxPermissions::WithAdditionalPermissions @@ -87,14 +87,14 @@ pub(super) fn normalize_and_validate_additional_permissions( } let Some(additional_permissions) = additional_permissions else { return Err( - "missing `additional_permissions`; provide `fs_read` and/or `fs_write` when using `with_additional_permissions`" + "missing `additional_permissions`; provide `file_system.read` and/or `file_system.write` when using `with_additional_permissions`" .to_string(), ); }; let normalized = normalize_additional_permissions(additional_permissions, cwd)?; if normalized.is_empty() { return Err( - "`additional_permissions` must include at least one path in `fs_read` or `fs_write`" + "`additional_permissions` must include at least one path in `file_system.read` or `file_system.write`" .to_string(), ); } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 0a0bb5347..95dbe05e5 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -33,7 +33,7 @@ use crate::tools::runtimes::shell::ShellRuntime; use crate::tools::runtimes::shell::ShellRuntimeBackend; use crate::tools::sandboxing::ToolCtx; use crate::tools::spec::ShellCommandBackendConfig; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; pub struct ShellHandler; @@ -50,7 +50,7 @@ pub struct ShellCommandHandler { struct RunExecLikeArgs { tool_name: String, exec_params: ExecParams, - additional_permissions: Option, + additional_permissions: Option, prefix_rule: Option>, session: Arc, turn: Arc, diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 9c5e3bc4b..57a109dd4 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -21,8 +21,8 @@ use crate::unified_exec::UnifiedExecProcessManager; use crate::unified_exec::UnifiedExecResponse; use crate::unified_exec::WriteStdinRequest; use async_trait::async_trait; -use codex_protocol::models::AdditionalPermissions; use codex_protocol::models::FunctionCallOutputBody; +use codex_protocol::models::PermissionProfile; use serde::Deserialize; use std::path::PathBuf; use std::sync::Arc; @@ -47,7 +47,7 @@ pub(crate) struct ExecCommandArgs { #[serde(default)] sandbox_permissions: SandboxPermissions, #[serde(default)] - additional_permissions: Option, + additional_permissions: Option, #[serde(default)] justification: Option, #[serde(default)] diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 6dc20002d..937ab341d 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -10,7 +10,7 @@ use crate::sandboxing::CommandSpec; use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; use crate::tools::sandboxing::ToolError; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use std::collections::HashMap; use std::path::Path; @@ -26,7 +26,7 @@ pub(crate) fn build_command_spec( env: &HashMap, expiration: ExecExpiration, sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option, justification: Option, ) -> Result { let (program, args) = command diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 0a5353755..80d3b22eb 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -31,7 +31,7 @@ use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::sandbox_override_for_first_attempt; use crate::tools::sandboxing::with_cached_approval; use codex_network_proxy::NetworkProxy; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; use std::collections::HashMap; @@ -46,7 +46,7 @@ pub struct ShellRequest { pub explicit_env_overrides: HashMap, pub network: Option, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, pub justification: Option, pub exec_approval_requirement: ExecApprovalRequirement, } @@ -89,7 +89,7 @@ pub(crate) struct ApprovalKey { command: Vec, cwd: PathBuf, sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option, } impl ShellRuntime { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 893dae27e..4b0c53d12 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -32,7 +32,7 @@ use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecProcess; use crate::unified_exec::UnifiedExecProcessManager; use codex_network_proxy::NetworkProxy; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; use std::collections::HashMap; @@ -47,7 +47,7 @@ pub struct UnifiedExecRequest { pub network: Option, pub tty: bool, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, pub justification: Option, pub exec_approval_requirement: ExecApprovalRequirement, } @@ -58,7 +58,7 @@ pub struct UnifiedExecApprovalKey { pub cwd: PathBuf, pub tty: bool, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, } pub struct UnifiedExecRuntime<'a> { diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 3c9ab4bf3..0f50cf873 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -256,29 +256,36 @@ fn create_approval_parameters(request_permission_enabled: bool) -> BTreeMap, pub tty: bool, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, pub justification: Option, pub prefix_rule: Option>, } diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index d6003e4b8..cda482079 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -5,7 +5,8 @@ use codex_core::config::Constrained; use codex_core::features::Feature; use codex_core::sandboxing::SandboxPermissions; use codex_protocol::config_types::ReasoningSummary; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::FileSystemPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecApprovalRequestEvent; @@ -79,7 +80,7 @@ fn parse_result(item: &Value) -> CommandResult { fn shell_event_with_request_permissions( call_id: &str, command: &str, - additional_permissions: &AdditionalPermissions, + additional_permissions: &PermissionProfile, ) -> Result { let args = json!({ "command": command, @@ -184,9 +185,12 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res let _ = fs::remove_file(&requested_write); let call_id = "request_permissions_skip_approval"; let command = "touch requested-but-unused.txt"; - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![requested_write.clone()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![requested_write.clone()]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, command, &requested_permissions)?; @@ -266,9 +270,12 @@ async fn read_only_with_additional_permissions_widens_to_unrequested_cwd_write() "printf {:?} > {:?} && cat {:?}", "cwd-widened", unrequested_write, unrequested_write ); - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![requested_write.clone()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![requested_write.clone()]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -354,9 +361,12 @@ async fn read_only_with_additional_permissions_widens_to_unrequested_tmp_write() "printf {:?} > {:?} && cat {:?}", "tmp-widened", tmp_write, tmp_write ); - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![requested_write.clone()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![requested_write.clone()]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -442,13 +452,19 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> "printf {:?} > {:?} && cat {:?}", "outside-cwd-ok", outside_write, outside_write ); - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![outside_dir.path().to_path_buf()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![outside_dir.path().to_path_buf()]), + }), + ..Default::default() }; - let normalized_requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![outside_dir.path().canonicalize()?], + let normalized_requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![outside_dir.path().canonicalize()?]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -530,13 +546,19 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul "printf {:?} > {:?} && cat {:?}", "should-not-write", outside_write, outside_write ); - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![outside_dir.path().to_path_buf()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![outside_dir.path().to_path_buf()]), + }), + ..Default::default() }; - let normalized_requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![outside_dir.path().canonicalize()?], + let normalized_requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![outside_dir.path().canonicalize()?]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index e0f0c0625..f742dc632 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::path::PathBuf; use crate::mcp::RequestId; -use crate::models::AdditionalPermissions; +use crate::models::PermissionProfile; use crate::parse_command::ParsedCommand; use crate::protocol::FileChange; use schemars::JsonSchema; @@ -106,7 +106,7 @@ pub struct ExecApprovalRequestEvent { /// Optional additional filesystem permissions requested for this command. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] - pub additional_permissions: Option, + pub additional_permissions: Option, pub parsed_cmd: Vec, } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index c9fea3a29..ff5030c75 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -53,16 +53,68 @@ impl SandboxPermissions { } #[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] -pub struct AdditionalPermissions { - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub fs_read: Vec, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub fs_write: Vec, +pub struct FileSystemPermissions { + pub read: Option>, + pub write: Option>, } -impl AdditionalPermissions { +impl FileSystemPermissions { pub fn is_empty(&self) -> bool { - self.fs_read.is_empty() && self.fs_write.is_empty() + self.read.is_none() && self.write.is_none() + } +} + +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +pub struct MacOsPermissions { + pub preferences: Option, + pub automations: Option, + pub accessibility: Option, + pub calendar: Option, +} + +impl MacOsPermissions { + pub fn is_empty(&self) -> bool { + self.preferences.is_none() + && self.automations.is_none() + && self.accessibility.is_none() + && self.calendar.is_none() + } +} + +#[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +#[serde(untagged)] +pub enum MacOsPreferencesValue { + Bool(bool), + Mode(String), +} + +#[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +#[serde(untagged)] +pub enum MacOsAutomationValue { + Bool(bool), + BundleIds(Vec), +} + +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +pub struct PermissionProfile { + pub network: Option, + pub file_system: Option, + pub macos: Option, +} + +impl PermissionProfile { + pub fn is_empty(&self) -> bool { + self.network.is_none() + && self + .file_system + .as_ref() + .map(FileSystemPermissions::is_empty) + .unwrap_or(true) + && self + .macos + .as_ref() + .map(MacOsPermissions::is_empty) + .unwrap_or(true) } } @@ -800,7 +852,7 @@ pub struct ShellToolCallParams { pub prefix_rule: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] - pub additional_permissions: Option, + pub additional_permissions: Option, #[serde(skip_serializing_if = "Option::is_none")] pub justification: Option, } @@ -826,7 +878,7 @@ pub struct ShellCommandToolCallParams { pub prefix_rule: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] - pub additional_permissions: Option, + pub additional_permissions: Option, #[serde(skip_serializing_if = "Option::is_none")] pub justification: Option, } diff --git a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md b/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md index 7a9f71e96..1c9a3853a 100644 --- a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md +++ b/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md @@ -8,8 +8,8 @@ When you need extra filesystem access for one command, use: - `sandbox_permissions: "with_additional_permissions"` - `additional_permissions` with one or both fields: - - `fs_read`: list of paths that need read access - - `fs_write`: list of paths that need write access + - `file_system.read`: list of paths that need read access + - `file_system.write`: list of paths that need write access This keeps execution inside the current sandbox policy, while adding only the requested permissions for that command, unless an exec-policy allow rule applies and authorizes running the command outside the sandbox. diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 76b1c3d60..0cfd48bec 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -18,7 +18,7 @@ use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; use codex_core::features::Features; use codex_protocol::mcp::RequestId; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ElicitationAction; use codex_protocol::protocol::ExecPolicyAmendment; use codex_protocol::protocol::FileChange; @@ -46,7 +46,7 @@ pub(crate) enum ApprovalRequest { reason: Option, network_approval_context: Option, proposed_execpolicy_amendment: Option, - additional_permissions: Option, + additional_permissions: Option, }, ApplyPatch { id: String, @@ -450,7 +450,7 @@ enum ApprovalVariant { command: Vec, network_approval_context: Option, proposed_execpolicy_amendment: Option, - additional_permissions: Option, + additional_permissions: Option, }, ApplyPatch { id: String, @@ -486,7 +486,7 @@ impl ApprovalOption { fn exec_options( proposed_execpolicy_amendment: Option, network_approval_context: Option<&NetworkApprovalContext>, - additional_permissions: Option<&AdditionalPermissions>, + additional_permissions: Option<&PermissionProfile>, ) -> Vec { if network_approval_context.is_some() { return vec![ @@ -562,26 +562,26 @@ fn exec_options( } fn format_additional_permissions_rule( - additional_permissions: &AdditionalPermissions, + additional_permissions: &PermissionProfile, ) -> Option { let mut parts = Vec::new(); - if !additional_permissions.fs_read.is_empty() { - let reads = additional_permissions - .fs_read - .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); - parts.push(format!("read {reads}")); - } - if !additional_permissions.fs_write.is_empty() { - let writes = additional_permissions - .fs_write - .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); - parts.push(format!("write {writes}")); + if let Some(file_system) = additional_permissions.file_system.as_ref() { + if let Some(read) = file_system.read.as_ref() { + let reads = read + .iter() + .map(|path| format!("`{}`", path.display())) + .collect::>() + .join(", "); + parts.push(format!("read {reads}")); + } + if let Some(write) = file_system.write.as_ref() { + let writes = write + .iter() + .map(|path| format!("`{}`", path.display())) + .collect::>() + .join(", "); + parts.push(format!("write {writes}")); + } } if parts.is_empty() { @@ -641,6 +641,7 @@ fn elicitation_options() -> Vec { mod tests { use super::*; use crate::app_event::AppEvent; + use codex_protocol::models::FileSystemPermissions; use codex_protocol::protocol::NetworkApprovalProtocol; use insta::assert_snapshot; use pretty_assertions::assert_eq; @@ -800,9 +801,12 @@ mod tests { #[test] fn additional_permissions_exec_options_hide_execpolicy_amendment() { - let additional_permissions = AdditionalPermissions { - fs_read: vec![PathBuf::from("/tmp/readme.txt")], - fs_write: vec![PathBuf::from("/tmp/out.txt")], + let additional_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![PathBuf::from("/tmp/readme.txt")]), + write: Some(vec![PathBuf::from("/tmp/out.txt")]), + }), + ..Default::default() }; let options = exec_options(None, None, Some(&additional_permissions)); @@ -826,9 +830,12 @@ mod tests { reason: None, network_approval_context: None, proposed_execpolicy_amendment: None, - additional_permissions: Some(AdditionalPermissions { - fs_read: vec![PathBuf::from("/tmp/readme.txt")], - fs_write: vec![PathBuf::from("/tmp/out.txt")], + additional_permissions: Some(PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![PathBuf::from("/tmp/readme.txt")]), + write: Some(vec![PathBuf::from("/tmp/out.txt")]), + }), + ..Default::default() }), }; @@ -862,9 +869,12 @@ mod tests { reason: Some("need filesystem access".into()), network_approval_context: None, proposed_execpolicy_amendment: None, - additional_permissions: Some(AdditionalPermissions { - fs_read: vec![PathBuf::from("/tmp/readme.txt")], - fs_write: vec![PathBuf::from("/tmp/out.txt")], + additional_permissions: Some(PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![PathBuf::from("/tmp/readme.txt")]), + write: Some(vec![PathBuf::from("/tmp/out.txt")]), + }), + ..Default::default() }), };