diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index e5287e1c65..b55e14f4ad 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -253,6 +262,217 @@ } ] }, + "FileSystemAccessMode": { + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "pattern": { + "type": "string" + }, + "type": { + "enum": [ + "glob_pattern" + ], + "title": "GlobPatternFileSystemPathType", + "type": "string" + } + }, + "required": [ + "pattern", + "type" + ], + "title": "GlobPatternFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "NetworkApprovalContext": { "properties": { "host": { diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json index ac8d5c4010..2c8bfba83c 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -39,6 +48,217 @@ }, "type": "object" }, + "FileSystemAccessMode": { + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "pattern": { + "type": "string" + }, + "type": { + "enum": [ + "glob_pattern" + ], + "title": "GlobPatternFileSystemPathType", + "type": "string" + } + }, + "required": [ + "pattern", + "type" + ], + "title": "GlobPatternFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "RequestPermissionProfile": { "additionalProperties": false, "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json index 7b0c2b1a3b..4564138527 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -39,6 +48,217 @@ }, "type": "object" }, + "FileSystemAccessMode": { + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "pattern": { + "type": "string" + }, + "type": { + "enum": [ + "glob_pattern" + ], + "title": "GlobPatternFileSystemPathType", + "type": "string" + } + }, + "required": [ + "pattern", + "type" + ], + "title": "GlobPatternFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "GrantedPermissionProfile": { "properties": { "fileSystem": { diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index b31e69f203..b778ae0205 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/AbsolutePathBuf" @@ -586,6 +595,217 @@ ], "type": "object" }, + "FileSystemAccessMode": { + "enum": [ + "read", + "write", + "none" + ], + "type": "string" + }, + "FileSystemPath": { + "oneOf": [ + { + "properties": { + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": { + "enum": [ + "path" + ], + "title": "PathFileSystemPathType", + "type": "string" + } + }, + "required": [ + "path", + "type" + ], + "title": "PathFileSystemPath", + "type": "object" + }, + { + "properties": { + "pattern": { + "type": "string" + }, + "type": { + "enum": [ + "glob_pattern" + ], + "title": "GlobPatternFileSystemPathType", + "type": "string" + } + }, + "required": [ + "pattern", + "type" + ], + "title": "GlobPatternFileSystemPath", + "type": "object" + }, + { + "properties": { + "type": { + "enum": [ + "special" + ], + "title": "SpecialFileSystemPathType", + "type": "string" + }, + "value": { + "$ref": "#/definitions/FileSystemSpecialPath" + } + }, + "required": [ + "type", + "value" + ], + "title": "SpecialFileSystemPath", + "type": "object" + } + ] + }, + "FileSystemSandboxEntry": { + "properties": { + "access": { + "$ref": "#/definitions/FileSystemAccessMode" + }, + "path": { + "$ref": "#/definitions/FileSystemPath" + } + }, + "required": [ + "access", + "path" + ], + "type": "object" + }, + "FileSystemSpecialPath": { + "oneOf": [ + { + "properties": { + "kind": { + "enum": [ + "root" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "RootFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "minimal" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "MinimalFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "current_working_directory" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "CurrentWorkingDirectoryFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "project_roots" + ], + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind" + ], + "title": "KindFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "tmpdir" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "TmpdirFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "slash_tmp" + ], + "type": "string" + } + }, + "required": [ + "kind" + ], + "title": "SlashTmpFileSystemSpecialPath", + "type": "object" + }, + { + "properties": { + "kind": { + "enum": [ + "unknown" + ], + "type": "string" + }, + "path": { + "type": "string" + }, + "subpath": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "kind", + "path" + ], + "type": "object" + } + ] + }, "McpElicitationArrayType": { "enum": [ "array" 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 a532b66b2c..60517b416b 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 @@ -7,6 +7,15 @@ }, "AdditionalFileSystemPermissions": { "properties": { + "entries": { + "items": { + "$ref": "#/definitions/v2/FileSystemSandboxEntry" + }, + "type": [ + "array", + "null" + ] + }, "read": { "items": { "$ref": "#/definitions/v2/AbsolutePathBuf" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts index 0ed8a1b12f..21af5dda71 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts @@ -2,5 +2,6 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "../AbsolutePathBuf"; +import type { FileSystemSandboxEntry } from "./FileSystemSandboxEntry"; -export type AdditionalFileSystemPermissions = { read: Array | null, write: Array | null, }; +export type AdditionalFileSystemPermissions = { read: Array | null, write: Array | null, entries?: Array, }; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index db42843246..2198adf05c 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -2041,6 +2041,7 @@ mod tests { file_system: Some(v2::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/allowed")]), write: None, + entries: None, }), }), proposed_execpolicy_amendment: None, diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index f10cf7d599..8d183035cb 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1155,22 +1155,46 @@ impl From for NetworkApprovalContext { pub struct AdditionalFileSystemPermissions { pub read: Option>, pub write: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub entries: Option>, } impl From for AdditionalFileSystemPermissions { fn from(value: CoreFileSystemPermissions) -> Self { - Self { - read: value.read, - write: value.write, + if let Some((read, write)) = value.legacy_read_write_roots() { + Self { + read, + write, + entries: None, + } + } else { + Self { + read: None, + write: None, + entries: Some( + value + .entries + .into_iter() + .map(FileSystemSandboxEntry::from) + .collect(), + ), + } } } } impl From for CoreFileSystemPermissions { fn from(value: AdditionalFileSystemPermissions) -> Self { - Self { - read: value.read, - write: value.write, + if let Some(entries) = value.entries { + Self { + entries: entries + .into_iter() + .map(CoreFileSystemSandboxEntry::from) + .collect(), + } + } else { + CoreFileSystemPermissions::from_read_write_roots(value.read, value.write) } } } @@ -6756,6 +6780,7 @@ mod tests { AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), + entries: None, }), } ); @@ -6766,16 +6791,16 @@ mod tests { network: Some(CoreNetworkPermissions { enabled: Some(true), }), - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![ + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) .expect("path must be absolute"), ]), - write: Some(vec![ + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), - }), + )), } ); } @@ -6809,6 +6834,53 @@ mod tests { ); } + #[test] + fn additional_file_system_permissions_preserves_canonical_entries() { + let core_permissions = CoreFileSystemPermissions { + entries: vec![ + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::Special { + value: CoreFileSystemSpecialPath::Root, + }, + access: CoreFileSystemAccessMode::Write, + }, + CoreFileSystemSandboxEntry { + path: CoreFileSystemPath::GlobPattern { + pattern: "**/*.env".to_string(), + }, + access: CoreFileSystemAccessMode::None, + }, + ], + }; + + let permissions = AdditionalFileSystemPermissions::from(core_permissions.clone()); + assert_eq!( + permissions, + AdditionalFileSystemPermissions { + read: None, + write: None, + entries: Some(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: "**/*.env".to_string(), + }, + access: FileSystemAccessMode::None, + }, + ]), + } + ); + assert_eq!( + CoreFileSystemPermissions::from(permissions), + core_permissions + ); + } + #[test] fn permissions_request_approval_response_uses_granted_permission_profile_without_macos() { let read_only_path = if cfg!(windows) { @@ -6849,6 +6921,7 @@ mod tests { AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), + entries: None, }), } ); @@ -6859,16 +6932,16 @@ mod tests { network: Some(CoreNetworkPermissions { enabled: Some(true), }), - file_system: Some(CoreFileSystemPermissions { - read: Some(vec![ + file_system: Some(CoreFileSystemPermissions::from_read_write_roots( + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_only_path)) .expect("path must be absolute"), ]), - write: Some(vec![ + Some(vec![ AbsolutePathBuf::try_from(PathBuf::from(read_write_path)) .expect("path must be absolute"), ]), - }), + )), } ); } diff --git a/codex-rs/app-server/src/transport/mod.rs b/codex-rs/app-server/src/transport/mod.rs index 75a4971905..680caf0dd5 100644 --- a/codex-rs/app-server/src/transport/mod.rs +++ b/codex-rs/app-server/src/transport/mod.rs @@ -780,6 +780,7 @@ mod tests { codex_app_server_protocol::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/allowed")]), write: None, + entries: None, }, ), }, @@ -842,6 +843,7 @@ mod tests { codex_app_server_protocol::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/allowed")]), write: None, + entries: None, }, ), }, diff --git a/codex-rs/app-server/tests/suite/v2/request_permissions.rs b/codex-rs/app-server/tests/suite/v2/request_permissions.rs index 5a0679415d..05a45011de 100644 --- a/codex-rs/app-server/tests/suite/v2/request_permissions.rs +++ b/codex-rs/app-server/tests/suite/v2/request_permissions.rs @@ -93,6 +93,7 @@ async fn request_permissions_round_trip() -> Result<()> { file_system: Some(codex_app_server_protocol::AdditionalFileSystemPermissions { read: None, write: Some(vec![requested_writes[0].clone()]), + entries: None, }), }, scope: PermissionGrantScope::Turn, diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index e713c59b4e..675be5fecf 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -196,10 +196,10 @@ fn write_permissions_for_paths( .ok()?; let permissions = (!write_paths.is_empty()).then_some(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(write_paths), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(write_paths), + )), ..Default::default() })?; diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 63dd771b08..bae937be8f 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -238,12 +238,12 @@ mod tests { fn file_system_permissions(path: &std::path::Path) -> PermissionProfile { PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![ + file_system: Some(FileSystemPermissions::from_read_write_roots( + None, + Some(vec![ AbsolutePathBuf::from_absolute_path(path).expect("absolute path"), ]), - }), + )), ..Default::default() } } diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 7b0ac256d4..87ca4be18a 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -3,6 +3,9 @@ use std::collections::HashMap; use codex_app_server_protocol::JSONRPCErrorError; use codex_protocol::models::FileSystemPermissions; 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_protocol::protocol::ReadOnlyAccess; @@ -121,16 +124,23 @@ impl FileSystemSandboxRunner { match additional_permissions.and_then(|permissions| permissions.file_system.clone()) { Some(mut file_system) => { if let Some(helper_read_root) = &helper_read_root { - let read_paths = file_system.read.get_or_insert_with(Vec::new); - if !read_paths.contains(helper_read_root) { - read_paths.push(helper_read_root.clone()); + let helper_read_entry = FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: helper_read_root.clone(), + }, + access: FileSystemAccessMode::Read, + }; + if !file_system.entries.contains(&helper_read_entry) { + file_system.entries.push(helper_read_entry); } } Some(file_system) } - None => helper_read_root.map(|helper_read_root| FileSystemPermissions { - read: Some(vec![helper_read_root]), - write: None, + None => helper_read_root.map(|helper_read_root| { + FileSystemPermissions::from_read_write_roots( + Some(vec![helper_read_root]), + /*write*/ None, + ) }), }; @@ -373,27 +383,20 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![writable.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![writable.clone()]), + )), })); + let (read, write) = permissions + .file_system + .as_ref() + .and_then(FileSystemPermissions::legacy_read_write_roots) + .expect("helper permissions should stay lossless as legacy read/write roots"); assert_eq!(permissions.network, None); - assert_eq!( - permissions - .file_system - .as_ref() - .and_then(|fs| fs.write.clone()), - Some(vec![writable]) - ); - assert_eq!( - permissions - .file_system - .as_ref() - .and_then(|fs| fs.read.clone()), - Some(vec![readable]) - ); + assert_eq!(write, Some(vec![writable])); + assert_eq!(read, Some(vec![readable])); } #[test] @@ -415,10 +418,10 @@ mod tests { assert_eq!(permissions.network, None); assert_eq!( permissions.file_system, - Some(FileSystemPermissions { - read: Some(vec![readable]), - write: None, - }) + Some(FileSystemPermissions::from_read_write_roots( + Some(vec![readable]), + None, + )) ); } } diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index 8b9fb90ecc..49c5cf4f05 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -475,10 +475,10 @@ async fn file_system_sandboxed_write_allows_additional_write_root(use_remote: bo let mut sandbox = read_only_sandbox(readable_dir); sandbox.additional_permissions = Some(PermissionProfile { network: None, - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![absolute_path(writable_dir)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + None, + Some(vec![absolute_path(writable_dir)]), + )), }); file_system diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 8c55d056cd..edb54acfbb 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::fmt; +use std::io; use std::path::Path; use std::path::PathBuf; use std::sync::LazyLock; @@ -16,6 +17,13 @@ use ts_rs::TS; use crate::config_types::ApprovalsReviewer; use crate::config_types::CollaborationMode; use crate::config_types::SandboxMode; +use crate::permissions::FileSystemAccessMode; +use crate::permissions::FileSystemPath; +use crate::permissions::FileSystemSandboxEntry; +use crate::permissions::FileSystemSandboxKind; +use crate::permissions::FileSystemSandboxPolicy; +use crate::permissions::FileSystemSpecialPath; +use crate::permissions::NetworkSandboxPolicy; use crate::protocol::AskForApproval; use crate::protocol::COLLABORATION_MODE_CLOSE_TAG; use crate::protocol::COLLABORATION_MODE_OPEN_TAG; @@ -135,15 +143,126 @@ impl SandboxPermissions { } } -#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, JsonSchema, TS)] pub struct FileSystemPermissions { - pub read: Option>, - pub write: Option>, + pub entries: Vec, } +pub type LegacyReadWriteRoots = (Option>, Option>); + impl FileSystemPermissions { pub fn is_empty(&self) -> bool { - self.read.is_none() && self.write.is_none() + self.entries.is_empty() + } + + pub fn from_read_write_roots( + read: Option>, + write: Option>, + ) -> Self { + let mut entries = Vec::new(); + if let Some(read) = read { + entries.extend(read.into_iter().map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Read, + })); + } + if let Some(write) = write { + entries.extend(write.into_iter().map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Write, + })); + } + Self { entries } + } + + 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 { + self.as_legacy_permissions() + .map(|legacy| (legacy.read, legacy.write)) + } + + fn as_legacy_permissions(&self) -> Option { + let mut read = Vec::new(); + let mut write = Vec::new(); + + for entry in &self.entries { + let FileSystemPath::Path { path } = &entry.path else { + return None; + }; + match entry.access { + FileSystemAccessMode::Read => read.push(path.clone()), + FileSystemAccessMode::Write => write.push(path.clone()), + FileSystemAccessMode::None => return None, + } + } + + Some(LegacyFileSystemPermissions { + read: (!read.is_empty()).then_some(read), + write: (!write.is_empty()).then_some(write), + }) + } +} + +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +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)] +struct CanonicalFileSystemPermissions { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + entries: Vec, +} + +#[derive(Debug, Clone, Deserialize)] +#[serde(untagged)] +enum FileSystemPermissionsDe { + Canonical(CanonicalFileSystemPermissions), + Legacy(LegacyFileSystemPermissions), +} + +impl Serialize for FileSystemPermissions { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + if let Some(legacy) = self.as_legacy_permissions() { + legacy.serialize(serializer) + } else { + CanonicalFileSystemPermissions { + entries: self.entries.clone(), + } + .serialize(serializer) + } + } +} + +impl<'de> Deserialize<'de> for FileSystemPermissions { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + match FileSystemPermissionsDe::deserialize(deserializer)? { + FileSystemPermissionsDe::Canonical(CanonicalFileSystemPermissions { entries }) => { + Ok(Self { entries }) + } + FileSystemPermissionsDe::Legacy(LegacyFileSystemPermissions { read, write }) => { + Ok(Self::from_read_write_roots(read, write)) + } + } } } @@ -168,6 +287,79 @@ impl PermissionProfile { pub fn is_empty(&self) -> bool { self.network.is_none() && self.file_system.is_none() } + + pub fn from_runtime_permissions( + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + ) -> Self { + Self { + network: Some(network_sandbox_policy.into()), + file_system: Some(file_system_sandbox_policy.into()), + } + } + + pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self { + Self::from_runtime_permissions( + &FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, cwd), + NetworkSandboxPolicy::from(sandbox_policy), + ) + } + + pub fn file_system_sandbox_policy(&self) -> FileSystemSandboxPolicy { + self.file_system.as_ref().map_or_else( + || FileSystemSandboxPolicy::restricted(Vec::new()), + FileSystemSandboxPolicy::from, + ) + } + + pub fn network_sandbox_policy(&self) -> NetworkSandboxPolicy { + if self + .network + .as_ref() + .and_then(|network| network.enabled) + .unwrap_or(false) + { + NetworkSandboxPolicy::Enabled + } else { + NetworkSandboxPolicy::Restricted + } + } + + pub fn to_legacy_sandbox_policy(&self, cwd: &Path) -> io::Result { + self.file_system_sandbox_policy() + .to_legacy_sandbox_policy(self.network_sandbox_policy(), cwd) + } +} + +impl From for NetworkPermissions { + fn from(value: NetworkSandboxPolicy) -> Self { + Self { + enabled: Some(value.is_enabled()), + } + } +} + +impl From<&FileSystemSandboxPolicy> for FileSystemPermissions { + fn from(value: &FileSystemSandboxPolicy) -> Self { + let entries = match value.kind { + FileSystemSandboxKind::Restricted => value.entries.clone(), + FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => { + vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }] + } + }; + Self { entries } + } +} + +impl From<&FileSystemPermissions> for FileSystemSandboxPolicy { + fn from(value: &FileSystemPermissions) -> Self { + FileSystemSandboxPolicy::restricted(value.entries.clone()) + } } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 80cfa823e7..ed56567454 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -46,6 +46,7 @@ impl NetworkSandboxPolicy { Debug, Clone, Copy, + Hash, PartialEq, Eq, PartialOrd, @@ -74,7 +75,7 @@ impl FileSystemAccessMode { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] #[serde(tag = "kind", rename_all = "snake_case")] #[ts(tag = "kind")] pub enum FileSystemSpecialPath { @@ -117,7 +118,7 @@ impl FileSystemSpecialPath { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] pub struct FileSystemSandboxEntry { pub path: FileSystemPath, pub access: FileSystemAccessMode, @@ -239,7 +240,7 @@ impl ReadDenyMatcher { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] #[ts(tag = "type")] pub enum FileSystemPath { @@ -755,13 +756,14 @@ impl FileSystemSandboxPolicy { FileSystemSandboxKind::Restricted => { let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok(); let mut include_platform_defaults = false; - let mut has_full_disk_read_access = false; - let mut has_full_disk_write_access = false; + let has_full_disk_read_access = self.has_full_disk_read_access(); + let has_full_disk_write_access = self.has_full_disk_write_access(); let mut workspace_root_writable = false; let mut writable_roots = Vec::new(); let mut readable_roots = Vec::new(); let mut tmpdir_writable = false; let mut slash_tmp_writable = false; + let mut unbridgeable_root_write = false; for entry in &self.entries { match &entry.path { @@ -780,10 +782,15 @@ impl FileSystemSandboxPolicy { FileSystemPath::Special { value } => match value { FileSystemSpecialPath::Root => match entry.access { FileSystemAccessMode::None => {} - FileSystemAccessMode::Read => has_full_disk_read_access = true, + FileSystemAccessMode::Read => { + if !has_full_disk_read_access + && let Some(cwd) = cwd_absolute.as_ref() + { + readable_roots.push(absolute_root_path_for_cwd(cwd)); + } + } FileSystemAccessMode::Write => { - has_full_disk_read_access = true; - has_full_disk_write_access = true; + unbridgeable_root_write = true; } }, FileSystemSpecialPath::Minimal => { @@ -878,7 +885,11 @@ impl FileSystemSandboxPolicy { exclude_tmpdir_env_var: !tmpdir_writable, exclude_slash_tmp: !slash_tmp_writable, } - } else if !writable_roots.is_empty() || tmpdir_writable || slash_tmp_writable { + } else if unbridgeable_root_write + || !writable_roots.is_empty() + || tmpdir_writable + || slash_tmp_writable + { return Err(io::Error::new( io::ErrorKind::InvalidInput, "permissions profile requests filesystem writes outside the workspace root, which is not supported until the runtime enforces FileSystemSandboxPolicy directly", @@ -2087,6 +2098,11 @@ mod tests { assert!( policy.needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),) ); + assert!( + policy + .to_legacy_sandbox_policy(NetworkSandboxPolicy::Restricted, cwd.path()) + .is_err() + ); } #[test] diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index f4e019ee28..c860947b03 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -130,10 +130,10 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path]), + Some(Vec::new()), + )), }), }, policy: &SandboxPolicy::ExternalSandbox { @@ -183,10 +183,10 @@ fn transform_additional_permissions_preserves_denied_entries() { cwd: cwd.clone(), env: HashMap::new(), additional_permissions: Some(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: None, - write: Some(vec![allowed_path.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + None, + Some(vec![allowed_path.clone()]), + )), ..Default::default() }), }, diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 73ecf81c9b..bcd02acd55 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -1,7 +1,6 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; -use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxKind; @@ -45,13 +44,29 @@ pub fn normalize_additional_permissions( let file_system = additional_permissions .file_system .map(|file_system| { - let read = file_system - .read - .map(|paths| normalize_permission_paths(paths, "file_system.read")); - let write = file_system - .write - .map(|paths| normalize_permission_paths(paths, "file_system.write")); - FileSystemPermissions { read, write } + let mut entries = Vec::with_capacity(file_system.entries.len()); + for entry in file_system.entries { + let path = match entry.path { + FileSystemPath::Path { path } => FileSystemPath::Path { + path: canonicalize_preserving_symlinks(path.as_path()) + .ok() + .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) + .unwrap_or(path), + }, + FileSystemPath::GlobPattern { pattern } => { + FileSystemPath::GlobPattern { pattern } + } + FileSystemPath::Special { value } => FileSystemPath::Special { value }, + }; + let normalized_entry = FileSystemSandboxEntry { + path, + access: entry.access, + }; + if !entries.contains(&normalized_entry) { + entries.push(normalized_entry); + } + } + FileSystemPermissions { entries } }) .filter(|file_system| !file_system.is_empty()); Ok(PermissionProfile { @@ -89,8 +104,7 @@ pub fn merge_permission_profiles( }; let file_system = match (base.file_system.as_ref(), permissions.file_system.as_ref()) { (Some(base), Some(permissions)) => Some(FileSystemPermissions { - read: merge_permission_paths(base.read.as_ref(), permissions.read.as_ref()), - write: merge_permission_paths(base.write.as_ref(), permissions.write.as_ref()), + entries: merge_permission_entries(&base.entries, &permissions.entries), }) .filter(|file_system| !file_system.is_empty()), (Some(base), None) => Some(base.clone()), @@ -116,11 +130,12 @@ pub fn intersect_permission_profiles( .file_system .map(|requested_file_system| { let granted_file_system = granted.file_system.unwrap_or_default(); - let read = - intersect_permission_paths(requested_file_system.read, granted_file_system.read); - let write = - intersect_permission_paths(requested_file_system.write, granted_file_system.write); - FileSystemPermissions { read, write } + let entries = requested_file_system + .entries + .into_iter() + .filter(|entry| granted_file_system.entries.contains(entry)) + .collect(); + FileSystemPermissions { entries } }) .filter(|file_system| !file_system.is_empty()); let network = match (requested.network, granted.network) { @@ -143,67 +158,17 @@ pub fn intersect_permission_profiles( } } -fn intersect_permission_paths( - requested: Option>, - granted: Option>, -) -> Option> { - requested.and_then(|requested_paths| { - if requested_paths.is_empty() { - return granted.map(|_| Vec::new()); - } - - let granted_paths = granted.unwrap_or_default(); - Some( - requested_paths - .into_iter() - .filter(|path| granted_paths.contains(path)) - .collect::>(), - ) - .filter(|paths| !paths.is_empty()) - }) -} - -fn normalize_permission_paths( - paths: Vec, - _permission_kind: &str, -) -> Vec { - let mut out = Vec::with_capacity(paths.len()); - let mut seen = HashSet::new(); - - for path in paths { - let canonicalized = canonicalize_preserving_symlinks(path.as_path()) - .ok() - .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) - .unwrap_or(path); - if seen.insert(canonicalized.clone()) { - out.push(canonicalized); +fn merge_permission_entries( + base: &[FileSystemSandboxEntry], + permissions: &[FileSystemSandboxEntry], +) -> Vec { + let mut merged = Vec::with_capacity(base.len() + permissions.len()); + for entry in base.iter().chain(permissions.iter()) { + if !merged.contains(entry) { + merged.push(entry.clone()); } } - - out -} - -fn merge_permission_paths( - base: Option<&Vec>, - permissions: Option<&Vec>, -) -> Option> { - match (base, permissions) { - (Some(base), Some(permissions)) => { - let mut merged = Vec::with_capacity(base.len() + permissions.len()); - let mut seen = HashSet::with_capacity(base.len() + permissions.len()); - - for path in base.iter().chain(permissions.iter()) { - if seen.insert(path.clone()) { - merged.push(path.clone()); - } - } - - Some(merged).filter(|paths| !paths.is_empty()) - } - (Some(base), None) => Some(base.clone()), - (None, Some(permissions)) => Some(permissions.clone()), - (None, None) => None, - } + merged } fn dedup_absolute_paths(paths: Vec) -> Vec { @@ -225,14 +190,24 @@ fn additional_permission_roots( additional_permissions .file_system .as_ref() - .and_then(|file_system| file_system.read.clone()) + .map(|file_system| { + file_system + .explicit_path_entries() + .filter_map(|(path, access)| access.can_read().then_some(path.clone())) + .collect() + }) .unwrap_or_default(), ), dedup_absolute_paths( additional_permissions .file_system .as_ref() - .and_then(|file_system| file_system.write.clone()) + .map(|file_system| { + file_system + .explicit_path_entries() + .filter_map(|(path, access)| access.can_write().then_some(path.clone())) + .collect() + }) .unwrap_or_default(), ), ) @@ -240,28 +215,14 @@ fn additional_permission_roots( fn merge_file_system_policy_with_additional_permissions( file_system_policy: &FileSystemSandboxPolicy, - extra_reads: Vec, - extra_writes: Vec, + additional_permissions: &FileSystemPermissions, ) -> FileSystemSandboxPolicy { match file_system_policy.kind { FileSystemSandboxKind::Restricted => { let mut merged_policy = file_system_policy.clone(); - for path in extra_reads { - let entry = FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, - access: FileSystemAccessMode::Read, - }; - if !merged_policy.entries.contains(&entry) { - merged_policy.entries.push(entry); - } - } - for path in extra_writes { - let entry = FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, - access: FileSystemAccessMode::Write, - }; - if !merged_policy.entries.contains(&entry) { - merged_policy.entries.push(entry); + for entry in &additional_permissions.entries { + if !merged_policy.entries.contains(entry) { + merged_policy.entries.push(entry.clone()); } } merged_policy @@ -280,14 +241,15 @@ pub fn effective_file_system_sandbox_policy( return file_system_policy.clone(); }; - let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions); - if extra_reads.is_empty() && extra_writes.is_empty() { + let Some(file_system_permissions) = additional_permissions.file_system.as_ref() else { + return file_system_policy.clone(); + }; + if file_system_permissions.is_empty() { file_system_policy.clone() } else { merge_file_system_policy_with_additional_permissions( file_system_policy, - extra_reads, - extra_writes, + file_system_permissions, ) } } diff --git a/codex-rs/sandboxing/src/policy_transforms_tests.rs b/codex-rs/sandboxing/src/policy_transforms_tests.rs index f33d312f15..3e16462ec8 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -106,10 +106,10 @@ fn normalize_additional_permissions_preserves_network() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(vec![path.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(vec![path.clone()]), + )), }) .expect("permissions"); @@ -121,10 +121,10 @@ fn normalize_additional_permissions_preserves_network() { ); assert_eq!( permissions.file_system, - Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(vec![path]), - }) + Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(vec![path]), + )) ); } @@ -141,23 +141,23 @@ fn normalize_additional_permissions_preserves_symlinked_write_paths() { let link_write_dir = AbsolutePathBuf::from_absolute_path(link_root.join("write")).expect("link write dir"); let permissions = normalize_additional_permissions(PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![link_write_dir]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![link_write_dir]), + )), ..Default::default() }) .expect("permissions"); assert_eq!( permissions.file_system, - Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![ + Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![ AbsolutePathBuf::from_absolute_path(link_root.join("write")) - .expect("link write dir") + .expect("link write dir"), ]), - }) + )) ); } @@ -165,10 +165,7 @@ fn normalize_additional_permissions_preserves_symlinked_write_paths() { fn normalize_additional_permissions_drops_empty_nested_profiles() { let permissions = normalize_additional_permissions(PermissionProfile { network: Some(NetworkPermissions { enabled: None }), - file_system: Some(FileSystemPermissions { - read: None, - write: None, - }), + file_system: Some(FileSystemPermissions::default()), }) .expect("permissions"); @@ -183,10 +180,10 @@ fn intersect_permission_profiles_preserves_explicit_empty_requested_reads() { ) .expect("absolute temp dir"); let requested = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![path]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![path]), + )), ..Default::default() }; let granted = requested.clone(); @@ -205,10 +202,10 @@ fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() { ) .expect("absolute temp dir"); let requested = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![path]), - write: None, - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path]), + None, + )), ..Default::default() }; @@ -226,10 +223,10 @@ fn intersect_permission_profiles_drops_explicit_empty_reads_without_grant() { ) .expect("absolute temp dir"); let requested = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![path]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![path]), + )), ..Default::default() }; @@ -258,10 +255,10 @@ fn read_only_additional_permissions_can_enable_network_without_writes() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path.clone()]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path.clone()]), + Some(Vec::new()), + )), }, ); @@ -292,10 +289,10 @@ fn external_sandbox_additional_permissions_can_enable_network() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![path]), - write: Some(Vec::new()), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![path]), + Some(Vec::new()), + )), }, ); @@ -331,8 +328,10 @@ fn merge_file_system_policy_with_additional_permissions_preserves_unreadable_roo access: FileSystemAccessMode::None, }, ]), - vec![allowed_path.clone()], - Vec::new(), + &FileSystemPermissions::from_read_write_roots( + Some(vec![allowed_path.clone()]), + Some(Vec::new()), + ), ); assert_eq!( @@ -402,10 +401,10 @@ fn effective_file_system_sandbox_policy_merges_additional_write_roots() { }, ]); let additional_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![]), - write: Some(vec![allowed_path.clone()]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![]), + Some(vec![allowed_path.clone()]), + )), ..Default::default() }; diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 0d87a34b32..c28cd899aa 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -9059,6 +9059,7 @@ guardian_approval = true file_system: Some(AdditionalFileSystemPermissions { read: Some(vec![test_absolute_path("/tmp/read-only")]), write: Some(vec![test_absolute_path("/tmp/write")]), + entries: None, }), }); params.proposed_network_policy_amendments = Some(vec![AppServerNetworkPolicyAmendment { @@ -9091,10 +9092,10 @@ guardian_approval = true network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![test_absolute_path("/tmp/read-only")]), - write: Some(vec![test_absolute_path("/tmp/write")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![test_absolute_path("/tmp/read-only")]), + Some(vec![test_absolute_path("/tmp/write")]), + )), }) ); assert_eq!( @@ -9166,6 +9167,7 @@ guardian_approval = true file_system: Some(AdditionalFileSystemPermissions { read: Some(vec![test_absolute_path("/tmp/read-only")]), write: Some(vec![test_absolute_path("/tmp/write")]), + entries: None, }), }, }, @@ -9187,10 +9189,10 @@ guardian_approval = true network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![test_absolute_path("/tmp/read-only")]), - write: Some(vec![test_absolute_path("/tmp/write")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![test_absolute_path("/tmp/read-only")]), + Some(vec![test_absolute_path("/tmp/write")]), + )), } ); } diff --git a/codex-rs/tui/src/app/app_server_requests.rs b/codex-rs/tui/src/app/app_server_requests.rs index e2483eb25d..c54aae826f 100644 --- a/codex-rs/tui/src/app/app_server_requests.rs +++ b/codex-rs/tui/src/app/app_server_requests.rs @@ -536,10 +536,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path(read_path)]), - write: Some(vec![absolute_path(write_path)]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path(read_path)]), + Some(vec![absolute_path(write_path)]), + )), }, scope: codex_protocol::request_permissions::PermissionGrantScope::Session, }, @@ -558,6 +558,7 @@ mod tests { file_system: Some(AdditionalFileSystemPermissions { read: Some(vec![absolute_path(read_path)]), write: Some(vec![absolute_path(write_path)]), + entries: None, }), }, scope: PermissionGrantScope::Session, diff --git a/codex-rs/tui/src/app_server_approval_conversions.rs b/codex-rs/tui/src/app_server_approval_conversions.rs index 8d2335eb7d..7b8e777d5b 100644 --- a/codex-rs/tui/src/app_server_approval_conversions.rs +++ b/codex-rs/tui/src/app_server_approval_conversions.rs @@ -1,4 +1,3 @@ -use codex_app_server_protocol::AdditionalFileSystemPermissions; use codex_app_server_protocol::AdditionalNetworkPermissions; use codex_app_server_protocol::GrantedPermissionProfile; use codex_app_server_protocol::NetworkApprovalContext as AppServerNetworkApprovalContext; @@ -35,12 +34,7 @@ pub(crate) fn granted_permission_profile_from_request( network: value.network.map(|network| AdditionalNetworkPermissions { enabled: network.enabled, }), - file_system: value - .file_system - .map(|file_system| AdditionalFileSystemPermissions { - read: file_system.read, - write: file_system.write, - }), + file_system: value.file_system.map(Into::into), } } @@ -50,6 +44,10 @@ mod tests { use super::network_approval_context_to_core; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::NetworkApprovalContext; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile; @@ -82,10 +80,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/read-only")]), - write: Some(vec![absolute_path("/tmp/write")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/read-only")]), + Some(vec![absolute_path("/tmp/write")]), + )), }), codex_app_server_protocol::GrantedPermissionProfile { network: Some(codex_app_server_protocol::AdditionalNetworkPermissions { @@ -94,6 +92,37 @@ mod tests { file_system: Some(codex_app_server_protocol::AdditionalFileSystemPermissions { read: Some(vec![absolute_path("/tmp/read-only")]), write: Some(vec![absolute_path("/tmp/write")]), + entries: None, + }), + } + ); + } + + #[test] + fn converts_request_permissions_into_canonical_granted_permissions() { + assert_eq!( + granted_permission_profile_from_request(CoreRequestPermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }], + }), + ..Default::default() + }), + codex_app_server_protocol::GrantedPermissionProfile { + network: None, + file_system: Some(codex_app_server_protocol::AdditionalFileSystemPermissions { + read: None, + write: None, + entries: Some(vec![codex_app_server_protocol::FileSystemSandboxEntry { + path: codex_app_server_protocol::FileSystemPath::Special { + value: codex_app_server_protocol::FileSystemSpecialPath::Root, + }, + access: codex_app_server_protocol::FileSystemAccessMode::Write, + },]), }), } ); diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 0e2bb2b2e3..1c148cd02a 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -21,6 +21,10 @@ use codex_features::Features; use codex_protocol::ThreadId; use codex_protocol::mcp::RequestId; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::ElicitationAction; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; @@ -798,22 +802,33 @@ pub(crate) fn format_additional_permissions_rule( parts.push("network".to_string()); } if let Some(file_system) = additional_permissions.file_system.as_ref() { - if let Some(read) = file_system.read.as_ref() { - let reads = read + let reads = format_file_system_entry_paths( + file_system + .entries .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); + .filter(|entry| entry.access == FileSystemAccessMode::Read), + ); + if !reads.is_empty() { parts.push(format!("read {reads}")); } - if let Some(write) = file_system.write.as_ref() { - let writes = write + let writes = format_file_system_entry_paths( + file_system + .entries .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); + .filter(|entry| entry.access == FileSystemAccessMode::Write), + ); + if !writes.is_empty() { parts.push(format!("write {writes}")); } + let denied_reads = format_file_system_entry_paths( + file_system + .entries + .iter() + .filter(|entry| entry.access == FileSystemAccessMode::None), + ); + if !denied_reads.is_empty() { + parts.push(format!("deny read {denied_reads}")); + } } if parts.is_empty() { None @@ -828,6 +843,38 @@ pub(crate) fn format_requested_permissions_rule( format_additional_permissions_rule(&permissions.clone().into()) } +fn format_file_system_entry_paths<'a>( + entries: impl Iterator, +) -> String { + entries + .map(|entry| match &entry.path { + FileSystemPath::Path { path } => format!("`{}`", path.display()), + FileSystemPath::GlobPattern { pattern } => format!("glob `{pattern}`"), + FileSystemPath::Special { value } => format!("`{}`", special_path_label(value)), + }) + .collect::>() + .join(", ") +} + +fn special_path_label(value: &FileSystemSpecialPath) -> String { + match value { + FileSystemSpecialPath::Root => ":root".to_string(), + FileSystemSpecialPath::Minimal => ":minimal".to_string(), + FileSystemSpecialPath::CurrentWorkingDirectory => ":cwd".to_string(), + FileSystemSpecialPath::ProjectRoots { subpath } => path_label(":project_roots", subpath), + FileSystemSpecialPath::Tmpdir => ":tmpdir".to_string(), + FileSystemSpecialPath::SlashTmp => "/tmp".to_string(), + FileSystemSpecialPath::Unknown { path, subpath } => path_label(path, subpath), + } +} + +fn path_label(base: &str, subpath: &Option) -> String { + match subpath { + Some(subpath) => format!("{base}/{}", subpath.display()), + None => base.to_string(), + } +} + fn patch_options() -> Vec { vec![ ApprovalOption { @@ -903,6 +950,9 @@ mod tests { use crate::app_event::AppEvent; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::ExecPolicyAmendment; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::protocol::NetworkPolicyAmendment; @@ -965,10 +1015,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), }, } } @@ -1266,10 +1316,10 @@ mod tests { #[test] fn additional_permissions_exec_options_hide_execpolicy_amendment() { let additional_permissions = PermissionProfile { - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), ..Default::default() }; let options = exec_options( @@ -1304,6 +1354,34 @@ mod tests { ); } + #[test] + fn additional_permissions_rule_shows_non_path_file_system_entries() { + let additional_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + entries: vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: "**/*.env".to_string(), + }, + access: FileSystemAccessMode::None, + }, + ], + }), + ..Default::default() + }; + + assert_eq!( + format_additional_permissions_rule(&additional_permissions), + Some("write `:root`; deny read glob `**/*.env`".to_string()) + ); + } + #[test] fn permissions_session_shortcut_submits_session_scope() { let (tx, mut rx) = unbounded_channel::(); @@ -1347,10 +1425,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), }), }; @@ -1397,10 +1475,10 @@ mod tests { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![absolute_path("/tmp/readme.txt")]), - write: Some(vec![absolute_path("/tmp/out.txt")]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![absolute_path("/tmp/readme.txt")]), + Some(vec![absolute_path("/tmp/out.txt")]), + )), }), }; diff --git a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs index 512b415926..472a13c118 100644 --- a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs +++ b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs @@ -113,6 +113,7 @@ fn app_server_exec_approval_request_preserves_permissions_context() { file_system: Some(AppServerAdditionalFileSystemPermissions { read: Some(vec![read_path.clone()]), write: Some(vec![write_path.clone()]), + entries: None, }), }), proposed_execpolicy_amendment: None, @@ -135,10 +136,10 @@ fn app_server_exec_approval_request_preserves_permissions_context() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![read_path]), - write: Some(vec![write_path]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![read_path]), + Some(vec![write_path]), + )), }) ); } @@ -162,6 +163,7 @@ fn app_server_request_permissions_preserves_file_system_permissions() { file_system: Some(AppServerAdditionalFileSystemPermissions { read: Some(vec![read_path.clone()]), write: Some(vec![write_path.clone()]), + entries: None, }), }, }); @@ -172,10 +174,10 @@ fn app_server_request_permissions_preserves_file_system_permissions() { network: Some(NetworkPermissions { enabled: Some(true), }), - file_system: Some(FileSystemPermissions { - read: Some(vec![read_path]), - write: Some(vec![write_path]), - }), + file_system: Some(FileSystemPermissions::from_read_write_roots( + Some(vec![read_path]), + Some(vec![write_path]), + )), } ); }