From 72b41c55fb32f62373e070d6ac0cde7ba563989b Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Wed, 15 Jul 2026 01:26:55 +0000 Subject: [PATCH] Separate exec permission paths from core models (#33200) ## Why Core filesystem permission models operate on native absolute paths, while exec sandbox contexts must serialize paths as portable URIs. ## What changed - Make the core filesystem permission and profile types concrete over `AbsolutePathBuf` instead of generic over their path representation. - Add exec-specific permission types that convert native permission paths to `PathUri` values at the sandbox protocol boundary. - Replace the legacy read/write-roots tuple with a named `LegacyReadWriteRoots` struct. ## Testing - Add coverage that exec sandbox permission paths serialize as URIs and round trip through the filesystem protocol. GitOrigin-RevId: a0ef5f8aba9bbfdf00a00fcc4f199314836bc85d --- .../src/protocol/v2/permissions.rs | 31 +-- .../src/tools/handlers/apply_patch_tests.rs | 2 +- .../core/tests/suite/request_permissions.rs | 10 +- codex-rs/exec-server-protocol/src/protocol.rs | 37 +++ codex-rs/file-system/src/lib.rs | 215 ++++++++++++++--- codex-rs/protocol/src/models.rs | 222 ++++-------------- codex-rs/protocol/src/permissions.rs | 56 +---- 7 files changed, 292 insertions(+), 281 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs b/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs index d1d7c680f6..a6976c9c07 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs @@ -7,6 +7,7 @@ use codex_protocol::approvals::NetworkPolicyRuleAction as CoreNetworkPolicyRuleA use codex_protocol::models::ActivePermissionProfile as CoreActivePermissionProfile; use codex_protocol::models::AdditionalPermissionProfile as CoreAdditionalPermissionProfile; use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions; +use codex_protocol::models::LegacyReadWriteRoots; use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions; use codex_protocol::permissions::FileSystemAccessMode as CoreFileSystemAccessMode; use codex_protocol::permissions::FileSystemPath as CoreFileSystemPath; @@ -69,9 +70,9 @@ pub struct AdditionalFileSystemPermissions { } // TODO(anp): Remove this conversion once core permission paths use PathUri. -impl From> for AdditionalFileSystemPermissions { - fn from(value: CoreFileSystemPermissions) -> Self { - if let Some((read, write)) = value.legacy_read_write_roots() { +impl From for AdditionalFileSystemPermissions { + fn from(value: CoreFileSystemPermissions) -> Self { + if let Some(LegacyReadWriteRoots { read, write }) = value.legacy_read_write_roots() { let mut entries = Vec::with_capacity( read.as_ref().map_or(0, Vec::len) + write.as_ref().map_or(0, Vec::len), ); @@ -125,7 +126,7 @@ impl From> for AdditionalFileSystemPe } // TODO(anp): Remove this conversion once core permission paths use PathUri. -impl TryFrom for CoreFileSystemPermissions { +impl TryFrom for CoreFileSystemPermissions { type Error = io::Error; fn try_from(value: AdditionalFileSystemPermissions) -> Result { @@ -133,7 +134,7 @@ impl TryFrom for CoreFileSystemPermissions::try_from) + .map(CoreFileSystemSandboxEntry::try_from) .collect::>()?, glob_scan_max_depth: None, } @@ -221,7 +222,7 @@ impl TryFrom for CoreRequestPermissionProfile { network: value.network.map(CoreNetworkPermissions::from), file_system: value .file_system - .map(CoreFileSystemPermissions::::try_from) + .map(CoreFileSystemPermissions::try_from) .transpose()?, }) } @@ -300,7 +301,7 @@ impl From for CoreFileSystemSpecialPath { #[serde(tag = "type", rename_all = "snake_case")] #[ts(tag = "type")] #[ts(export_to = "v2/")] -// TODO(anp): Rename this type to distinguish it from the generic protocol FileSystemPath. +// TODO(anp): Rename this type to distinguish it from the protocol FileSystemPath. pub enum FileSystemPath { Path { path: LegacyAppPathString }, GlobPattern { pattern: String }, @@ -308,8 +309,8 @@ pub enum FileSystemPath { } // TODO(anp): Remove this conversion once core permission paths use PathUri. -impl From> for FileSystemPath { - fn from(value: CoreFileSystemPath) -> Self { +impl From for FileSystemPath { + fn from(value: CoreFileSystemPath) -> Self { match value { CoreFileSystemPath::Path { path } => Self::Path { path: LegacyAppPathString::from_abs_path(&path), @@ -323,7 +324,7 @@ impl From> for FileSystemPath { } // TODO(anp): Remove this conversion once core permission paths use PathUri. -impl TryFrom for CoreFileSystemPath { +impl TryFrom for CoreFileSystemPath { type Error = io::Error; fn try_from(value: FileSystemPath) -> Result { @@ -351,8 +352,8 @@ pub struct FileSystemSandboxEntry { } // TODO(anp): Remove this conversion once core permission paths use PathUri. -impl From> for FileSystemSandboxEntry { - fn from(value: CoreFileSystemSandboxEntry) -> Self { +impl From for FileSystemSandboxEntry { + fn from(value: CoreFileSystemSandboxEntry) -> Self { Self { path: value.path.into(), access: value.access.into(), @@ -360,7 +361,7 @@ impl From> for FileSystemSandboxEntr } } -impl TryFrom for CoreFileSystemSandboxEntry { +impl TryFrom for CoreFileSystemSandboxEntry { type Error = io::Error; fn try_from(value: FileSystemSandboxEntry) -> Result { @@ -479,7 +480,7 @@ impl TryFrom for CoreAdditionalPermissionProfile { network: value.network.map(CoreNetworkPermissions::from), file_system: value .file_system - .map(CoreFileSystemPermissions::::try_from) + .map(CoreFileSystemPermissions::try_from) .transpose()?, }) } @@ -505,7 +506,7 @@ impl TryFrom for CoreAdditionalPermissionProfile { network: value.network.map(CoreNetworkPermissions::from), file_system: value .file_system - .map(CoreFileSystemPermissions::::try_from) + .map(CoreFileSystemPermissions::try_from) .transpose()?, }) } diff --git a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs index f346b3d143..71dc323c60 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -290,7 +290,7 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() { permissions .and_then(|profile| profile.file_system) .and_then(|fs| fs.legacy_read_write_roots()) - .and_then(|(_read, write)| write), + .and_then(|roots| roots.write), Some(vec![expected_outside]) ); } diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index afa8e2194d..bc0da01b96 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -1649,7 +1649,10 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() let approval_file_system = approval_permissions .file_system .expect("expected filesystem permissions"); - let (approval_reads, approval_writes) = approval_file_system + let codex_protocol::models::LegacyReadWriteRoots { + read: approval_reads, + write: approval_writes, + } = approval_file_system .legacy_read_write_roots() .expect("expected legacy-compatible permissions"); assert!(approval_reads.as_ref().is_none_or(Vec::is_empty)); @@ -1657,7 +1660,10 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions() let mut approval_writes = approval_writes.unwrap_or_default(); approval_writes.sort_by_key(|path| path.display().to_string()); - let (_, expected_writes) = merged_permissions + let codex_protocol::models::LegacyReadWriteRoots { + write: expected_writes, + .. + } = merged_permissions .file_system .expect("expected merged filesystem permissions") .legacy_read_write_roots() diff --git a/codex-rs/exec-server-protocol/src/protocol.rs b/codex-rs/exec-server-protocol/src/protocol.rs index c55749cb73..6eb2b92848 100644 --- a/codex-rs/exec-server-protocol/src/protocol.rs +++ b/codex-rs/exec-server-protocol/src/protocol.rs @@ -597,6 +597,11 @@ mod tests { use codex_file_system::FileSystemSandboxContext; use codex_network_proxy::ManagedNetworkSandboxContext; use codex_protocol::models::PermissionProfile; + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSandboxPolicy; + use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::collections::HashMap; @@ -694,6 +699,38 @@ mod tests { .expect_err("native absolute sandbox cwd should not deserialize as a URI"); } + #[test] + fn filesystem_protocol_round_trips_permission_paths_as_uris() { + let native_cwd = std::env::current_dir().expect("current directory"); + let cwd = PathUri::from_host_native_path(&native_cwd).expect("cwd URI"); + let mut file_system_policy = + FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: native_cwd.try_into().expect("absolute cwd"), + }, + access: FileSystemAccessMode::Read, + }]); + file_system_policy.glob_scan_max_depth = Some(2); + let permissions = PermissionProfile::from_runtime_permissions( + &file_system_policy, + NetworkSandboxPolicy::Restricted, + ); + let sandbox = + FileSystemSandboxContext::from_permission_profile_with_cwd(permissions, cwd.clone()); + + let serialized = serde_json::to_value(&sandbox).expect("serialize sandbox"); + + assert_eq!( + serialized["permissions"]["file_system"]["entries"][0]["path"]["path"], + serde_json::json!(cwd.to_string()) + ); + assert_eq!( + serde_json::from_value::(serialized) + .expect("deserialize sandbox"), + sandbox + ); + } + #[test] fn http_request_timeout_treats_omitted_and_null_as_no_timeout() { let omitted: HttpRequestParams = serde_json::from_value(serde_json::json!({ diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index e402ef3382..c9effdc20c 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -1,27 +1,30 @@ mod find_up; -pub use find_up::FindUpErrorPolicy; -pub use find_up::find_nearest_ancestor_with_markers; -pub use find_up::find_nearest_native_ancestor_with_markers; - use bytes::Bytes; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::ManagedFileSystemPermissions; use codex_protocol::models::PermissionProfile; use codex_protocol::models::SandboxEnforcement; +use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; +pub use find_up::FindUpErrorPolicy; +pub use find_up::find_nearest_ancestor_with_markers; +pub use find_up::find_nearest_native_ancestor_with_markers; use futures::Stream; +use serde::Deserialize; +use serde::Serialize; use std::collections::HashSet; use std::collections::VecDeque; use std::future::Future; use std::io; +use std::num::NonZeroUsize; use std::path::Path; use std::pin::Pin; use std::task::Context; @@ -119,10 +122,163 @@ pub struct WalkOutcome { pub truncated: bool, } -#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum ExecFileSystemPath { + Path { path: PathUri }, + GlobPattern { pattern: String }, + Special { value: FileSystemSpecialPath }, +} + +impl From for ExecFileSystemPath { + fn from(value: FileSystemPath) -> Self { + match value { + FileSystemPath::Path { path } => Self::Path { + path: PathUri::from_abs_path(&path), + }, + FileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, + FileSystemPath::Special { value } => Self::Special { value }, + } + } +} + +impl TryFrom for FileSystemPath { + type Error = io::Error; + + fn try_from(value: ExecFileSystemPath) -> Result { + Ok(match value { + ExecFileSystemPath::Path { path } => Self::Path { + path: path.to_abs_path()?, + }, + ExecFileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, + ExecFileSystemPath::Special { value } => Self::Special { value }, + }) + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +pub struct ExecFileSystemSandboxEntry { + pub path: ExecFileSystemPath, + pub access: FileSystemAccessMode, +} + +impl From for ExecFileSystemSandboxEntry { + fn from(value: FileSystemSandboxEntry) -> Self { + Self { + path: value.path.into(), + access: value.access, + } + } +} + +impl TryFrom for FileSystemSandboxEntry { + type Error = io::Error; + + fn try_from(value: ExecFileSystemSandboxEntry) -> Result { + Ok(Self { + path: value.path.try_into()?, + access: value.access, + }) + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum ExecManagedFileSystemPermissions { + Restricted { + entries: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + glob_scan_max_depth: Option, + }, + Unrestricted, +} + +impl From for ExecManagedFileSystemPermissions { + fn from(value: ManagedFileSystemPermissions) -> Self { + match value { + ManagedFileSystemPermissions::Restricted { + entries, + glob_scan_max_depth, + } => Self::Restricted { + entries: entries.into_iter().map(Into::into).collect(), + glob_scan_max_depth, + }, + ManagedFileSystemPermissions::Unrestricted => Self::Unrestricted, + } + } +} + +impl TryFrom for ManagedFileSystemPermissions { + type Error = io::Error; + + fn try_from(value: ExecManagedFileSystemPermissions) -> Result { + Ok(match value { + ExecManagedFileSystemPermissions::Restricted { + entries, + glob_scan_max_depth, + } => Self::Restricted { + entries: entries + .into_iter() + .map(TryInto::try_into) + .collect::>()?, + glob_scan_max_depth, + }, + ExecManagedFileSystemPermissions::Unrestricted => Self::Unrestricted, + }) + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum ExecPermissionProfile { + Managed { + file_system: ExecManagedFileSystemPermissions, + network: NetworkSandboxPolicy, + }, + Disabled, + External { + network: NetworkSandboxPolicy, + }, +} + +impl From for ExecPermissionProfile { + fn from(value: PermissionProfile) -> Self { + match value { + PermissionProfile::Managed { + file_system, + network, + } => Self::Managed { + file_system: file_system.into(), + network, + }, + PermissionProfile::Disabled => Self::Disabled, + PermissionProfile::External { network } => Self::External { network }, + } + } +} + +impl TryFrom for PermissionProfile { + type Error = io::Error; + + fn try_from(value: ExecPermissionProfile) -> Result { + Ok(match value { + ExecPermissionProfile::Managed { + file_system, + network, + } => Self::Managed { + file_system: file_system.try_into()?, + network, + }, + ExecPermissionProfile::Disabled => Self::Disabled, + ExecPermissionProfile::External { network } => Self::External { network }, + }) + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct FileSystemSandboxContext { - pub permissions: PermissionProfile, + pub permissions: ExecPermissionProfile, #[serde(default, skip_serializing_if = "Option::is_none")] pub cwd: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -147,30 +303,23 @@ impl FileSystemSandboxContext { &sandbox_policy, &native_cwd, ); - let permissions = - PermissionProfile::::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - NetworkSandboxPolicy::from(&sandbox_policy), - ); + let permissions = PermissionProfile::from_runtime_permissions_with_enforcement( + SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), + &file_system_sandbox_policy, + NetworkSandboxPolicy::from(&sandbox_policy), + ); Ok(Self::from_permission_profile_with_cwd(permissions, cwd)) } - pub fn from_permission_profile(permissions: PermissionProfile) -> Self { + pub fn from_permission_profile(permissions: PermissionProfile) -> Self { Self::from_permissions_and_cwd(permissions, /*cwd*/ None) } - pub fn from_permission_profile_with_cwd( - permissions: PermissionProfile, - cwd: PathUri, - ) -> Self { + pub fn from_permission_profile_with_cwd(permissions: PermissionProfile, cwd: PathUri) -> Self { Self::from_permissions_and_cwd(permissions, Some(cwd)) } - fn from_permissions_and_cwd( - permissions: PermissionProfile, - cwd: Option, - ) -> Self { + fn from_permissions_and_cwd(permissions: PermissionProfile, cwd: Option) -> Self { let workspace_roots = cwd.iter().cloned().collect(); Self { permissions: permissions.into(), @@ -183,9 +332,7 @@ impl FileSystemSandboxContext { } pub fn should_run_in_sandbox(&self) -> bool { - let Ok(permissions) = - PermissionProfile::::try_from(self.permissions.clone()) - else { + let Ok(permissions) = PermissionProfile::try_from(self.permissions.clone()) else { // A sandbox context for another host must not select the unsandboxed filesystem. return true; }; @@ -196,22 +343,22 @@ impl FileSystemSandboxContext { pub fn has_cwd_dependent_permissions(&self) -> bool { match &self.permissions { - PermissionProfile::Managed { - file_system: ManagedFileSystemPermissions::Restricted { entries, .. }, + ExecPermissionProfile::Managed { + file_system: ExecManagedFileSystemPermissions::Restricted { entries, .. }, .. } => entries.iter().any(|entry| match &entry.path { - FileSystemPath::GlobPattern { pattern } => !Path::new(pattern).is_absolute(), - FileSystemPath::Special { + ExecFileSystemPath::GlobPattern { pattern } => !Path::new(pattern).is_absolute(), + ExecFileSystemPath::Special { value: FileSystemSpecialPath::ProjectRoots { .. }, } => true, - FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => false, + ExecFileSystemPath::Path { .. } | ExecFileSystemPath::Special { .. } => false, }), - PermissionProfile::Managed { - file_system: ManagedFileSystemPermissions::Unrestricted, + ExecPermissionProfile::Managed { + file_system: ExecManagedFileSystemPermissions::Unrestricted, .. } - | PermissionProfile::Disabled - | PermissionProfile::External { .. } => false, + | ExecPermissionProfile::Disabled + | ExecPermissionProfile::External { .. } => false, } } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 91fd42a555..287780e5b5 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -23,7 +23,6 @@ use crate::protocol::SandboxPolicy; use crate::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_image::ImageProcessingError; -use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use crate::ResponseItemId; @@ -64,59 +63,29 @@ impl SandboxPermissions { } } -#[derive(Debug, Clone, Eq, Hash, PartialEq, JsonSchema, TS)] -pub struct FileSystemPermissions { - pub entries: Vec>, +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, JsonSchema, TS)] +pub struct FileSystemPermissions { + pub entries: Vec, pub glob_scan_max_depth: Option, } -impl From> for FileSystemPermissions { - fn from(value: FileSystemPermissions) -> Self { - FileSystemPermissions { - entries: value - .entries - .into_iter() - .map(FileSystemSandboxEntry::::from) - .collect(), - glob_scan_max_depth: value.glob_scan_max_depth, - } - } +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct LegacyReadWriteRoots { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub read: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub write: Option>, } -impl TryFrom> for FileSystemPermissions { - type Error = io::Error; - - fn try_from(value: FileSystemPermissions) -> Result { - Ok(FileSystemPermissions { - entries: value - .entries - .into_iter() - .map(FileSystemSandboxEntry::::try_from) - .collect::>()?, - glob_scan_max_depth: value.glob_scan_max_depth, - }) - } -} - -impl Default for FileSystemPermissions { - fn default() -> Self { - Self { - entries: Vec::new(), - glob_scan_max_depth: None, - } - } -} - -pub type LegacyReadWriteRoots = - (Option>, Option>); -impl FileSystemPermissions { +impl FileSystemPermissions { pub fn is_empty(&self) -> bool { self.entries.is_empty() } pub fn from_read_write_roots( - read: Option>, - write: Option>, + read: Option>, + write: Option>, ) -> Self { let mut entries = Vec::new(); if let Some(read) = read { @@ -137,25 +106,20 @@ impl FileSystemPermissions { } } - pub fn explicit_path_entries(&self) -> impl Iterator { + pub fn explicit_path_entries( + &self, + ) -> impl Iterator { self.entries.iter().filter_map(|entry| match &entry.path { FileSystemPath::Path { path } => Some((path, entry.access)), FileSystemPath::GlobPattern { .. } | FileSystemPath::Special { .. } => None, }) } - pub fn legacy_read_write_roots(&self) -> Option> - where - PathType: Clone, - { + pub fn legacy_read_write_roots(&self) -> Option { self.as_legacy_permissions() - .map(|legacy| (legacy.read, legacy.write)) } - fn as_legacy_permissions(&self) -> Option> - where - PathType: Clone, - { + fn as_legacy_permissions(&self) -> Option { if self.glob_scan_max_depth.is_some() { return None; } @@ -174,7 +138,7 @@ impl FileSystemPermissions { } } - Some(LegacyFileSystemPermissions { + Some(LegacyReadWriteRoots { read: (!read.is_empty()).then_some(read), write: (!write.is_empty()).then_some(write), }) @@ -183,35 +147,21 @@ impl FileSystemPermissions { #[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] -#[serde(bound(deserialize = "PathType: Deserialize<'de>"))] -struct LegacyFileSystemPermissions { - #[serde(default, skip_serializing_if = "Option::is_none")] - read: Option>, - #[serde(default, skip_serializing_if = "Option::is_none")] - write: Option>, -} - -#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -#[serde(bound(deserialize = "PathType: Deserialize<'de>"))] -struct CanonicalFileSystemPermissions { +struct CanonicalFileSystemPermissions { #[serde(default, skip_serializing_if = "Vec::is_empty")] - entries: Vec>, + entries: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] glob_scan_max_depth: Option, } #[derive(Debug, Clone, Deserialize)] #[serde(untagged)] -enum FileSystemPermissionsDe { - Canonical(CanonicalFileSystemPermissions), - Legacy(LegacyFileSystemPermissions), +enum FileSystemPermissionsDe { + Canonical(CanonicalFileSystemPermissions), + Legacy(LegacyReadWriteRoots), } -impl Serialize for FileSystemPermissions -where - PathType: Clone + Serialize, -{ +impl Serialize for FileSystemPermissions { fn serialize(&self, serializer: S) -> Result where S: Serializer, @@ -228,10 +178,7 @@ where } } -impl<'de, PathType> Deserialize<'de> for FileSystemPermissions -where - PathType: Deserialize<'de>, -{ +impl<'de> Deserialize<'de> for FileSystemPermissions { fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, @@ -244,7 +191,7 @@ where entries, glob_scan_max_depth, }), - FileSystemPermissionsDe::Legacy(LegacyFileSystemPermissions { read, write }) => { + FileSystemPermissionsDe::Legacy(LegacyReadWriteRoots { read, write }) => { Ok(Self::from_read_write_roots(read, write)) } } @@ -304,12 +251,12 @@ impl SandboxEnforcement { #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] #[ts(tag = "type")] -pub enum ManagedFileSystemPermissions { +pub enum ManagedFileSystemPermissions { /// Apply a managed filesystem sandbox from the listed entries. #[serde(rename_all = "snake_case")] #[ts(rename_all = "snake_case")] Restricted { - entries: Vec>, + entries: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] glob_scan_max_depth: Option, @@ -318,50 +265,6 @@ pub enum ManagedFileSystemPermissions { Unrestricted, } -impl From> for ManagedFileSystemPermissions { - fn from(value: ManagedFileSystemPermissions) -> Self { - match value { - ManagedFileSystemPermissions::Restricted { - entries, - glob_scan_max_depth, - } => ManagedFileSystemPermissions::Restricted { - entries: entries - .into_iter() - .map(FileSystemSandboxEntry::::from) - .collect(), - glob_scan_max_depth, - }, - ManagedFileSystemPermissions::Unrestricted => { - ManagedFileSystemPermissions::Unrestricted - } - } - } -} - -impl TryFrom> - for ManagedFileSystemPermissions -{ - type Error = io::Error; - - fn try_from(value: ManagedFileSystemPermissions) -> Result { - Ok(match value { - ManagedFileSystemPermissions::Restricted { - entries, - glob_scan_max_depth, - } => ManagedFileSystemPermissions::Restricted { - entries: entries - .into_iter() - .map(FileSystemSandboxEntry::::try_from) - .collect::>()?, - glob_scan_max_depth, - }, - ManagedFileSystemPermissions::Unrestricted => { - ManagedFileSystemPermissions::Unrestricted - } - }) - } -} - impl ManagedFileSystemPermissions { fn from_sandbox_policy(file_system_sandbox_policy: &FileSystemSandboxPolicy) -> Self { match file_system_sandbox_policy.kind { @@ -406,12 +309,12 @@ pub const BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS: &str = ":danger-full-a #[derive(Debug, Clone, Eq, PartialEq, Serialize, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] #[ts(tag = "type")] -pub enum PermissionProfile { +pub enum PermissionProfile { /// Codex owns sandbox construction for this profile. #[serde(rename_all = "snake_case")] #[ts(rename_all = "snake_case")] Managed { - file_system: ManagedFileSystemPermissions, + file_system: ManagedFileSystemPermissions, network: NetworkSandboxPolicy, }, /// Do not apply an outer sandbox. @@ -422,40 +325,6 @@ pub enum PermissionProfile { External { network: NetworkSandboxPolicy }, } -impl From> for PermissionProfile { - fn from(value: PermissionProfile) -> Self { - match value { - PermissionProfile::Managed { - file_system, - network, - } => PermissionProfile::Managed { - file_system: file_system.into(), - network, - }, - PermissionProfile::Disabled => PermissionProfile::Disabled, - PermissionProfile::External { network } => PermissionProfile::External { network }, - } - } -} - -impl TryFrom> for PermissionProfile { - type Error = io::Error; - - fn try_from(value: PermissionProfile) -> Result { - Ok(match value { - PermissionProfile::Managed { - file_system, - network, - } => PermissionProfile::Managed { - file_system: file_system.try_into()?, - network, - }, - PermissionProfile::Disabled => PermissionProfile::Disabled, - PermissionProfile::External { network } => PermissionProfile::External { network }, - }) - } -} - /// Metadata for the named or implicit built-in permissions profile that /// produced the active `PermissionProfile`. /// @@ -489,7 +358,7 @@ impl ActivePermissionProfile { } } -impl Default for PermissionProfile { +impl Default for PermissionProfile { fn default() -> Self { Self::Managed { file_system: ManagedFileSystemPermissions::Restricted { @@ -677,10 +546,10 @@ impl PermissionProfile { #[derive(Debug, Clone, Deserialize)] #[serde(tag = "type", rename_all = "snake_case")] -enum TaggedPermissionProfile { +enum TaggedPermissionProfile { #[serde(rename_all = "snake_case")] Managed { - file_system: ManagedFileSystemPermissions, + file_system: ManagedFileSystemPermissions, network: NetworkSandboxPolicy, }, Disabled, @@ -690,8 +559,8 @@ enum TaggedPermissionProfile { }, } -impl From> for PermissionProfile { - fn from(value: TaggedPermissionProfile) -> Self { +impl From for PermissionProfile { + fn from(value: TaggedPermissionProfile) -> Self { match value { TaggedPermissionProfile::Managed { file_system, @@ -710,13 +579,13 @@ impl From> for PermissionProfile { +struct LegacyPermissionProfile { network: Option, - file_system: Option>, + file_system: Option, } -impl From> for PermissionProfile { - fn from(value: LegacyPermissionProfile) -> Self { +impl From for PermissionProfile { + fn from(value: LegacyPermissionProfile) -> Self { let file_system = value.file_system.map_or_else( || ManagedFileSystemPermissions::Restricted { entries: Vec::new(), @@ -746,15 +615,12 @@ impl From> for PermissionProfile { - Tagged(TaggedPermissionProfile), - Legacy(LegacyPermissionProfile), +enum PermissionProfileDe { + Tagged(TaggedPermissionProfile), + Legacy(LegacyPermissionProfile), } -impl<'de, PathType> Deserialize<'de> for PermissionProfile -where - PathType: Deserialize<'de>, -{ +impl<'de> Deserialize<'de> for PermissionProfile { fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index eba2f0e0aa..ec37292e18 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -6,7 +6,6 @@ use std::path::PathBuf; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_preserving_symlinks; -use codex_utils_path_uri::PathUri; use globset::GlobBuilder; use globset::GlobMatcher; use schemars::JsonSchema; @@ -177,31 +176,11 @@ impl FileSystemSpecialPath { } #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] -pub struct FileSystemSandboxEntry { - pub path: FileSystemPath, +pub struct FileSystemSandboxEntry { + pub path: FileSystemPath, pub access: FileSystemAccessMode, } -impl From> for FileSystemSandboxEntry { - fn from(value: FileSystemSandboxEntry) -> Self { - FileSystemSandboxEntry { - path: value.path.into(), - access: value.access, - } - } -} - -impl TryFrom> for FileSystemSandboxEntry { - type Error = io::Error; - - fn try_from(value: FileSystemSandboxEntry) -> Result { - Ok(FileSystemSandboxEntry { - path: value.path.try_into()?, - access: value.access, - }) - } -} - #[derive( Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Display, Default, JsonSchema, TS, )] @@ -359,9 +338,10 @@ enum InvalidDenyReadGlobBehavior { #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] #[ts(tag = "type")] -pub enum FileSystemPath { +pub enum FileSystemPath { Path { - path: PathType, + // TODO(anp): Use PathUri once permission paths no longer require native-path rollout serialization. + path: AbsolutePathBuf, }, /// A git-style glob pattern. Pattern entries currently support /// FileSystemAccessMode::Deny only. @@ -373,32 +353,6 @@ pub enum FileSystemPath { }, } -impl From> for FileSystemPath { - fn from(value: FileSystemPath) -> Self { - match value { - FileSystemPath::Path { path } => FileSystemPath::Path { - path: PathUri::from_abs_path(&path), - }, - FileSystemPath::GlobPattern { pattern } => FileSystemPath::GlobPattern { pattern }, - FileSystemPath::Special { value } => FileSystemPath::Special { value }, - } - } -} - -impl TryFrom> for FileSystemPath { - type Error = io::Error; - - fn try_from(value: FileSystemPath) -> Result { - Ok(match value { - FileSystemPath::Path { path } => FileSystemPath::Path { - path: path.to_abs_path()?, - }, - FileSystemPath::GlobPattern { pattern } => FileSystemPath::GlobPattern { pattern }, - FileSystemPath::Special { value } => FileSystemPath::Special { value }, - }) - } -} - const PROJECT_ROOTS_GLOB_PATTERN_PREFIX: &str = "codex-project-roots://"; pub fn project_roots_glob_pattern(subpath: &Path) -> String {