diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index ebf4862bc..9fdd75672 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -574,6 +574,156 @@ fn permissions_profiles_reject_nested_entries_for_non_project_roots() -> std::io Ok(()) } +fn load_workspace_permission_profile(profile: PermissionProfileToml) -> std::io::Result { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?; + + Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some("workspace".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([("workspace".to_string(), profile)]), + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + ..Default::default() + }, + codex_home.path().to_path_buf(), + ) +} + +#[test] +fn permissions_profiles_allow_unknown_special_paths() -> std::io::Result<()> { + let config = load_workspace_permission_profile(PermissionProfileToml { + filesystem: Some(FilesystemPermissionsToml { + entries: BTreeMap::from([( + ":future_special_path".to_string(), + FilesystemPermissionToml::Access(FileSystemAccessMode::Read), + )]), + }), + network: None, + })?; + + assert_eq!( + config.permissions.file_system_sandbox_policy, + FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::unknown(":future_special_path", None), + }, + access: FileSystemAccessMode::Read, + }]), + ); + assert_eq!( + config.permissions.sandbox_policy.get(), + &SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + } + ); + assert!( + config.startup_warnings.iter().any(|warning| warning.contains( + "Configured filesystem path `:future_special_path` is not recognized by this version of Codex and will be ignored." + )), + "{:?}", + config.startup_warnings + ); + Ok(()) +} + +#[test] +fn permissions_profiles_allow_unknown_special_paths_with_nested_entries() -> std::io::Result<()> { + let config = load_workspace_permission_profile(PermissionProfileToml { + filesystem: Some(FilesystemPermissionsToml { + entries: BTreeMap::from([( + ":future_special_path".to_string(), + FilesystemPermissionToml::Scoped(BTreeMap::from([( + "docs".to_string(), + FileSystemAccessMode::Read, + )])), + )]), + }), + network: None, + })?; + + assert_eq!( + config.permissions.file_system_sandbox_policy, + FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::unknown(":future_special_path", Some("docs".into())), + }, + access: FileSystemAccessMode::Read, + }]), + ); + assert!( + config.startup_warnings.iter().any(|warning| warning.contains( + "Configured filesystem path `:future_special_path` with nested entry `docs` is not recognized by this version of Codex and will be ignored." + )), + "{:?}", + config.startup_warnings + ); + Ok(()) +} + +#[test] +fn permissions_profiles_allow_missing_filesystem_with_warning() -> std::io::Result<()> { + let config = load_workspace_permission_profile(PermissionProfileToml { + filesystem: None, + network: None, + })?; + + assert_eq!( + config.permissions.file_system_sandbox_policy, + FileSystemSandboxPolicy::restricted(Vec::new()) + ); + assert_eq!( + config.permissions.sandbox_policy.get(), + &SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + } + ); + assert!( + config.startup_warnings.iter().any(|warning| warning.contains( + "Permissions profile `workspace` does not define any recognized filesystem entries for this version of Codex." + )), + "{:?}", + config.startup_warnings + ); + Ok(()) +} + +#[test] +fn permissions_profiles_allow_empty_filesystem_with_warning() -> std::io::Result<()> { + let config = load_workspace_permission_profile(PermissionProfileToml { + filesystem: Some(FilesystemPermissionsToml { + entries: BTreeMap::new(), + }), + network: None, + })?; + + assert_eq!( + config.permissions.file_system_sandbox_policy, + FileSystemSandboxPolicy::restricted(Vec::new()) + ); + assert!( + config.startup_warnings.iter().any(|warning| warning.contains( + "Permissions profile `workspace` does not define any recognized filesystem entries for this version of Codex." + )), + "{:?}", + config.startup_warnings + ); + Ok(()) +} + #[test] fn permissions_profiles_reject_project_root_parent_traversal() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 91c466550..0c9a3a282 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1971,7 +1971,11 @@ impl Config { let configured_network_proxy_config = network_proxy_config_from_profile_network(profile.network.as_ref()); let (mut file_system_sandbox_policy, network_sandbox_policy) = - compile_permission_profile(permissions, default_permissions)?; + compile_permission_profile( + permissions, + default_permissions, + &mut startup_warnings, + )?; let mut sandbox_policy = file_system_sandbox_policy .to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd)?; if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) { diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index b96039cec..b931c0f41 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::BTreeMap; use std::io; use std::path::Component; @@ -158,30 +159,27 @@ pub(crate) fn resolve_permission_profile<'a>( pub(crate) fn compile_permission_profile( permissions: &PermissionsToml, profile_name: &str, + startup_warnings: &mut Vec, ) -> io::Result<(FileSystemSandboxPolicy, NetworkSandboxPolicy)> { let profile = resolve_permission_profile(permissions, profile_name)?; - let filesystem = profile.filesystem.as_ref().ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - format!( - "permissions profile `{profile_name}` must define a `[permissions.{profile_name}.filesystem]` table" - ), - ) - })?; - - if filesystem.is_empty() { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!( - "permissions profile `{profile_name}` must define at least one filesystem entry" - ), - )); - } - let mut entries = Vec::new(); - for (path, permission) in &filesystem.entries { - compile_filesystem_permission(path, permission, &mut entries)?; + if let Some(filesystem) = profile.filesystem.as_ref() { + if filesystem.is_empty() { + push_warning( + startup_warnings, + missing_filesystem_entries_warning(profile_name), + ); + } else { + for (path, permission) in &filesystem.entries { + compile_filesystem_permission(path, permission, &mut entries, startup_warnings)?; + } + } + } else { + push_warning( + startup_warnings, + missing_filesystem_entries_warning(profile_name), + ); } let network_sandbox_policy = compile_network_sandbox_policy(profile.network.as_ref()); @@ -207,16 +205,17 @@ fn compile_filesystem_permission( path: &str, permission: &FilesystemPermissionToml, entries: &mut Vec, + startup_warnings: &mut Vec, ) -> io::Result<()> { match permission { FilesystemPermissionToml::Access(access) => entries.push(FileSystemSandboxEntry { - path: compile_filesystem_path(path)?, + path: compile_filesystem_path(path, startup_warnings)?, access: *access, }), FilesystemPermissionToml::Scoped(scoped_entries) => { for (subpath, access) in scoped_entries { entries.push(FileSystemSandboxEntry { - path: compile_scoped_filesystem_path(path, subpath)?, + path: compile_scoped_filesystem_path(path, subpath, startup_warnings)?, access: *access, }); } @@ -225,8 +224,12 @@ fn compile_filesystem_permission( Ok(()) } -fn compile_filesystem_path(path: &str) -> io::Result { - if let Some(special) = parse_special_path(path)? { +fn compile_filesystem_path( + path: &str, + startup_warnings: &mut Vec, +) -> io::Result { + if let Some(special) = parse_special_path(path) { + maybe_push_unknown_special_path_warning(&special, startup_warnings); return Ok(FileSystemPath::Special { value: special }); } @@ -234,21 +237,33 @@ fn compile_filesystem_path(path: &str) -> io::Result { Ok(FileSystemPath::Path { path }) } -fn compile_scoped_filesystem_path(path: &str, subpath: &str) -> io::Result { +fn compile_scoped_filesystem_path( + path: &str, + subpath: &str, + startup_warnings: &mut Vec, +) -> io::Result { if subpath == "." { - return compile_filesystem_path(path); + return compile_filesystem_path(path, startup_warnings); } - if let Some(special) = parse_special_path(path)? { - if !matches!(special, FileSystemSpecialPath::ProjectRoots { .. }) { - return Err(io::Error::new( + if let Some(special) = parse_special_path(path) { + let subpath = parse_relative_subpath(subpath)?; + let special = match special { + FileSystemSpecialPath::ProjectRoots { .. } => Ok(FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(Some(subpath)), + }), + FileSystemSpecialPath::Unknown { path, .. } => Ok(FileSystemPath::Special { + value: FileSystemSpecialPath::unknown(path, Some(subpath)), + }), + _ => Err(io::Error::new( io::ErrorKind::InvalidInput, format!("filesystem path `{path}` does not support nested entries"), - )); + )), + }?; + if let FileSystemPath::Special { value } = &special { + maybe_push_unknown_special_path_warning(value, startup_warnings); } - return Ok(FileSystemPath::Special { - value: FileSystemSpecialPath::project_roots(Some(parse_relative_subpath(subpath)?)), - }); + return Ok(special); } let subpath = parse_relative_subpath(subpath)?; @@ -257,33 +272,90 @@ fn compile_scoped_filesystem_path(path: &str, subpath: &str) -> io::Result io::Result> { - let special = match path { +// WARNING: keep this parser forward-compatible. +// Adding a new `:special_path` must not make older Codex versions reject the +// config. Unknown values intentionally round-trip through +// `FileSystemSpecialPath::Unknown` so they can be surfaced as warnings and +// ignored, rather than aborting config load. +fn parse_special_path(path: &str) -> Option { + match path { ":root" => Some(FileSystemSpecialPath::Root), ":minimal" => Some(FileSystemSpecialPath::Minimal), ":project_roots" => Some(FileSystemSpecialPath::project_roots(None)), ":tmpdir" => Some(FileSystemSpecialPath::Tmpdir), - _ if path.starts_with(':') => { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("unknown filesystem special path `{path}`"), - )); - } + _ if path.starts_with(':') => Some(FileSystemSpecialPath::unknown(path, None)), _ => None, - }; - - Ok(special) + } } fn parse_absolute_path(path: &str) -> io::Result { - let path_ref = Path::new(path); - if !path_ref.is_absolute() && path != "~" && !path.starts_with("~/") { + parse_absolute_path_for_platform(path, cfg!(windows)) +} + +fn parse_absolute_path_for_platform(path: &str, is_windows: bool) -> io::Result { + let path_ref = normalize_absolute_path_for_platform(path, is_windows); + if !is_absolute_path_for_platform(path, path_ref.as_ref(), is_windows) + && path != "~" + && !path.starts_with("~/") + { return Err(io::Error::new( io::ErrorKind::InvalidInput, format!("filesystem path `{path}` must be absolute, use `~/...`, or start with `:`"), )); } - AbsolutePathBuf::from_absolute_path(path_ref) + AbsolutePathBuf::from_absolute_path(path_ref.as_ref()) +} + +fn is_absolute_path_for_platform(path: &str, normalized_path: &Path, is_windows: bool) -> bool { + if is_windows { + is_windows_absolute_path(path) + || is_windows_absolute_path(&normalized_path.to_string_lossy()) + } else { + normalized_path.is_absolute() + } +} + +fn normalize_absolute_path_for_platform(path: &str, is_windows: bool) -> Cow<'_, Path> { + if !is_windows { + return Cow::Borrowed(Path::new(path)); + } + + match normalize_windows_device_path(path) { + Some(normalized) => Cow::Owned(PathBuf::from(normalized)), + None => Cow::Borrowed(Path::new(path)), + } +} + +fn normalize_windows_device_path(path: &str) -> Option { + if let Some(unc) = path.strip_prefix(r"\\?\UNC\") { + return Some(format!(r"\\{unc}")); + } + if let Some(unc) = path.strip_prefix(r"\\.\UNC\") { + return Some(format!(r"\\{unc}")); + } + if let Some(path) = path.strip_prefix(r"\\?\") + && is_windows_drive_absolute_path(path) + { + return Some(path.to_string()); + } + if let Some(path) = path.strip_prefix(r"\\.\") + && is_windows_drive_absolute_path(path) + { + return Some(path.to_string()); + } + None +} + +fn is_windows_absolute_path(path: &str) -> bool { + is_windows_drive_absolute_path(path) || path.starts_with(r"\\") +} + +fn is_windows_drive_absolute_path(path: &str) -> bool { + let bytes = path.as_bytes(); + bytes.len() >= 3 + && bytes[0].is_ascii_alphabetic() + && bytes[1] == b':' + && matches!(bytes[2], b'\\' | b'/') } fn parse_relative_subpath(subpath: &str) -> io::Result { @@ -304,3 +376,48 @@ fn parse_relative_subpath(subpath: &str) -> io::Result { ), )) } + +fn push_warning(startup_warnings: &mut Vec, message: String) { + tracing::warn!("{message}"); + startup_warnings.push(message); +} + +fn missing_filesystem_entries_warning(profile_name: &str) -> String { + format!( + "Permissions profile `{profile_name}` does not define any recognized filesystem entries for this version of Codex. Filesystem access will remain restricted. Upgrade Codex if this profile expects filesystem permissions." + ) +} + +fn maybe_push_unknown_special_path_warning( + special: &FileSystemSpecialPath, + startup_warnings: &mut Vec, +) { + let FileSystemSpecialPath::Unknown { path, subpath } = special else { + return; + }; + push_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() + ), + 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." + ), + }, + ); +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn normalize_absolute_path_for_platform_simplifies_windows_verbatim_paths() { + let parsed = + normalize_absolute_path_for_platform(r"\\?\D:\c\x\worktrees\2508\swift-base", true); + assert_eq!(parsed, PathBuf::from(r"D:\c\x\worktrees\2508\swift-base")); + } +} diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index abe9bc581..9624f36cd 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -67,12 +67,33 @@ pub enum FileSystemSpecialPath { }, Tmpdir, SlashTmp, + /// WARNING: `:special_path` tokens are part of config compatibility. + /// Do not make older runtimes reject newly introduced tokens. + /// New parser support should be additive, while unknown values must stay + /// representable so config from a newer Codex degrades to warn-and-ignore + /// instead of failing to load. Codex 0.112.0 rejected unknown values here, + /// which broke forward compatibility for newer config. + /// Preserves future special-path tokens so older runtimes can ignore them + /// without rejecting config authored by a newer release. + Unknown { + path: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + subpath: Option, + }, } impl FileSystemSpecialPath { pub fn project_roots(subpath: Option) -> Self { Self::ProjectRoots { subpath } } + + pub fn unknown(path: impl Into, subpath: Option) -> Self { + Self::Unknown { + path: path.into(), + subpath, + } + } } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] @@ -437,6 +458,7 @@ impl FileSystemSandboxPolicy { readable_roots.push(path); } } + FileSystemSpecialPath::Unknown { .. } => {} }, } } @@ -634,7 +656,9 @@ fn resolve_file_system_special_path( cwd: Option<&AbsolutePathBuf>, ) -> Option { match value { - FileSystemSpecialPath::Root | FileSystemSpecialPath::Minimal => None, + FileSystemSpecialPath::Root + | FileSystemSpecialPath::Minimal + | FileSystemSpecialPath::Unknown { .. } => None, FileSystemSpecialPath::CurrentWorkingDirectory => { let cwd = cwd?; Some(cwd.clone()) @@ -779,3 +803,36 @@ fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option std::io::Result<()> { + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::unknown(":future_special_path", None), + }, + access: FileSystemAccessMode::Write, + }]); + + let sandbox_policy = policy.to_legacy_sandbox_policy( + NetworkSandboxPolicy::Restricted, + Path::new("/tmp/workspace"), + )?; + + assert_eq!( + sandbox_policy, + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + } + ); + Ok(()) + } +}