diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 5070d8da3a..822cfaa46d 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -122,6 +122,7 @@ impl ExecRequest { windows_sandbox_level, windows_sandbox_private_desktop, permission_profile, + effective_filesystem_permissions: _, file_system_sandbox_policy, network_sandbox_policy, arg0, diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index bfc3ab94b3..94b157dcfc 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -46,7 +46,9 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::PatchApplyUpdatedEvent; -use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; +use codex_sandboxing::EffectiveFilesystemPermissions; +use codex_sandboxing::FilesystemPermissionsContext; +use codex_sandboxing::policy_transforms::effective_permission_profile; use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_sandboxing::policy_transforms::normalize_additional_permissions; use codex_tools::ToolName; @@ -224,8 +226,7 @@ fn to_abs_path(cwd: &AbsolutePathBuf, path: &Path) -> Option { fn write_permissions_for_paths( file_paths: &[AbsolutePathBuf], - file_system_sandbox_policy: &codex_protocol::permissions::FileSystemSandboxPolicy, - cwd: &AbsolutePathBuf, + effective_filesystem_permissions: &EffectiveFilesystemPermissions, ) -> Option { let write_paths = file_paths .iter() @@ -234,9 +235,7 @@ fn write_permissions_for_paths( .unwrap_or_else(|| path.clone()) .into_path_buf() }) - .filter(|path| { - !file_system_sandbox_policy.can_write_path_with_cwd(path.as_path(), cwd.as_path()) - }) + .filter(|path| !effective_filesystem_permissions.can_write(path.as_path())) .collect::>() .into_iter() .map(AbsolutePathBuf::from_absolute_path) @@ -267,34 +266,46 @@ async fn effective_patch_permissions( turn: &TurnContext, action: &ApplyPatchAction, cwd: &AbsolutePathBuf, -) -> ( - Vec, - crate::tools::handlers::EffectiveAdditionalPermissions, - codex_protocol::permissions::FileSystemSandboxPolicy, -) { +) -> Result< + ( + Vec, + crate::tools::handlers::EffectiveAdditionalPermissions, + codex_protocol::permissions::FileSystemSandboxPolicy, + ), + FunctionCallError, +> { let file_paths = file_paths_for_action(action); let granted_permissions = merge_permission_profiles( session.granted_session_permissions().await.as_ref(), session.granted_turn_permissions().await.as_ref(), ); - let base_file_system_sandbox_policy = turn.file_system_sandbox_policy(); - let file_system_sandbox_policy = effective_file_system_sandbox_policy( - &base_file_system_sandbox_policy, - granted_permissions.as_ref(), - ); + let effective_permission_profile = + effective_permission_profile(&turn.permission_profile(), granted_permissions.as_ref()); + let file_system_sandbox_policy = effective_permission_profile.file_system_sandbox_policy(); + let effective_filesystem_permissions = EffectiveFilesystemPermissions::from_profile( + &effective_permission_profile, + FilesystemPermissionsContext { + policy_evaluation_cwd: cwd, + }, + ) + .map_err(|err| { + FunctionCallError::RespondToModel(format!( + "failed to derive effective filesystem permissions for apply_patch: {err}" + )) + })?; let effective_additional_permissions = apply_granted_turn_permissions( session, cwd.as_path(), crate::sandboxing::SandboxPermissions::UseDefault, - write_permissions_for_paths(&file_paths, &file_system_sandbox_policy, cwd), + write_permissions_for_paths(&file_paths, &effective_filesystem_permissions), ) .await; - ( + Ok(( file_paths, effective_additional_permissions, file_system_sandbox_policy, - ) + )) } #[async_trait::async_trait] @@ -354,7 +365,7 @@ impl ToolExecutor for ApplyPatchHandler { codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { let (file_paths, effective_additional_permissions, file_system_sandbox_policy) = effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes, &cwd) - .await; + .await?; match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes) .await { @@ -506,7 +517,7 @@ pub(crate) async fn intercept_apply_patch( { codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { let (approval_keys, effective_additional_permissions, file_system_sandbox_policy) = - effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes, cwd).await; + effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes, cwd).await?; match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes) .await { 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 e9a118641a..92ba316057 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -1,7 +1,9 @@ use super::*; use codex_apply_patch::MaybeApplyPatchVerified; use codex_exec_server::LOCAL_FS; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::FileChange; use core_test_support::PathBufExt; use core_test_support::PathExt; @@ -27,6 +29,24 @@ fn sample_patch() -> &'static str { *** End Patch"# } +fn effective_permissions( + sandbox_policy: &FileSystemSandboxPolicy, + cwd: &AbsolutePathBuf, +) -> codex_sandboxing::EffectiveFilesystemPermissions { + let permission_profile = PermissionProfile::from_runtime_permissions( + sandbox_policy, + NetworkSandboxPolicy::Restricted, + ) + .materialize_project_roots_with_workspace_roots(std::slice::from_ref(cwd)); + codex_sandboxing::EffectiveFilesystemPermissions::from_profile( + &permission_profile, + codex_sandboxing::FilesystemPermissionsContext { + policy_evaluation_cwd: cwd, + }, + ) + .expect("derive effective filesystem permissions") +} + async fn invocation_for_payload(payload: ToolPayload) -> ToolInvocation { let (session, turn) = make_session_and_context().await; ToolInvocation { @@ -255,8 +275,9 @@ fn write_permissions_for_paths_skip_dirs_already_writable_under_workspace_root() /*exclude_tmpdir_env_var*/ true, /*exclude_slash_tmp*/ false, ); + let effective_permissions = effective_permissions(&sandbox_policy, &cwd); - let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd); + let permissions = write_permissions_for_paths(&[file_path], &effective_permissions); assert_eq!(permissions, None); } @@ -276,8 +297,9 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() { /*exclude_tmpdir_env_var*/ true, /*exclude_slash_tmp*/ true, ); + let effective_permissions = effective_permissions(&sandbox_policy, &cwd_abs); - let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd_abs); + let permissions = write_permissions_for_paths(&[file_path], &effective_permissions); let expected_outside = dunce::simplified(&outside.canonicalize().expect("canonicalize outside dir")).abs(); diff --git a/codex-rs/sandboxing/src/effective_filesystem_permissions.rs b/codex-rs/sandboxing/src/effective_filesystem_permissions.rs index d0f5f5c7ed..0c1de7d16f 100644 --- a/codex-rs/sandboxing/src/effective_filesystem_permissions.rs +++ b/codex-rs/sandboxing/src/effective_filesystem_permissions.rs @@ -25,7 +25,7 @@ pub enum FilesystemPermissionsMode { External, } -/// A deny-read glob accepted by the existing runtime matcher. +/// A deny-read glob retained in effective filesystem enforcement inputs. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ValidatedDenyGlob { pattern: String, @@ -83,9 +83,11 @@ impl EffectiveFilesystemPermissions { if contains_unmaterialized_workspace_roots(&file_system_policy) { return Err(FilesystemPermissionsError::UnmaterializedWorkspaceRoots); } + // Direct enforcement queries have historically failed closed for malformed + // deny patterns. Platform lowering that expands concrete targets can still + // validate the patterns before acting on the filesystem. let read_deny_matcher = - ReadDenyMatcher::try_new(&file_system_policy, context.policy_evaluation_cwd.as_path()) - .map_err(FilesystemPermissionsError::InvalidDenyGlob)?; + ReadDenyMatcher::new(&file_system_policy, context.policy_evaluation_cwd.as_path()); let mode = match file_system_policy.kind { FileSystemSandboxKind::Restricted => FilesystemPermissionsMode::Restricted, FileSystemSandboxKind::Unrestricted => FilesystemPermissionsMode::Unrestricted, diff --git a/codex-rs/sandboxing/src/effective_filesystem_permissions_tests.rs b/codex-rs/sandboxing/src/effective_filesystem_permissions_tests.rs index fb061f46bf..24df41c7b6 100644 --- a/codex-rs/sandboxing/src/effective_filesystem_permissions_tests.rs +++ b/codex-rs/sandboxing/src/effective_filesystem_permissions_tests.rs @@ -252,3 +252,28 @@ fn effective_permissions_preserve_accepted_deny_glob_matching() { ); assert_eq!(effective.can_read(denied_path.as_path()), false); } + +#[test] +fn effective_permissions_fail_closed_for_malformed_deny_globs() { + let temp_dir = TempDir::new().expect("temp dir"); + let cwd = absolute_path(temp_dir.path()); + let readable_path = cwd.join("readable.txt"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: cwd.clone() }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::GlobPattern { + pattern: format!("{}/**/[z-a]", cwd.as_path().display()), + }, + access: FileSystemAccessMode::Deny, + }, + ]); + let permission_profile = + PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted); + let effective = derive_effective(&permission_profile, &cwd); + + assert_eq!(effective.is_read_denied(readable_path.as_path()), true); + assert_eq!(effective.can_read(readable_path.as_path()), false); +} diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index e04f4d5168..ded09922c7 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -41,6 +41,12 @@ impl From for CodexErr { SandboxTransformError::MissingLinuxSandboxExecutable => { CodexErr::LandlockSandboxExecutableNotProvided } + SandboxTransformError::InvalidPermissionProfileCwd(message) => { + CodexErr::UnsupportedOperation(message) + } + SandboxTransformError::EffectiveFilesystemPermissions(err) => { + CodexErr::UnsupportedOperation(err.to_string()) + } #[cfg(target_os = "linux")] SandboxTransformError::Wsl1UnsupportedForBubblewrap => { CodexErr::UnsupportedOperation(crate::bwrap::WSL1_BWRAP_WARNING.to_string()) diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 82c49f7908..8dc909f751 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -1,3 +1,6 @@ +use crate::EffectiveFilesystemPermissions; +use crate::FilesystemPermissionsContext; +use crate::FilesystemPermissionsError; #[cfg(target_os = "linux")] use crate::bwrap::WSL1_BWRAP_WARNING; #[cfg(target_os = "linux")] @@ -80,6 +83,7 @@ pub struct SandboxExecRequest { pub windows_sandbox_level: WindowsSandboxLevel, pub windows_sandbox_private_desktop: bool, pub permission_profile: PermissionProfile, + pub effective_filesystem_permissions: EffectiveFilesystemPermissions, pub file_system_sandbox_policy: FileSystemSandboxPolicy, pub network_sandbox_policy: NetworkSandboxPolicy, pub arg0: Option, @@ -106,6 +110,8 @@ pub struct SandboxTransformRequest<'a> { #[derive(Debug)] pub enum SandboxTransformError { MissingLinuxSandboxExecutable, + InvalidPermissionProfileCwd(String), + EffectiveFilesystemPermissions(FilesystemPermissionsError), #[cfg(target_os = "linux")] Wsl1UnsupportedForBubblewrap, #[cfg(not(target_os = "macos"))] @@ -118,6 +124,8 @@ impl std::fmt::Display for SandboxTransformError { Self::MissingLinuxSandboxExecutable => { write!(f, "missing codex-linux-sandbox executable path") } + Self::InvalidPermissionProfileCwd(message) => f.write_str(message), + Self::EffectiveFilesystemPermissions(err) => write!(f, "{err}"), #[cfg(target_os = "linux")] Self::Wsl1UnsupportedForBubblewrap => write!(f, "{WSL1_BWRAP_WARNING}"), #[cfg(not(target_os = "macos"))] @@ -186,6 +194,15 @@ impl SandboxManager { effective_permission_profile(permissions, additional_permissions.as_ref()); let (effective_file_system_policy, effective_network_policy) = effective_permission_profile.to_runtime_permissions(); + let policy_evaluation_cwd = AbsolutePathBuf::from_absolute_path(sandbox_policy_cwd) + .map_err(|err| SandboxTransformError::InvalidPermissionProfileCwd(err.to_string()))?; + let effective_filesystem_permissions = EffectiveFilesystemPermissions::from_profile( + &effective_permission_profile, + FilesystemPermissionsContext { + policy_evaluation_cwd: &policy_evaluation_cwd, + }, + ) + .map_err(SandboxTransformError::EffectiveFilesystemPermissions)?; let mut argv = Vec::with_capacity(1 + command.args.len()); argv.push(command.program); argv.extend(command.args.into_iter().map(OsString::from)); @@ -253,6 +270,7 @@ impl SandboxManager { windows_sandbox_level, windows_sandbox_private_desktop, permission_profile: effective_permission_profile, + effective_filesystem_permissions, file_system_sandbox_policy: effective_file_system_policy, network_sandbox_policy: effective_network_policy, arg0: arg0_override, diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index b00e9779be..d008fcdd02 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -217,6 +217,18 @@ fn transform_additional_permissions_preserves_denied_entries() { }) .expect("transform"); + assert_eq!( + exec_request + .effective_filesystem_permissions + .can_write(allowed_path.as_path()), + true + ); + assert_eq!( + exec_request + .effective_filesystem_permissions + .can_read(denied_path.as_path()), + false + ); assert_eq!( exec_request.file_system_sandbox_policy, FileSystemSandboxPolicy::restricted(vec![