mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Avoid creating metadata paths in the Windows sandbox (#33430)
## Why Elevated Windows sandbox setup could turn the built-in read-only protections for workspace metadata into deny-write paths, creating metadata directories that did not already exist. ## What changed - Track whether filesystem permission entries come from a built-in profile or were explicitly configured. - Exclude generated metadata defaults from direct Windows deny-write overrides while continuing to enforce explicit read-only carveouts for the same paths. - Preserve the distinction while materializing and normalizing permission paths, without exposing generated defaults as user configuration. ## Testing Added Windows coverage confirming that elevated sandbox setup leaves missing workspace metadata absent and still produces overrides for explicit metadata carveouts. GitOrigin-RevId: 701bcb229f631b18ab5ced22f3edf6f303d932f3
This commit is contained in:
1 parent
79177c3e20
commit
cb79b3cdf9
11 files changed
+340
-60
No files matched your search
@@ -343,6 +343,11 @@ 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 {
|
||||
@@ -351,6 +356,20 @@ 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://";
|
||||
@@ -403,6 +422,25 @@ 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| {
|
||||
@@ -496,9 +534,12 @@ impl FileSystemSandboxPolicy {
|
||||
}
|
||||
|
||||
match &entry.path {
|
||||
FileSystemPath::Path { .. } => !self.has_same_target_write_override(entry),
|
||||
FileSystemPath::Path { .. } | FileSystemPath::GeneratedDefaultPath { .. } => {
|
||||
!self.has_same_target_write_override(entry)
|
||||
}
|
||||
FileSystemPath::GlobPattern { .. } => true,
|
||||
FileSystemPath::Special { value } => match value {
|
||||
FileSystemPath::Special { value }
|
||||
| FileSystemPath::GeneratedDefaultSpecial { value } => match value {
|
||||
FileSystemSpecialPath::Root => entry.access == FileSystemAccessMode::Deny,
|
||||
FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => {
|
||||
false
|
||||
@@ -717,6 +758,13 @@ 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 { .. },
|
||||
} => {
|
||||
@@ -734,7 +782,9 @@ impl FileSystemSandboxPolicy {
|
||||
}
|
||||
}
|
||||
FileSystemPath::Special { value: _ } => {}
|
||||
FileSystemPath::GeneratedDefaultSpecial { value: _ } => {}
|
||||
FileSystemPath::Path { .. } => {}
|
||||
FileSystemPath::GeneratedDefaultPath { .. } => {}
|
||||
}
|
||||
}
|
||||
self
|
||||
@@ -749,6 +799,22 @@ 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 },
|
||||
} => {
|
||||
@@ -786,12 +852,24 @@ 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;
|
||||
@@ -1095,7 +1173,10 @@ impl FileSystemSandboxPolicy {
|
||||
FileSystemPath::GlobPattern { pattern } => {
|
||||
Some(AbsolutePathBuf::resolve_path_against_base(pattern, cwd))
|
||||
}
|
||||
FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => None,
|
||||
FileSystemPath::Path { .. }
|
||||
| FileSystemPath::GeneratedDefaultPath { .. }
|
||||
| FileSystemPath::Special { .. }
|
||||
| FileSystemPath::GeneratedDefaultSpecial { .. } => None,
|
||||
})
|
||||
.map(|pattern| pattern.to_string_lossy().into_owned())
|
||||
.collect::<Vec<_>>();
|
||||
@@ -1138,7 +1219,8 @@ impl FileSystemSandboxPolicy {
|
||||
for entry in &self.entries {
|
||||
match &entry.path {
|
||||
FileSystemPath::GlobPattern { .. } => {}
|
||||
FileSystemPath::Path { path } => {
|
||||
FileSystemPath::Path { path }
|
||||
| FileSystemPath::GeneratedDefaultPath { path } => {
|
||||
if entry.access.can_write() {
|
||||
if cwd_absolute.as_ref().is_some_and(|cwd| cwd == path) {
|
||||
workspace_root_writable = true;
|
||||
@@ -1147,7 +1229,8 @@ impl FileSystemSandboxPolicy {
|
||||
}
|
||||
}
|
||||
}
|
||||
FileSystemPath::Special { value } => match value {
|
||||
FileSystemPath::Special { value }
|
||||
| FileSystemPath::GeneratedDefaultSpecial { value } => match value {
|
||||
FileSystemSpecialPath::Root => match entry.access {
|
||||
FileSystemAccessMode::Deny => {}
|
||||
FileSystemAccessMode::Read => {}
|
||||
@@ -1289,9 +1372,13 @@ fn resolve_file_system_path(
|
||||
cwd: Option<&AbsolutePathBuf>,
|
||||
) -> Option<AbsolutePathBuf> {
|
||||
match path {
|
||||
FileSystemPath::Path { path } => Some(path.clone()),
|
||||
FileSystemPath::Path { path } | FileSystemPath::GeneratedDefaultPath { path } => {
|
||||
Some(path.clone())
|
||||
}
|
||||
FileSystemPath::GlobPattern { .. } => None,
|
||||
FileSystemPath::Special { value } => resolve_file_system_special_path(value, cwd),
|
||||
FileSystemPath::Special { value } | FileSystemPath::GeneratedDefaultSpecial { value } => {
|
||||
resolve_file_system_special_path(value, cwd)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1335,16 +1422,32 @@ fn resolve_candidate_path(path: &Path, cwd: &Path) -> Option<AbsolutePathBuf> {
|
||||
/// by `has_write_narrowing_entries`.
|
||||
fn file_system_paths_share_target(left: &FileSystemPath, right: &FileSystemPath) -> bool {
|
||||
match (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: 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 }, FileSystemPath::Special { value })
|
||||
| (FileSystemPath::Special { value }, FileSystemPath::Path { path }) => {
|
||||
special_path_matches_absolute_path(value, path)
|
||||
}
|
||||
| (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::GlobPattern { pattern: left },
|
||||
FileSystemPath::GlobPattern { pattern: right },
|
||||
@@ -1670,7 +1773,7 @@ fn append_default_read_only_project_root_subpath_if_no_explicit_rule(
|
||||
) {
|
||||
append_default_read_only_entry_if_no_explicit_rule(
|
||||
entries,
|
||||
FileSystemPath::Special {
|
||||
FileSystemPath::GeneratedDefaultSpecial {
|
||||
value: FileSystemSpecialPath::project_roots(Some(subpath.into())),
|
||||
},
|
||||
);
|
||||
@@ -1970,19 +2073,19 @@ mod tests {
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
path: FileSystemPath::GeneratedDefaultSpecial {
|
||||
value: FileSystemSpecialPath::project_roots(Some(".git".into())),
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
path: FileSystemPath::GeneratedDefaultSpecial {
|
||||
value: FileSystemSpecialPath::project_roots(Some(".agents".into())),
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
path: FileSystemPath::GeneratedDefaultSpecial {
|
||||
value: FileSystemSpecialPath::project_roots(Some(".codex".into())),
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
@@ -2129,7 +2232,7 @@ mod tests {
|
||||
];
|
||||
expected_entries.extend(PROTECTED_METADATA_PATH_NAMES.iter().map(|name| {
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
path: FileSystemPath::GeneratedDefaultSpecial {
|
||||
value: FileSystemSpecialPath::project_roots(Some((*name).into())),
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
|
||||
Reference in new issue
Block a user