From 5a85351dfe04ae2930858e54ecccba9e239b7ccd Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 16 Jul 2026 16:03:48 +0000 Subject: [PATCH] Remove generated-default filesystem path variants (#33632) ## What changed - Represent built-in filesystem policy entries with the same `Path` and `Special` variants used by other entries. - Remove generated-default filtering from Windows sandbox policy resolution and simplify elevated deny-write handling to rely on direct-runtime enforcement and the legacy policy projection. - Update protocol conversions, policy transforms, and profile expectations for the reduced filesystem path model. GitOrigin-RevId: a65793e6855a03d52a575e82cb3d4b9b2e32db5c --- .../src/protocol/v2/permissions.rs | 6 +- codex-rs/core/src/config/config_tests.rs | 2 +- codex-rs/core/src/config/permissions.rs | 2 - .../core/src/context/environment_context.rs | 4 +- codex-rs/core/src/exec_tests.rs | 45 ------ codex-rs/core/tests/suite/windows_sandbox.rs | 53 ------- codex-rs/file-system/src/lib.rs | 22 +-- codex-rs/protocol/src/models.rs | 5 +- codex-rs/protocol/src/permissions.rs | 145 +++--------------- codex-rs/sandboxing/src/policy_transforms.rs | 83 ++++------ codex-rs/sandboxing/src/windows.rs | 31 +--- 11 files changed, 59 insertions(+), 339 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 b0656f5f9b35..a6976c9c07a9 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs @@ -312,13 +312,11 @@ pub enum FileSystemPath { impl From for FileSystemPath { fn from(value: CoreFileSystemPath) -> Self { match value { - CoreFileSystemPath::Path { path } - | CoreFileSystemPath::GeneratedDefaultPath { path } => Self::Path { + CoreFileSystemPath::Path { path } => Self::Path { path: LegacyAppPathString::from_abs_path(&path), }, CoreFileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, - CoreFileSystemPath::Special { value } - | CoreFileSystemPath::GeneratedDefaultSpecial { value } => Self::Special { + CoreFileSystemPath::Special { value } => Self::Special { value: value.into(), }, } diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 739cc2ad493f..7d878e84aa3c 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -4167,7 +4167,7 @@ exclude_slash_tmp = true file_system_policy .entries .contains(&FileSystemSandboxEntry { - path: FileSystemPath::GeneratedDefaultPath { + path: FileSystemPath::Path { path: AbsolutePathBuf::resolve_path_against_base( subpath, cwd.path() diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index 1e26725a7902..47fa8636196f 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -249,8 +249,6 @@ fn insert_filesystem_permission_toml( FileSystemPath::Special { value } => { insert_special_filesystem_permission_toml(entries, value, entry.access); } - FileSystemPath::GeneratedDefaultPath { .. } - | FileSystemPath::GeneratedDefaultSpecial { .. } => {} } } diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index 64647b3c53e1..eec5fa1b0afa 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -153,13 +153,13 @@ fn render_file_system_entry(rendered: &mut String, entry: &FileSystemSandboxEntr } rendered.push_str("\">"); match &entry.path { - FileSystemPath::Path { path } | FileSystemPath::GeneratedDefaultPath { path } => { + FileSystemPath::Path { path } => { push_text_element(rendered, "path", path.to_string_lossy().as_ref()); } FileSystemPath::GlobPattern { pattern } => { push_text_element(rendered, "glob", pattern); } - FileSystemPath::Special { value } | FileSystemPath::GeneratedDefaultSpecial { value } => { + FileSystemPath::Special { value } => { let value = render_special_path(value); push_text_element(rendered, "special", &value); } diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 21f0733537d7..1a4477599de3 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -803,51 +803,6 @@ fn windows_elevated_supports_split_write_read_carveouts() { ); } -#[cfg(target_os = "windows")] -#[test] -fn windows_workspace_defaults_do_not_hide_explicit_metadata_carveouts() { - let temp_dir = tempfile::TempDir::new().expect("tempdir"); - let cwd = temp_dir.path().canonicalize().expect("canonical cwd").abs(); - - let default_profile = PermissionProfile::workspace_write(); - let default_overrides = resolve_windows_elevated_filesystem_overrides( - SandboxType::WindowsRestrictedToken, - &default_profile, - &cwd, - /*use_windows_elevated_backend*/ true, - ) - .expect("resolve workspace defaults"); - assert!( - default_overrides.is_none_or(|overrides| overrides.additional_deny_write_paths.is_empty()) - ); - - for name in codex_protocol::permissions::PROTECTED_METADATA_PATH_NAMES { - let (mut explicit_policy, network_policy) = default_profile.to_runtime_permissions(); - explicit_policy - .entries - .push(codex_protocol::permissions::FileSystemSandboxEntry { - path: codex_protocol::permissions::FileSystemPath::Special { - value: codex_protocol::permissions::FileSystemSpecialPath::project_roots(Some( - (*name).into(), - )), - }, - access: codex_protocol::permissions::FileSystemAccessMode::Read, - }); - let explicit_profile = - PermissionProfile::from_runtime_permissions(&explicit_policy, network_policy); - - let overrides = resolve_windows_elevated_filesystem_overrides( - SandboxType::WindowsRestrictedToken, - &explicit_profile, - &cwd, - /*use_windows_elevated_backend*/ true, - ) - .expect("resolve explicit metadata carveout") - .expect("explicit metadata carveout needs an override"); - assert_eq!(overrides.additional_deny_write_paths, vec![cwd.join(name)]); - } -} - #[test] fn windows_elevated_supports_unreadable_split_carveouts() { let temp_dir = tempfile::TempDir::new().expect("tempdir"); diff --git a/codex-rs/core/tests/suite/windows_sandbox.rs b/codex-rs/core/tests/suite/windows_sandbox.rs index 4392250e56d0..b8895f49e4e1 100644 --- a/codex-rs/core/tests/suite/windows_sandbox.rs +++ b/codex-rs/core/tests/suite/windows_sandbox.rs @@ -197,59 +197,6 @@ async fn windows_restricted_token_rejects_exact_and_glob_deny_read_policy() -> a Ok(()) } -#[tokio::test] -#[serial(codex_home)] -async fn windows_elevated_does_not_create_missing_workspace_metadata() -> anyhow::Result<()> { - let codex_home = - codex_home_for_windows_sandbox_test("windows-elevated-missing-metadata-codex-home")?; - let _codex_home_guard = EnvVarGuard::set("CODEX_HOME", codex_home.path().as_os_str()); - stage_windows_sandbox_helpers()?; - let workspace = TempDir::new()?; - let cwd = dunce::canonicalize(workspace.path())?.abs(); - let permission_profile = PermissionProfile::workspace_write() - .materialize_project_roots_with_workspace_roots(std::slice::from_ref(&cwd)); - - let output = process_exec_tool_call( - ExecParams { - command: vec![ - "cmd.exe".to_string(), - "/D".to_string(), - "/C".to_string(), - "echo sandbox-ok".to_string(), - ], - cwd: cwd.clone(), - expiration: 10_000.into(), - capture_policy: ExecCapturePolicy::ShellTool, - env: HashMap::new(), - network: None, - network_environment_id: None, - sandbox_permissions: SandboxPermissions::UseDefault, - windows_sandbox_level: WindowsSandboxLevel::Elevated, - windows_sandbox_private_desktop: false, - justification: None, - arg0: None, - }, - &permission_profile, - &cwd, - std::slice::from_ref(&cwd), - &None, - /*use_legacy_landlock*/ false, - /*stdout_stream*/ None, - ) - .await?; - - assert_eq!(output.exit_code, 0, "sandboxed command should complete"); - for name in codex_protocol::permissions::PROTECTED_METADATA_PATH_NAMES { - let path = cwd.join(name); - assert!( - !path.exists(), - "elevated setup should not create missing workspace metadata: {}", - path.display() - ); - } - Ok(()) -} - #[tokio::test] #[serial(codex_home)] async fn windows_elevated_enforces_deny_read_and_protects_setup_marker() -> anyhow::Result<()> { diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 5224b6f1016f..c9effdc20caf 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -126,10 +126,8 @@ pub struct WalkOutcome { #[serde(tag = "type", rename_all = "snake_case")] pub enum ExecFileSystemPath { Path { path: PathUri }, - GeneratedDefaultPath { path: PathUri }, GlobPattern { pattern: String }, Special { value: FileSystemSpecialPath }, - GeneratedDefaultSpecial { value: FileSystemSpecialPath }, } impl From for ExecFileSystemPath { @@ -138,14 +136,8 @@ impl From for ExecFileSystemPath { FileSystemPath::Path { path } => Self::Path { path: PathUri::from_abs_path(&path), }, - FileSystemPath::GeneratedDefaultPath { path } => Self::GeneratedDefaultPath { - path: PathUri::from_abs_path(&path), - }, FileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, FileSystemPath::Special { value } => Self::Special { value }, - FileSystemPath::GeneratedDefaultSpecial { value } => { - Self::GeneratedDefaultSpecial { value } - } } } } @@ -158,14 +150,8 @@ impl TryFrom for FileSystemPath { ExecFileSystemPath::Path { path } => Self::Path { path: path.to_abs_path()?, }, - ExecFileSystemPath::GeneratedDefaultPath { path } => Self::GeneratedDefaultPath { - path: path.to_abs_path()?, - }, ExecFileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, ExecFileSystemPath::Special { value } => Self::Special { value }, - ExecFileSystemPath::GeneratedDefaultSpecial { value } => { - Self::GeneratedDefaultSpecial { value } - } }) } } @@ -364,14 +350,8 @@ impl FileSystemSandboxContext { ExecFileSystemPath::GlobPattern { pattern } => !Path::new(pattern).is_absolute(), ExecFileSystemPath::Special { value: FileSystemSpecialPath::ProjectRoots { .. }, - } - | ExecFileSystemPath::GeneratedDefaultSpecial { - value: FileSystemSpecialPath::ProjectRoots { .. }, } => true, - ExecFileSystemPath::Path { .. } - | ExecFileSystemPath::GeneratedDefaultPath { .. } - | ExecFileSystemPath::Special { .. } - | ExecFileSystemPath::GeneratedDefaultSpecial { .. } => false, + ExecFileSystemPath::Path { .. } | ExecFileSystemPath::Special { .. } => false, }), ExecPermissionProfile::Managed { file_system: ExecManagedFileSystemPermissions::Unrestricted, diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 882f5e5d4d8e..56d0bce102dc 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -111,10 +111,7 @@ impl FileSystemPermissions { ) -> impl Iterator { self.entries.iter().filter_map(|entry| match &entry.path { FileSystemPath::Path { path } => Some((path, entry.access)), - FileSystemPath::GeneratedDefaultPath { .. } - | FileSystemPath::GlobPattern { .. } - | FileSystemPath::Special { .. } - | FileSystemPath::GeneratedDefaultSpecial { .. } => None, + FileSystemPath::GlobPattern { .. } | FileSystemPath::Special { .. } => None, }) } diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index bbd8b8290ccf..ec37292e18a3 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -343,11 +343,6 @@ pub enum FileSystemPath { // TODO(anp): Use PathUri once permission paths no longer require native-path rollout serialization. path: AbsolutePathBuf, }, - /// A concrete path synthesized by a built-in permissions profile rather - /// than explicitly configured by the user. - GeneratedDefaultPath { - path: AbsolutePathBuf, - }, /// A git-style glob pattern. Pattern entries currently support /// FileSystemAccessMode::Deny only. GlobPattern { @@ -356,20 +351,6 @@ pub enum FileSystemPath { Special { value: FileSystemSpecialPath, }, - /// A symbolic path synthesized by a built-in permissions profile rather - /// than explicitly configured by the user. - GeneratedDefaultSpecial { - value: FileSystemSpecialPath, - }, -} - -impl FileSystemPath { - pub fn is_generated_default(&self) -> bool { - matches!( - self, - Self::GeneratedDefaultPath { .. } | Self::GeneratedDefaultSpecial { .. } - ) - } } const PROJECT_ROOTS_GLOB_PATTERN_PREFIX: &str = "codex-project-roots://"; @@ -422,25 +403,6 @@ impl FileSystemSandboxPolicy { } } - /// Removes entries synthesized by built-in profiles while preserving - /// explicitly configured entries targeting the same paths. - pub fn remove_generated_defaults(&mut self) { - self.entries - .retain(|entry| !entry.path.is_generated_default()); - } - - pub fn has_explicit_non_write_entry_for_path_with_cwd(&self, path: &Path, cwd: &Path) -> bool { - let Some(path) = resolve_candidate_path(path, cwd) else { - return false; - }; - let cwd = AbsolutePathBuf::from_absolute_path(cwd).ok(); - self.entries.iter().any(|entry| { - !entry.path.is_generated_default() - && !entry.access.can_write() - && resolve_entry_path(&entry.path, cwd.as_ref()).as_ref() == Some(&path) - }) - } - fn has_root_access(&self, predicate: impl Fn(FileSystemAccessMode) -> bool) -> bool { matches!(self.kind, FileSystemSandboxKind::Restricted) && self.entries.iter().any(|entry| { @@ -534,12 +496,9 @@ impl FileSystemSandboxPolicy { } match &entry.path { - FileSystemPath::Path { .. } | FileSystemPath::GeneratedDefaultPath { .. } => { - !self.has_same_target_write_override(entry) - } + FileSystemPath::Path { .. } => !self.has_same_target_write_override(entry), FileSystemPath::GlobPattern { .. } => true, - FileSystemPath::Special { value } - | FileSystemPath::GeneratedDefaultSpecial { value } => match value { + FileSystemPath::Special { value } => match value { FileSystemSpecialPath::Root => entry.access == FileSystemAccessMode::Deny, FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => { false @@ -758,13 +717,6 @@ impl FileSystemSandboxPolicy { let cwd = AbsolutePathBuf::from_absolute_path(cwd).ok(); for entry in &mut self.entries { match &entry.path { - FileSystemPath::GeneratedDefaultSpecial { - value: FileSystemSpecialPath::ProjectRoots { .. }, - } => { - if let Some(path) = resolve_file_system_path(&entry.path, cwd.as_ref()) { - entry.path = FileSystemPath::GeneratedDefaultPath { path }; - } - } FileSystemPath::Special { value: FileSystemSpecialPath::ProjectRoots { .. }, } => { @@ -782,9 +734,7 @@ impl FileSystemSandboxPolicy { } } FileSystemPath::Special { value: _ } => {} - FileSystemPath::GeneratedDefaultSpecial { value: _ } => {} FileSystemPath::Path { .. } => {} - FileSystemPath::GeneratedDefaultPath { .. } => {} } } self @@ -799,22 +749,6 @@ impl FileSystemSandboxPolicy { let mut entries = Vec::with_capacity(self.entries.len()); for entry in self.entries { match entry.path { - FileSystemPath::GeneratedDefaultSpecial { - value: FileSystemSpecialPath::ProjectRoots { subpath }, - } => { - entries.extend(workspace_roots.iter().map(|root| FileSystemSandboxEntry { - path: FileSystemPath::GeneratedDefaultPath { - path: match subpath.as_ref() { - Some(subpath) => AbsolutePathBuf::resolve_path_against_base( - subpath, - root.as_path(), - ), - None => root.clone(), - }, - }, - access: entry.access, - })); - } FileSystemPath::Special { value: FileSystemSpecialPath::ProjectRoots { subpath }, } => { @@ -852,24 +786,12 @@ impl FileSystemSandboxPolicy { access: entry.access, }); } - FileSystemPath::GeneratedDefaultPath { path } => { - entries.push(FileSystemSandboxEntry { - path: FileSystemPath::GeneratedDefaultPath { path }, - access: entry.access, - }); - } FileSystemPath::Special { value } => { entries.push(FileSystemSandboxEntry { path: FileSystemPath::Special { value }, access: entry.access, }); } - FileSystemPath::GeneratedDefaultSpecial { value } => { - entries.push(FileSystemSandboxEntry { - path: FileSystemPath::GeneratedDefaultSpecial { value }, - access: entry.access, - }); - } } } self.entries = entries; @@ -1173,10 +1095,7 @@ impl FileSystemSandboxPolicy { FileSystemPath::GlobPattern { pattern } => { Some(AbsolutePathBuf::resolve_path_against_base(pattern, cwd)) } - FileSystemPath::Path { .. } - | FileSystemPath::GeneratedDefaultPath { .. } - | FileSystemPath::Special { .. } - | FileSystemPath::GeneratedDefaultSpecial { .. } => None, + FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => None, }) .map(|pattern| pattern.to_string_lossy().into_owned()) .collect::>(); @@ -1219,8 +1138,7 @@ impl FileSystemSandboxPolicy { for entry in &self.entries { match &entry.path { FileSystemPath::GlobPattern { .. } => {} - FileSystemPath::Path { path } - | FileSystemPath::GeneratedDefaultPath { path } => { + FileSystemPath::Path { path } => { if entry.access.can_write() { if cwd_absolute.as_ref().is_some_and(|cwd| cwd == path) { workspace_root_writable = true; @@ -1229,8 +1147,7 @@ impl FileSystemSandboxPolicy { } } } - FileSystemPath::Special { value } - | FileSystemPath::GeneratedDefaultSpecial { value } => match value { + FileSystemPath::Special { value } => match value { FileSystemSpecialPath::Root => match entry.access { FileSystemAccessMode::Deny => {} FileSystemAccessMode::Read => {} @@ -1372,13 +1289,9 @@ fn resolve_file_system_path( cwd: Option<&AbsolutePathBuf>, ) -> Option { match path { - FileSystemPath::Path { path } | FileSystemPath::GeneratedDefaultPath { path } => { - Some(path.clone()) - } + FileSystemPath::Path { path } => Some(path.clone()), FileSystemPath::GlobPattern { .. } => None, - FileSystemPath::Special { value } | FileSystemPath::GeneratedDefaultSpecial { value } => { - resolve_file_system_special_path(value, cwd) - } + FileSystemPath::Special { value } => resolve_file_system_special_path(value, cwd), } } @@ -1422,32 +1335,16 @@ fn resolve_candidate_path(path: &Path, cwd: &Path) -> Option { /// by `has_write_narrowing_entries`. fn file_system_paths_share_target(left: &FileSystemPath, right: &FileSystemPath) -> bool { match (left, right) { - ( - FileSystemPath::Path { path: left } - | FileSystemPath::GeneratedDefaultPath { path: left }, - FileSystemPath::Path { path: right } - | FileSystemPath::GeneratedDefaultPath { path: right }, - ) => left == right, - ( - FileSystemPath::Special { value: left } - | FileSystemPath::GeneratedDefaultSpecial { value: left }, - FileSystemPath::Special { value: right } - | FileSystemPath::GeneratedDefaultSpecial { value: right }, - ) => special_paths_share_target(left, right), + (FileSystemPath::Path { path: left }, FileSystemPath::Path { path: right }) => { + left == right + } + (FileSystemPath::Special { value: left }, FileSystemPath::Special { value: right }) => { + special_paths_share_target(left, right) + } (FileSystemPath::Path { path }, FileSystemPath::Special { value }) - | (FileSystemPath::Path { path }, FileSystemPath::GeneratedDefaultSpecial { value }) - | (FileSystemPath::GeneratedDefaultPath { path }, FileSystemPath::Special { value }) - | ( - FileSystemPath::GeneratedDefaultPath { path }, - FileSystemPath::GeneratedDefaultSpecial { value }, - ) - | (FileSystemPath::Special { value }, FileSystemPath::Path { path }) - | (FileSystemPath::Special { value }, FileSystemPath::GeneratedDefaultPath { path }) - | (FileSystemPath::GeneratedDefaultSpecial { value }, FileSystemPath::Path { path }) - | ( - FileSystemPath::GeneratedDefaultSpecial { value }, - FileSystemPath::GeneratedDefaultPath { path }, - ) => special_path_matches_absolute_path(value, path), + | (FileSystemPath::Special { value }, FileSystemPath::Path { path }) => { + special_path_matches_absolute_path(value, path) + } ( FileSystemPath::GlobPattern { pattern: left }, FileSystemPath::GlobPattern { pattern: right }, @@ -1773,7 +1670,7 @@ fn append_default_read_only_project_root_subpath_if_no_explicit_rule( ) { append_default_read_only_entry_if_no_explicit_rule( entries, - FileSystemPath::GeneratedDefaultSpecial { + FileSystemPath::Special { value: FileSystemSpecialPath::project_roots(Some(subpath.into())), }, ); @@ -2073,19 +1970,19 @@ mod tests { access: FileSystemAccessMode::Write, }, FileSystemSandboxEntry { - path: FileSystemPath::GeneratedDefaultSpecial { + path: FileSystemPath::Special { value: FileSystemSpecialPath::project_roots(Some(".git".into())), }, access: FileSystemAccessMode::Read, }, FileSystemSandboxEntry { - path: FileSystemPath::GeneratedDefaultSpecial { + path: FileSystemPath::Special { value: FileSystemSpecialPath::project_roots(Some(".agents".into())), }, access: FileSystemAccessMode::Read, }, FileSystemSandboxEntry { - path: FileSystemPath::GeneratedDefaultSpecial { + path: FileSystemPath::Special { value: FileSystemSpecialPath::project_roots(Some(".codex".into())), }, access: FileSystemAccessMode::Read, @@ -2232,7 +2129,7 @@ mod tests { ]; expected_entries.extend(PROTECTED_METADATA_PATH_NAMES.iter().map(|name| { FileSystemSandboxEntry { - path: FileSystemPath::GeneratedDefaultSpecial { + path: FileSystemPath::Special { value: FileSystemSpecialPath::project_roots(Some((*name).into())), }, access: FileSystemAccessMode::Read, diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 9166a804e01b..958a259182b6 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -41,21 +41,10 @@ pub fn normalize_additional_permissions( .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) .unwrap_or(path), }, - FileSystemPath::GeneratedDefaultPath { path } => { - FileSystemPath::GeneratedDefaultPath { - 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 }, - FileSystemPath::GeneratedDefaultSpecial { value } => { - FileSystemPath::GeneratedDefaultSpecial { value } - } }; let normalized_entry = FileSystemSandboxEntry { path, @@ -313,10 +302,7 @@ fn deny_entry_constrains_accepted_grant( match &deny_entry.path { FileSystemPath::GlobPattern { pattern } => glob_static_prefix_path(pattern, cwd) .is_some_and(|prefix| paths_overlap(prefix.as_path(), grant_path.as_path())), - FileSystemPath::Path { .. } - | FileSystemPath::GeneratedDefaultPath { .. } - | FileSystemPath::Special { .. } - | FileSystemPath::GeneratedDefaultSpecial { .. } => { + FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => { resolve_permission_path(&deny_entry.path, cwd).is_some_and(|deny_path| { paths_overlap(deny_path.as_path(), grant_path.as_path()) }) @@ -363,14 +349,6 @@ fn materialize_cwd_dependent_entry( cwd: &Path, ) -> FileSystemSandboxEntry { match &entry.path { - FileSystemPath::GeneratedDefaultSpecial { - value: FileSystemSpecialPath::ProjectRoots { .. }, - } => resolve_permission_path(&entry.path, cwd) - .map(|path| FileSystemSandboxEntry { - path: FileSystemPath::GeneratedDefaultPath { path }, - access: entry.access, - }) - .unwrap_or_else(|| entry.clone()), FileSystemPath::Special { value: FileSystemSpecialPath::ProjectRoots { .. }, } => resolve_permission_path(&entry.path, cwd) @@ -387,48 +365,41 @@ fn materialize_cwd_dependent_entry( }, access: entry.access, }, - FileSystemPath::Path { .. } - | FileSystemPath::GeneratedDefaultPath { .. } - | FileSystemPath::Special { .. } - | FileSystemPath::GeneratedDefaultSpecial { .. } => entry.clone(), + FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => entry.clone(), } } fn resolve_permission_path(path: &FileSystemPath, cwd: &Path) -> Option { match path { - FileSystemPath::Path { path } | FileSystemPath::GeneratedDefaultPath { path } => { - Some(path.clone()) - } + FileSystemPath::Path { path } => Some(path.clone()), FileSystemPath::GlobPattern { .. } => None, - FileSystemPath::Special { value } | FileSystemPath::GeneratedDefaultSpecial { value } => { - match value { - FileSystemSpecialPath::Root => { - let root = cwd.ancestors().last()?; - AbsolutePathBuf::from_absolute_path(root).ok() - } - FileSystemSpecialPath::ProjectRoots { subpath } => { - let cwd = AbsolutePathBuf::from_absolute_path(cwd).ok()?; - Some(match subpath { - Some(subpath) => { - AbsolutePathBuf::resolve_path_against_base(subpath, cwd.as_path()) - } - None => cwd, - }) - } - FileSystemSpecialPath::Tmpdir => { - let tmpdir = std::env::var_os("TMPDIR")?; - if tmpdir.is_empty() { - None - } else { - AbsolutePathBuf::from_absolute_path(PathBuf::from(tmpdir)).ok() + FileSystemPath::Special { value } => match value { + FileSystemSpecialPath::Root => { + let root = cwd.ancestors().last()?; + AbsolutePathBuf::from_absolute_path(root).ok() + } + FileSystemSpecialPath::ProjectRoots { subpath } => { + let cwd = AbsolutePathBuf::from_absolute_path(cwd).ok()?; + Some(match subpath { + Some(subpath) => { + AbsolutePathBuf::resolve_path_against_base(subpath, cwd.as_path()) } + None => cwd, + }) + } + FileSystemSpecialPath::Tmpdir => { + let tmpdir = std::env::var_os("TMPDIR")?; + if tmpdir.is_empty() { + None + } else { + AbsolutePathBuf::from_absolute_path(PathBuf::from(tmpdir)).ok() } - FileSystemSpecialPath::SlashTmp => AbsolutePathBuf::from_absolute_path("/tmp") - .ok() - .filter(|path| path.as_path().is_dir()), - FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => None, } - } + FileSystemSpecialPath::SlashTmp => AbsolutePathBuf::from_absolute_path("/tmp") + .ok() + .filter(|path| path.as_path().is_dir()), + FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => None, + }, } } diff --git a/codex-rs/sandboxing/src/windows.rs b/codex-rs/sandboxing/src/windows.rs index 0fef24e97d87..a57281145c25 100644 --- a/codex-rs/sandboxing/src/windows.rs +++ b/codex-rs/sandboxing/src/windows.rs @@ -87,7 +87,7 @@ pub fn resolve_windows_restricted_token_filesystem_overrides( return Ok(None); } - let (mut file_system_sandbox_policy, network_sandbox_policy) = + let (file_system_sandbox_policy, network_sandbox_policy) = permission_profile.to_runtime_permissions(); let needs_direct_runtime_enforcement = file_system_sandbox_policy @@ -107,11 +107,6 @@ pub fn resolve_windows_restricted_token_filesystem_overrides( )); } - // Windows protects built-in metadata defaults through the legacy writable - // root projection when those paths exist. Do not turn missing generated - // defaults into explicit deny-write sentinels. - file_system_sandbox_policy.remove_generated_defaults(); - // The restricted-token backend can still enforce split write restrictions, // but its WRITE_RESTRICTED token does not make capability SID deny-read ACEs // participate in read access checks. Read restrictions therefore require the @@ -226,7 +221,7 @@ pub fn resolve_windows_elevated_filesystem_overrides( return Ok(None); } - let (mut file_system_sandbox_policy, network_sandbox_policy) = + let (file_system_sandbox_policy, network_sandbox_policy) = permission_profile.to_runtime_permissions(); if !permission_profile_supports_windows_restricted_token_sandbox(permission_profile) { @@ -237,11 +232,6 @@ pub fn resolve_windows_elevated_filesystem_overrides( )); } - // Windows protects built-in metadata defaults through the legacy writable - // root projection when those paths exist. Do not turn missing generated - // defaults into explicit deny-write sentinels. - file_system_sandbox_policy.remove_generated_defaults(); - let additional_deny_read_paths = codex_windows_sandbox::resolve_windows_deny_read_paths( &file_system_sandbox_policy, sandbox_policy_cwd, @@ -258,12 +248,6 @@ pub fn resolve_windows_elevated_filesystem_overrides( let needs_direct_runtime_enforcement = file_system_sandbox_policy .needs_direct_runtime_enforcement(network_sandbox_policy, sandbox_policy_cwd); - let has_explicit_write_carveouts = split_writable_roots.iter().any(|writable_root| { - writable_root.read_only_subpaths.iter().any(|path| { - file_system_sandbox_policy - .has_explicit_non_write_entry_for_path_with_cwd(path.as_path(), sandbox_policy_cwd) - }) - }); let normalize_path = |path: PathBuf| dunce::canonicalize(&path).unwrap_or(path); let legacy_projection = compatibility_sandbox_policy_for_permission_profile( permission_profile, @@ -304,9 +288,7 @@ pub fn resolve_windows_elevated_filesystem_overrides( Some(split_root_paths) }; - let additional_deny_write_paths = if needs_direct_runtime_enforcement - || has_explicit_write_carveouts - { + let additional_deny_write_paths = if needs_direct_runtime_enforcement { let mut deny_paths = BTreeSet::new(); for writable_root in &split_writable_roots { let writable_root_path = normalize_path(writable_root.root.to_path_buf()); @@ -327,12 +309,7 @@ pub fn resolve_windows_elevated_filesystem_overrides( == read_only_subpath_suffix }) }); - let explicitly_configured = file_system_sandbox_policy - .has_explicit_non_write_entry_for_path_with_cwd( - read_only_subpath.as_path(), - sandbox_policy_cwd, - ); - if explicitly_configured || !already_denied_by_legacy { + if !already_denied_by_legacy { deny_paths.insert(normalize_path(read_only_subpath.to_path_buf())); } }