Preserve filesystem permission path conventions (#39084)

## Why

Filesystem permission paths can use a convention that differs from the host
running Codex. Converting them immediately to native absolute paths can change
the meaning of ambiguous paths such as `/C:/secret` or Windows UNC paths.

## What changed

- Store literal filesystem permission paths as `PathUri` values through the
  runtime policy and execution protocol.
- Keep legacy string-based serialization at explicit protocol boundaries and
  reject conversions that cannot be represented losslessly.
- Encode native paths as opaque URIs when a normal file URI would imply the
  wrong path convention.

## Testing

Added coverage for cross-platform and ambiguous path round trips, UNC path
variants, permission-profile serialization, and deny-policy enforcement.

GitOrigin-RevId: 5247713796d1f2bb4e02f94eb9fc82d4698060f0
This commit is contained in:
iceweasel-oai authored and copyberry committed 2026-08-17 21:49:30 +00:00
1 parent a4f37a5b7f
commit 2013e04354
46 files changed
+765 -260

No files matched your search

+260 -50
View File
@@ -6,6 +6,8 @@ use std::path::PathBuf;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::canonicalize_preserving_symlinks;
use codex_utils_path_uri::LegacyAppPathString;
use codex_utils_path_uri::PathUri;
use globset::GlobBuilder;
use globset::GlobMatcher;
use schemars::JsonSchema;
@@ -170,10 +172,20 @@ impl FileSystemSpecialPath {
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct FileSystemSandboxEntry {
pub path: FileSystemPath,
pub access: FileSystemAccessMode,
pub missing_path_behavior: Option<FileSystemSandboxEntryMissingPathBehavior>,
}
/// Serialized filesystem entry used at legacy string-based seams.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)]
#[schemars(rename = "FileSystemSandboxEntry")]
#[ts(rename = "FileSystemSandboxEntry")]
pub struct RawFileSystemSandboxEntry {
pub path: RawFileSystemPath,
pub access: FileSystemAccessMode,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub missing_path_behavior: Option<FileSystemSandboxEntryMissingPathBehavior>,
@@ -219,14 +231,24 @@ pub enum FileSystemSandboxKind {
ExternalSandbox,
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct FileSystemSandboxPolicy {
pub kind: FileSystemSandboxKind,
pub glob_scan_max_depth: Option<usize>,
pub entries: Vec<FileSystemSandboxEntry>,
}
/// Serialized filesystem policy used at legacy string-based seams.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)]
#[schemars(rename = "FileSystemSandboxPolicy")]
#[ts(rename = "FileSystemSandboxPolicy")]
pub struct RawFileSystemSandboxPolicy {
pub kind: FileSystemSandboxKind,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub glob_scan_max_depth: Option<usize>,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub entries: Vec<FileSystemSandboxEntry>,
pub entries: Vec<RawFileSystemSandboxEntry>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -361,13 +383,10 @@ enum InvalidDenyReadGlobBehavior {
ReturnError,
}
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)]
#[serde(tag = "type", rename_all = "snake_case")]
#[ts(tag = "type")]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum FileSystemPath {
Path {
// TODO(anp): Use PathUri once permission paths no longer require native-path rollout serialization.
path: AbsolutePathBuf,
path: PathUri,
},
/// A git-style glob pattern. Pattern entries currently support
/// FileSystemAccessMode::Deny only.
@@ -379,12 +398,142 @@ pub enum FileSystemPath {
},
}
/// Serialized filesystem path whose literal path variant preserves the raw
/// legacy string until an explicit seam conversion selects its meaning.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)]
#[serde(tag = "type", rename_all = "snake_case")]
#[ts(tag = "type")]
#[schemars(rename = "FileSystemPath")]
#[ts(rename = "FileSystemPath")]
pub enum RawFileSystemPath {
Path {
#[schemars(with = "AbsolutePathBuf")]
#[ts(type = "string")]
path: LegacyAppPathString,
},
GlobPattern {
pattern: String,
},
Special {
value: FileSystemSpecialPath,
},
}
impl From<AbsolutePathBuf> for FileSystemPath {
fn from(path: AbsolutePathBuf) -> Self {
Self::Path { path: path.into() }
}
}
impl From<PathUri> for FileSystemPath {
fn from(path: PathUri) -> Self {
Self::Path { path }
}
}
fn path_uri_from_raw(path: LegacyAppPathString) -> Result<PathUri, String> {
let native_path =
serde::de::value::StrDeserializer::<serde::de::value::Error>::new(path.as_str());
if let Ok(path) = AbsolutePathBuf::deserialize(native_path) {
return Ok(PathUri::from(path));
}
PathUri::try_from(path).map_err(|err| err.to_string())
}
fn raw_path_from_uri(path: PathUri) -> Result<LegacyAppPathString, String> {
let raw_path = LegacyAppPathString::from(path.clone());
if path_uri_from_raw(raw_path.clone()).as_ref() == Ok(&path) {
Ok(raw_path)
} else {
Err("permission path cannot be represented losslessly".to_string())
}
}
impl TryFrom<RawFileSystemPath> for FileSystemPath {
type Error = String;
fn try_from(path: RawFileSystemPath) -> Result<Self, Self::Error> {
Ok(match path {
RawFileSystemPath::Path { path } => Self::Path {
path: path_uri_from_raw(path)?,
},
RawFileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern },
RawFileSystemPath::Special { value } => Self::Special { value },
})
}
}
impl TryFrom<FileSystemPath> for RawFileSystemPath {
type Error = String;
fn try_from(path: FileSystemPath) -> Result<Self, Self::Error> {
Ok(match path {
FileSystemPath::Path { path } => Self::Path {
path: raw_path_from_uri(path)?,
},
FileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern },
FileSystemPath::Special { value } => Self::Special { value },
})
}
}
impl TryFrom<RawFileSystemSandboxEntry> for FileSystemSandboxEntry {
type Error = String;
fn try_from(entry: RawFileSystemSandboxEntry) -> Result<Self, Self::Error> {
Ok(Self {
path: entry.path.try_into()?,
access: entry.access,
missing_path_behavior: entry.missing_path_behavior,
})
}
}
impl TryFrom<FileSystemSandboxEntry> for RawFileSystemSandboxEntry {
type Error = String;
fn try_from(entry: FileSystemSandboxEntry) -> Result<Self, Self::Error> {
Ok(Self {
path: entry.path.try_into()?,
access: entry.access,
missing_path_behavior: entry.missing_path_behavior,
})
}
}
impl TryFrom<RawFileSystemSandboxPolicy> for FileSystemSandboxPolicy {
type Error = String;
fn try_from(policy: RawFileSystemSandboxPolicy) -> Result<Self, Self::Error> {
Ok(Self {
kind: policy.kind,
glob_scan_max_depth: policy.glob_scan_max_depth,
entries: policy
.entries
.into_iter()
.map(TryInto::try_into)
.collect::<Result<_, _>>()?,
})
}
}
impl TryFrom<FileSystemSandboxPolicy> for RawFileSystemSandboxPolicy {
type Error = String;
fn try_from(policy: FileSystemSandboxPolicy) -> Result<Self, Self::Error> {
Ok(Self {
kind: policy.kind,
glob_scan_max_depth: policy.glob_scan_max_depth,
entries: policy
.entries
.into_iter()
.map(TryInto::try_into)
.collect::<Result<_, _>>()?,
})
}
}
const PROJECT_ROOTS_GLOB_PATTERN_PREFIX: &str = "codex-project-roots://";
pub fn project_roots_glob_pattern(subpath: &Path) -> String {
@@ -931,7 +1080,7 @@ impl FileSystemSandboxPolicy {
for path in additional_writable_roots {
if !self.entries.iter().any(|entry| {
entry.access.can_write()
&& matches!(&entry.path, FileSystemPath::Path { path: existing } if existing == path)
&& matches!(&entry.path, FileSystemPath::Path { path: existing } if existing == &PathUri::from_abs_path(path))
}) {
self.entries.push(FileSystemSandboxEntry::new(
path.clone().into(),
@@ -1197,10 +1346,11 @@ impl FileSystemSandboxPolicy {
FileSystemPath::GlobPattern { .. } => {}
FileSystemPath::Path { path } => {
if entry.access.can_write() {
if cwd_absolute.as_ref().is_some_and(|cwd| cwd == path) {
let path = path.to_abs_path()?;
if cwd_absolute.as_ref().is_some_and(|cwd| cwd == &path) {
workspace_root_writable = true;
} else {
writable_roots.push(path.clone());
writable_roots.push(path);
}
}
}
@@ -1346,7 +1496,7 @@ fn resolve_file_system_path(
cwd: Option<&AbsolutePathBuf>,
) -> Option<AbsolutePathBuf> {
match path {
FileSystemPath::Path { path } => Some(path.clone()),
FileSystemPath::Path { path } => path.to_abs_path().ok(),
FileSystemPath::GlobPattern { .. } => None,
FileSystemPath::Special { value } => resolve_file_system_special_path(value, cwd),
}
@@ -1399,9 +1549,9 @@ fn file_system_paths_share_target(left: &FileSystemPath, right: &FileSystemPath)
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::Special { value }, FileSystemPath::Path { path }) => path
.to_abs_path()
.is_ok_and(|path| special_path_matches_absolute_path(value, &path)),
(
FileSystemPath::GlobPattern { pattern: left },
FileSystemPath::GlobPattern { pattern: right },
@@ -1941,6 +2091,67 @@ mod tests {
std::os::unix::fs::symlink(original, link)
}
#[test]
fn permission_paths_preserve_native_strings_across_path_conventions() {
for path in [
"/workspace/src",
r"C:\workspace\src",
r"\\server\share\src",
r"\\localhost\share",
] {
let expected = serde_json::json!({ "type": "path", "path": path });
let actual = serde_json::from_value::<RawFileSystemPath>(expected.clone())
.expect("valid raw permission path");
assert_eq!(
serde_json::to_value(actual).expect("lossless raw permission path"),
expected
);
}
}
#[cfg(windows)]
#[test]
fn permission_paths_preserve_native_slash_unc_strings() {
for path in ["//server/share/src", r"/\server/share/src"] {
let expected = serde_json::json!({ "type": "path", "path": path });
let actual = serde_json::from_value::<RawFileSystemPath>(expected.clone())
.expect("valid raw slash UNC permission path");
assert_eq!(
serde_json::to_value(actual).expect("lossless raw slash UNC permission path"),
expected
);
}
}
#[cfg(unix)]
#[test]
fn native_ambiguous_permission_paths_keep_deny_semantics() {
let cwd = TempDir::new().expect("tempdir");
for path in ["//server/share/secret", "/C:/secret"] {
let denied_path = serde_json::from_value::<RawFileSystemPath>(serde_json::json!({
"type": "path",
"path": path,
}))
.expect("raw permission path")
.try_into()
.expect("runtime permission path");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
FileSystemAccessMode::Read,
),
FileSystemSandboxEntry::new(denied_path, FileSystemAccessMode::Deny),
]);
assert!(
!policy.can_read_path_with_cwd(Path::new(path), cwd.path()),
"deny should apply to {path}"
);
}
}
#[test]
fn unknown_special_paths_are_ignored_by_legacy_bridge() -> std::io::Result<()> {
let policy = FileSystemSandboxPolicy::restricted(vec![
@@ -2174,7 +2385,7 @@ mod tests {
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: explicit_dot_codex.clone(),
path: explicit_dot_codex.clone().into(),
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
@@ -2215,7 +2426,7 @@ mod tests {
let root = AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute cwd");
let file_system_policy =
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Path { path: root },
path: root.into(),
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
}]);
@@ -2289,10 +2500,7 @@ mod tests {
)
.into_iter()
.map(|path| {
FileSystemSandboxEntry::skip_missing_path(
FileSystemPath::Path { path },
FileSystemAccessMode::Read,
)
FileSystemSandboxEntry::skip_missing_path(path.into(), FileSystemAccessMode::Read)
}),
);
@@ -2342,12 +2550,12 @@ mod tests {
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: link_root },
path: link_root.into(),
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: link_blocked },
path: link_blocked.into(),
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
@@ -2412,7 +2620,7 @@ mod tests {
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: link_blocked },
path: link_blocked.into(),
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
@@ -2470,7 +2678,7 @@ mod tests {
.expect("absolute canonical decoy");
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Path { path: root },
path: root.into(),
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
}]);
@@ -2511,12 +2719,12 @@ mod tests {
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: link_root },
path: link_root.into(),
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: link_private },
path: link_private.into(),
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
@@ -2560,12 +2768,12 @@ mod tests {
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: link_root },
path: link_root.into(),
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: link_private },
path: link_private.into(),
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
@@ -2604,12 +2812,12 @@ mod tests {
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: root },
path: root.into(),
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: alias },
path: alias.into(),
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
@@ -2671,7 +2879,7 @@ mod tests {
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: link_blocked },
path: link_blocked.into(),
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
@@ -2713,20 +2921,20 @@ mod tests {
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: docs.clone() },
path: docs.clone().into(),
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: docs_private.clone(),
path: docs_private.clone().into(),
},
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: docs_private_public.clone(),
path: docs_private_public.clone().into(),
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
@@ -2764,7 +2972,7 @@ mod tests {
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: docs },
path: docs.into(),
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
},
@@ -2852,7 +3060,7 @@ mod tests {
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: docs.clone() },
path: docs.clone().into(),
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
},
@@ -2892,7 +3100,7 @@ mod tests {
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: docs.clone() },
path: docs.clone().into(),
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
},
@@ -2952,12 +3160,12 @@ mod tests {
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: docs.clone() },
path: docs.clone().into(),
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: docs.clone() },
path: docs.clone().into(),
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
},
@@ -3035,7 +3243,7 @@ mod tests {
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: extra },
path: extra.into(),
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
},
@@ -3082,28 +3290,28 @@ mod tests {
FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: first.clone(),
path: first.clone().into(),
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: second.clone(),
path: second.clone().into(),
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: first.join(".git"),
path: first.join(".git").into(),
},
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: second.join(".git"),
path: second.join(".git").into(),
},
access: FileSystemAccessMode::Read,
missing_path_behavior: None,
@@ -3192,14 +3400,14 @@ mod tests {
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: extra.clone()
path: extra.clone().into()
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
},
FileSystemSandboxEntry::skip_missing_path(
FileSystemPath::Path {
path: extra.join(".git")
path: extra.join(".git").into()
},
FileSystemAccessMode::Read,
),
@@ -3218,7 +3426,7 @@ mod tests {
let denied = AbsolutePathBuf::try_from("/tmp/private").expect("absolute path");
let existing = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: denied.clone(),
path: denied.clone().into(),
},
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,
@@ -3234,7 +3442,7 @@ mod tests {
rebuilt.entries.iter().any(|entry| {
entry.path
== FileSystemPath::Path {
path: denied.clone(),
path: denied.clone().into(),
}
&& entry.access == FileSystemAccessMode::Deny
}),
@@ -3268,7 +3476,9 @@ mod tests {
fn deny_policy(path: &Path) -> FileSystemSandboxPolicy {
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: AbsolutePathBuf::try_from(path).expect("absolute deny path"),
path: AbsolutePathBuf::try_from(path)
.expect("absolute deny path")
.into(),
},
access: FileSystemAccessMode::Deny,
missing_path_behavior: None,