Preserve special filesystem subpaths as wire strings (#33107)

## Why

Special filesystem subpaths can contain relative or foreign-platform path
spellings. Treating this wire text as a host-native path can reinterpret it.

## What changed

- Store special filesystem subpaths as strings in the core permissions model.
- Represent app-server subpaths as `LegacyAppPathString` and convert them at
  the protocol boundary without changing their spelling.
- Add `LegacyAppPathString::from_string` for callers that already own legacy
  app-server path text.

GitOrigin-RevId: 2214f03aa997fd0e6eb2ab60d425304bb4bd6132
This commit is contained in:
Adam Perry @ OpenAI
2026-07-14 16:40:13 +00:00
committed by copyberry
parent fca8c00f11
commit 0396f99cf1
19 changed files with 199 additions and 97 deletions
@@ -401,9 +401,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -455,9 +459,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -186,9 +186,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -240,9 +244,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -182,9 +182,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -236,9 +240,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -1482,9 +1482,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -1536,9 +1540,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
+14 -6
View File
@@ -763,9 +763,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -817,9 +821,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -9772,9 +9772,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/v2/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -9826,9 +9830,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/v2/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -6005,9 +6005,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -6059,9 +6063,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -193,9 +193,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -247,9 +251,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -186,9 +186,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -240,9 +244,13 @@
"type": "string"
},
"subpath": {
"type": [
"string",
"null"
"anyOf": [
{
"$ref": "#/definitions/LegacyAppPathString"
},
{
"type": "null"
}
]
}
},
@@ -16,11 +16,12 @@
* boundary. Non-UTF-8 paths are converted to UTF-8 lossily because this API
* value is serialized as a JSON string.
*
* Deserialization accepts any UTF-8 string without interpreting or validating
* it. That unrestricted construction path is intentionally available only to
* serde: Codex-internal code cannot construct this type directly from a raw
* `String` and is instead encouraged to convert through [`PathUri`] or
* [`AbsolutePathBuf`]. Relative path text remains valid until an operation
* such as [`Self::to_path_uri`] requires an absolute path.
* Deserialization and [`Self::from_string`] accept any UTF-8 string without
* interpreting or validating it. Use [`Self::from_string`] when a caller
* already owns legacy app-server path text and needs to preserve its wire
* spelling; use [`Self::from_path`], [`Self::from_abs_path`], or
* [`Self::from_path_uri`] when converting an actual path value. Relative
* path text remains valid until an operation such as [`Self::to_path_uri`]
* requires an absolute path.
*/
export type LegacyAppPathString = string;
@@ -1,5 +1,6 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { LegacyAppPathString } from "../LegacyAppPathString";
export type FileSystemSpecialPath = { "kind": "root" } | { "kind": "minimal" } | { "kind": "project_roots", subpath: string | null, } | { "kind": "tmpdir" } | { "kind": "slash_tmp" } | { "kind": "unknown", path: string, subpath: string | null, };
export type FileSystemSpecialPath = { "kind": "root" } | { "kind": "minimal" } | { "kind": "project_roots", subpath: LegacyAppPathString | null, } | { "kind": "tmpdir" } | { "kind": "slash_tmp" } | { "kind": "unknown", path: string, subpath: LegacyAppPathString | null, };
@@ -23,7 +23,7 @@ use serde::Deserialize;
use serde::Serialize;
use std::io;
use std::num::NonZeroUsize;
use std::path::PathBuf;
use std::path::Path;
use ts_rs::TS;
v2_enum_from_core! {
@@ -244,13 +244,13 @@ pub enum FileSystemSpecialPath {
Minimal,
#[serde(alias = "current_working_directory")]
ProjectRoots {
subpath: Option<PathBuf>,
subpath: Option<LegacyAppPathString>,
},
Tmpdir,
SlashTmp,
Unknown {
path: String,
subpath: Option<PathBuf>,
subpath: Option<LegacyAppPathString>,
},
}
@@ -259,10 +259,21 @@ impl From<CoreFileSystemSpecialPath> for FileSystemSpecialPath {
match value {
CoreFileSystemSpecialPath::Root => Self::Root,
CoreFileSystemSpecialPath::Minimal => Self::Minimal,
CoreFileSystemSpecialPath::ProjectRoots { subpath } => Self::ProjectRoots { subpath },
CoreFileSystemSpecialPath::ProjectRoots { subpath } => Self::ProjectRoots {
subpath: subpath
.as_deref()
.map(Path::new)
.map(LegacyAppPathString::from_path),
},
CoreFileSystemSpecialPath::Tmpdir => Self::Tmpdir,
CoreFileSystemSpecialPath::SlashTmp => Self::SlashTmp,
CoreFileSystemSpecialPath::Unknown { path, subpath } => Self::Unknown { path, subpath },
CoreFileSystemSpecialPath::Unknown { path, subpath } => Self::Unknown {
path,
subpath: subpath
.as_deref()
.map(Path::new)
.map(LegacyAppPathString::from_path),
},
}
}
}
@@ -272,10 +283,15 @@ impl From<FileSystemSpecialPath> for CoreFileSystemSpecialPath {
match value {
FileSystemSpecialPath::Root => Self::Root,
FileSystemSpecialPath::Minimal => Self::Minimal,
FileSystemSpecialPath::ProjectRoots { subpath } => Self::ProjectRoots { subpath },
FileSystemSpecialPath::ProjectRoots { subpath } => Self::ProjectRoots {
subpath: subpath.map(LegacyAppPathString::into_string),
},
FileSystemSpecialPath::Tmpdir => Self::Tmpdir,
FileSystemSpecialPath::SlashTmp => Self::SlashTmp,
FileSystemSpecialPath::Unknown { path, subpath } => Self::Unknown { path, subpath },
FileSystemSpecialPath::Unknown { path, subpath } => Self::Unknown {
path,
subpath: subpath.map(LegacyAppPathString::into_string),
},
}
}
}
+8 -10
View File
@@ -274,7 +274,7 @@ fn insert_special_filesystem_permission_toml(
insert_scoped_filesystem_permission_toml(
entries,
":workspace_roots".to_string(),
subpath.unwrap_or_else(|| PathBuf::from(".")),
subpath.unwrap_or_else(|| ".".to_string()),
access,
);
}
@@ -303,7 +303,7 @@ fn insert_special_filesystem_permission_toml(
fn insert_scoped_filesystem_permission_toml(
entries: &mut BTreeMap<String, FilesystemPermissionToml>,
path: String,
subpath: PathBuf,
subpath: String,
access: FileSystemAccessMode,
) {
let permission = entries
@@ -311,13 +311,10 @@ fn insert_scoped_filesystem_permission_toml(
.or_insert_with(|| FilesystemPermissionToml::Scoped(BTreeMap::new()));
match permission {
FilesystemPermissionToml::Scoped(scoped_entries) => {
scoped_entries.insert(subpath.to_string_lossy().into_owned(), access);
scoped_entries.insert(subpath, access);
}
FilesystemPermissionToml::Access(_) => {
*permission = FilesystemPermissionToml::Scoped(BTreeMap::from([(
subpath.to_string_lossy().into_owned(),
access,
)]));
*permission = FilesystemPermissionToml::Scoped(BTreeMap::from([(subpath, access)]));
}
}
}
@@ -608,7 +605,9 @@ fn compile_scoped_filesystem_path(
}
if let Some(special) = parse_special_path(path) {
let subpath = parse_relative_subpath(subpath)?;
let subpath = parse_relative_subpath(subpath)?
.to_string_lossy()
.into_owned();
let special = match special {
FileSystemSpecialPath::ProjectRoots { .. } => Ok(FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(Some(subpath)),
@@ -894,8 +893,7 @@ fn maybe_push_unknown_special_path_warning(
startup_warnings,
match subpath.as_deref() {
Some(subpath) => format!(
"Configured filesystem path `{path}` with nested entry `{}` is not recognized by this version of Codex and will be ignored. Upgrade Codex if this path is required.",
subpath.display()
"Configured filesystem path `{path}` with nested entry `{subpath}` is not recognized by this version of Codex and will be ignored. Upgrade Codex if this path is required."
),
None => format!(
"Configured filesystem path `{path}` is not recognized by this version of Codex and will be ignored. Upgrade Codex if this path is required."
@@ -6,7 +6,6 @@ use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_utils_path_uri::PathUri;
use std::collections::HashSet;
use std::path::PathBuf;
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct FileSystemContext {
@@ -183,9 +182,9 @@ fn render_special_path(value: &FileSystemSpecialPath) -> String {
}
}
fn render_special_path_with_subpath(base: &str, subpath: &Option<PathBuf>) -> String {
fn render_special_path_with_subpath(base: &str, subpath: &Option<String>) -> String {
match subpath {
Some(subpath) => format!("{base}/{}", subpath.display()),
Some(subpath) => format!("{base}/{subpath}"),
None => base.to_string(),
}
}
@@ -156,7 +156,7 @@ fn workspace_write_permission_profile_with_private_denials() -> PermissionProfil
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(Some(PathBuf::from("private"))),
value: FileSystemSpecialPath::project_roots(Some("private".to_string())),
},
access: FileSystemAccessMode::Deny,
},
+1 -1
View File
@@ -410,7 +410,7 @@ fn create_filesystem_args(
// rules should keep their normal bwrap behavior, which can mask
// the first missing component to prevent creation under writable
// roots.
let project_subpath = subpath.as_path();
let project_subpath = Path::new(subpath);
if project_subpath != Path::new(".git")
&& project_subpath != Path::new(".agents")
&& project_subpath != Path::new(".codex")
+5 -5
View File
@@ -143,7 +143,7 @@ pub enum FileSystemSpecialPath {
ProjectRoots {
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
subpath: Option<PathBuf>,
subpath: Option<String>,
},
Tmpdir,
SlashTmp,
@@ -159,16 +159,16 @@ pub enum FileSystemSpecialPath {
path: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
subpath: Option<PathBuf>,
subpath: Option<String>,
},
}
impl FileSystemSpecialPath {
pub fn project_roots(subpath: Option<PathBuf>) -> Self {
pub fn project_roots(subpath: Option<String>) -> Self {
Self::ProjectRoots { subpath }
}
pub fn unknown(path: impl Into<String>, subpath: Option<PathBuf>) -> Self {
pub fn unknown(path: impl Into<String>, subpath: Option<String>) -> Self {
Self::Unknown {
path: path.into(),
subpath,
@@ -1712,7 +1712,7 @@ fn legacy_runtime_file_system_policy_for_cwd(
fn append_default_read_only_project_root_subpath_if_no_explicit_rule(
entries: &mut Vec<FileSystemSandboxEntry>,
subpath: impl Into<PathBuf>,
subpath: impl Into<String>,
) {
append_default_read_only_entry_if_no_explicit_rule(
entries,
@@ -55,6 +55,7 @@ use codex_protocol::ThreadId;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::LegacyAppPathString;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyEventKind;
@@ -1028,9 +1029,9 @@ fn special_path_label(value: &FileSystemSpecialPath) -> String {
}
}
fn path_label(base: &str, subpath: &Option<PathBuf>) -> String {
fn path_label(base: &str, subpath: &Option<LegacyAppPathString>) -> String {
match subpath {
Some(subpath) => format!("{base}/{}", subpath.display()),
Some(subpath) => format!("{base}/{subpath}"),
None => base.to_string(),
}
}
@@ -1871,7 +1872,9 @@ mod tests {
entries: Some(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::ProjectRoots {
subpath: Some(".git".into()),
subpath: Some(LegacyAppPathString::from_path(std::path::Path::new(
".git",
))),
},
},
access: FileSystemAccessMode::Read,
+18 -6
View File
@@ -24,18 +24,30 @@ use ts_rs::TS;
/// boundary. Non-UTF-8 paths are converted to UTF-8 lossily because this API
/// value is serialized as a JSON string.
///
/// Deserialization accepts any UTF-8 string without interpreting or validating
/// it. That unrestricted construction path is intentionally available only to
/// serde: Codex-internal code cannot construct this type directly from a raw
/// `String` and is instead encouraged to convert through [`PathUri`] or
/// [`AbsolutePathBuf`]. Relative path text remains valid until an operation
/// such as [`Self::to_path_uri`] requires an absolute path.
/// Deserialization and [`Self::from_string`] accept any UTF-8 string without
/// interpreting or validating it. Use [`Self::from_string`] when a caller
/// already owns legacy app-server path text and needs to preserve its wire
/// spelling; use [`Self::from_path`], [`Self::from_abs_path`], or
/// [`Self::from_path_uri`] when converting an actual path value. Relative
/// path text remains valid until an operation such as [`Self::to_path_uri`]
/// requires an absolute path.
#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, TS)]
#[serde(transparent)]
#[ts(type = "string")]
pub struct LegacyAppPathString(String);
impl LegacyAppPathString {
/// Preserves already-legacy app-server path text without interpreting it
/// using the current host.
///
/// This is for API-boundary values that are already strings, including
/// relative or foreign-platform spellings. Callers with a local
/// [`Path`], [`AbsolutePathBuf`], or [`PathUri`] should use the
/// corresponding typed constructor instead.
pub fn from_string(path: impl Into<String>) -> Self {
Self(path.into())
}
/// Preserves path text without interpreting it using the current host.
pub fn from_path(path: &Path) -> Self {
Self(path.to_string_lossy().into_owned())