agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

core: derive built-in permission profiles from raw policies (#25739)

Browse Source 
## Why

Permission profiles that extend a built-in profile should behave like
other TOML inheritance: parent entries provide defaults, and child keys
override matching fields before the profile is compiled.

That was not true for `:workspace`. Previously, a profile with `extends
= ":workspace"` seeded the compiled runtime
`PermissionProfile::workspace_write()` policy and then appended child
filesystem entries. A child override such as `":tmpdir" = "read"`
therefore left the inherited `":tmpdir" = "write"` entry in the final
policy. Since same-target `write` wins over `read` during runtime
resolution, the child override was ineffective.

This also needs a clear source of truth for the built-in profiles. The
protocol-level sandbox policy constructors now define the raw built-in
filesystem entries, and both `PermissionProfile` presets and
config-profile inheritance derive from those same values.

## What Changed

- Add a canonical `FileSystemSandboxPolicy::read_only()` constructor
while keeping the read-only and workspace-write raw filesystem entries
explicit and independent.
- Derive `PermissionProfile::read_only()` from
`FileSystemSandboxPolicy::read_only()`;
`PermissionProfile::workspace_write()` continues to derive from
`FileSystemSandboxPolicy::workspace_write()`.
- Build extensible `:read-only` and `:workspace` parent profiles by
projecting those canonical sandbox policies into
`PermissionProfileToml`, then merge user overrides at the TOML layer
before compilation.
- Add config parsing support for `:slash_tmp` so the built-in
`:workspace` parent can be expressed in the same TOML-shaped filesystem
table as user profiles.
- Document that `PermissionsToml::resolve_profile()` returns an
already-merged `PermissionProfileToml`, and return that profile directly
after removing the resolved-profile wrapper.
- Extend the config test for `extends = ":workspace"` to assert that
inherited `":slash_tmp" = "write"` is preserved and that a child
`":tmpdir" = "read"` entry replaces the inherited `write` entry.

## Verification

- `just test -p codex-config`
- `just test -p codex-protocol`
- `just test -p codex-core
permissions_profiles_resolve_extends_parent_first_with_child_overrides`
- `just test -p codex-core
default_permissions_profile_can_extend_builtin_workspace`
- `just test -p codex-core`
  - Result: 2596 passed, 4 failed, 1 timed out.
- The failures were existing sandbox/environment-sensitive tests
unrelated to this permissions change:

`suite::user_shell_cmd::user_shell_command_does_not_set_network_sandbox_env_var`,

`suite::user_shell_cmd::user_shell_command_history_is_persisted_and_shared_with_model`,

`suite::abort_tasks::interrupt_persists_turn_aborted_marker_in_next_request`,
    `suite::abort_tasks::interrupt_tool_records_history_entries`, and

`thread_manager::tests::start_thread_uses_all_default_environments_from_codex_home`.
This commit is contained in:
Michael Bolin
2026-06-02 10:57:35 -07:00
committed by GitHub
Unverified
parent ebb7980369
commit 593df8773d
7 changed files with 195 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/config/src/permissions_toml.rs
+9 -16
View File
@@ -30,11 +30,18 @@ impl PermissionsToml {
self.entries.is_empty()
}
/// Resolve `profile_name` and all of its `extends` ancestors into one TOML
/// profile.
///
/// Parent profiles are merged before their children, so child keys override
/// matching parent keys before callers compile the profile into runtime
/// permissions. The returned profile keeps the selected profile's
/// declaration metadata, such as `description` and `extends`.
pub fn resolve_profile<F>(
&self,
profile_name: &str,
mut parent_profile: F,
) -> Result<ResolvedPermissionProfileToml, PermissionProfileResolutionError>
) -> Result<PermissionProfileToml, PermissionProfileResolutionError>
where
F: FnMut(&str) -> Option<PermissionProfileToml>,
{
@@ -96,10 +103,7 @@ impl PermissionsToml {
.into_iter()
.rev()
.try_fold(profile, merge_permission_profiles)?;
return Ok(ResolvedPermissionProfileToml {
profile,
inherited_profile_names: profile_names.into_iter().skip(1).collect(),
});
return Ok(profile);
}
}
}
@@ -114,17 +118,6 @@ pub struct PermissionProfileToml {
pub network: Option<NetworkToml>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ResolvedPermissionProfileToml {
pub profile: PermissionProfileToml,
/// Names of profiles inherited while resolving `profile`, ordered from the
/// selected profile's direct parent to the farthest ancestor.
///
/// Callers use this to preserve which built-in baseline contributed the
/// resolved permissions after the parent profiles have been merged away.
pub inherited_profile_names: Vec<String>,
}
#[derive(Debug, Clone, PartialEq, Eq, Error)]
pub enum PermissionProfileResolutionError {
#[error("default_permissions refers to undefined profile `{profile_name}`")]
codex-rs/core/src/config/config_tests.rs
+43 -1
View File
@@ -2357,7 +2357,13 @@ async fn default_permissions_profile_can_extend_builtin_workspace() -> std::io::
description: None,
extends: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()),
workspace_roots: None,
filesystem: None,
filesystem: Some(FilesystemPermissionsToml {
glob_scan_max_depth: None,
entries: BTreeMap::from([(
":tmpdir".to_string(),
FilesystemPermissionToml::Access(FileSystemAccessMode::Read),
)]),
}),
network: Some(NetworkToml {
enabled: Some(true),
..Default::default()
@@ -2384,6 +2390,42 @@ async fn default_permissions_profile_can_extend_builtin_workspace() -> std::io::
!policy.can_write_path_with_cwd(&cwd.path().join(".git"), cwd.path()),
"expected profile extending :workspace to keep metadata carveouts, policy: {policy:?}"
);
assert!(
policy.entries.iter().any(|entry| matches!(
entry,
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::SlashTmp,
},
access: FileSystemAccessMode::Write,
}
)),
"expected profile extending :workspace to keep inherited :slash_tmp writes, policy: {policy:?}"
);
assert!(
policy.entries.iter().any(|entry| matches!(
entry,
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Tmpdir,
},
access: FileSystemAccessMode::Read,
}
)),
"expected child :tmpdir read entry to replace the inherited write entry, policy: {policy:?}"
);
assert!(
!policy.entries.iter().any(|entry| matches!(
entry,
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Tmpdir,
},
access: FileSystemAccessMode::Write,
}
)),
"expected inherited :tmpdir write entry to be removed, policy: {policy:?}"
);
assert_eq!(
config.permissions.network_sandbox_policy(),
NetworkSandboxPolicy::Enabled
codex-rs/core/src/config/permissions.rs
+125 -35
View File
@@ -1,4 +1,5 @@
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::io;
use std::path::Component;
use std::path::Path;
@@ -13,7 +14,6 @@ use codex_config::permissions_toml::NetworkUnixSocketPermissionToml;
use codex_config::permissions_toml::NetworkUnixSocketPermissionsToml;
use codex_config::permissions_toml::PermissionProfileToml;
use codex_config::permissions_toml::PermissionsToml;
use codex_config::permissions_toml::ResolvedPermissionProfileToml;
use codex_config::permissions_toml::WorkspaceRootsToml;
use codex_config::types::SandboxWorkspaceWrite;
use codex_features::NetworkProxyConfigToml;
@@ -194,27 +194,132 @@ pub(crate) fn apply_network_proxy_feature_config(
pub(crate) fn resolve_permission_profile(
permissions: &PermissionsToml,
profile_name: &str,
) -> io::Result<ResolvedPermissionProfileToml> {
) -> io::Result<PermissionProfileToml> {
permissions
.resolve_profile(profile_name, extensible_builtin_parent_profile_marker)
.resolve_profile(profile_name, extensible_builtin_parent_profile)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err.to_string()))
}
/// Built-in parents provide their runtime permissions below. Resolution only
/// needs an empty profile marker so inheritance can terminate while preserving
/// the built-in parent id in `inherited_profile_names`.
fn extensible_builtin_parent_profile_marker(profile_name: &str) -> Option<PermissionProfileToml> {
matches!(
profile_name,
BUILT_IN_READ_ONLY_PROFILE | BUILT_IN_WORKSPACE_PROFILE
)
.then_some(PermissionProfileToml {
fn extensible_builtin_parent_profile(profile_name: &str) -> Option<PermissionProfileToml> {
let file_system = match profile_name {
BUILT_IN_READ_ONLY_PROFILE => FileSystemSandboxPolicy::read_only(),
BUILT_IN_WORKSPACE_PROFILE => FileSystemSandboxPolicy::workspace_write(
&[],
/*exclude_tmpdir_env_var*/ false,
/*exclude_slash_tmp*/ false,
),
_ => return None,
};
Some(permission_profile_toml_from_file_system_policy(file_system))
}
fn permission_profile_toml_from_file_system_policy(
file_system: FileSystemSandboxPolicy,
) -> PermissionProfileToml {
let mut filesystem = FilesystemPermissionsToml {
glob_scan_max_depth: file_system.glob_scan_max_depth,
entries: BTreeMap::new(),
};
for entry in file_system.entries {
insert_filesystem_permission_toml(&mut filesystem.entries, entry);
}
PermissionProfileToml {
description: None,
extends: None,
workspace_roots: None,
filesystem: None,
filesystem: Some(filesystem),
network: None,
})
}
}
fn insert_filesystem_permission_toml(
entries: &mut BTreeMap<String, FilesystemPermissionToml>,
entry: FileSystemSandboxEntry,
) {
match entry.path {
FileSystemPath::Path { path } => {
entries.insert(
path.into_path_buf().to_string_lossy().into_owned(),
FilesystemPermissionToml::Access(entry.access),
);
}
FileSystemPath::GlobPattern { pattern } => {
entries.insert(pattern, FilesystemPermissionToml::Access(entry.access));
}
FileSystemPath::Special { value } => {
insert_special_filesystem_permission_toml(entries, value, entry.access);
}
}
}
fn insert_special_filesystem_permission_toml(
entries: &mut BTreeMap<String, FilesystemPermissionToml>,
value: FileSystemSpecialPath,
access: FileSystemAccessMode,
) {
match value {
FileSystemSpecialPath::Root => {
entries.insert(
":root".to_string(),
FilesystemPermissionToml::Access(access),
);
}
FileSystemSpecialPath::Minimal => {
entries.insert(
":minimal".to_string(),
FilesystemPermissionToml::Access(access),
);
}
FileSystemSpecialPath::ProjectRoots { subpath } => {
insert_scoped_filesystem_permission_toml(
entries,
":workspace_roots".to_string(),
subpath.unwrap_or_else(|| PathBuf::from(".")),
access,
);
}
FileSystemSpecialPath::Tmpdir => {
entries.insert(
":tmpdir".to_string(),
FilesystemPermissionToml::Access(access),
);
}
FileSystemSpecialPath::SlashTmp => {
entries.insert(
":slash_tmp".to_string(),
FilesystemPermissionToml::Access(access),
);
}
FileSystemSpecialPath::Unknown { path, subpath } => {
if let Some(subpath) = subpath {
insert_scoped_filesystem_permission_toml(entries, path, subpath, access);
} else {
entries.insert(path, FilesystemPermissionToml::Access(access));
}
}
};
}
fn insert_scoped_filesystem_permission_toml(
entries: &mut BTreeMap<String, FilesystemPermissionToml>,
path: String,
subpath: PathBuf,
access: FileSystemAccessMode,
) {
let permission = entries
.entry(path)
.or_insert_with(|| FilesystemPermissionToml::Scoped(BTreeMap::new()));
match permission {
FilesystemPermissionToml::Scoped(scoped_entries) => {
scoped_entries.insert(subpath.to_string_lossy().into_owned(), access);
}
FilesystemPermissionToml::Access(_) => {
*permission = FilesystemPermissionToml::Scoped(BTreeMap::from([(
subpath.to_string_lossy().into_owned(),
access,
)]));
}
}
}
pub(crate) fn network_proxy_config_for_profile_selection(
@@ -232,7 +337,7 @@ pub(crate) fn network_proxy_config_for_profile_selection(
"default_permissions requires a `[permissions]` table",
)
})?;
let profile = resolve_permission_profile(permissions, profile_name)?.profile;
let profile = resolve_permission_profile(permissions, profile_name)?;
Ok(network_proxy_config_from_profile_network(
profile.network.as_ref(),
))
@@ -244,25 +349,9 @@ pub(crate) fn compile_permission_profile(
policy_cwd: &Path,
startup_warnings: &mut Vec<String>,
) -> io::Result<(FileSystemSandboxPolicy, NetworkSandboxPolicy)> {
let ResolvedPermissionProfileToml {
profile,
inherited_profile_names,
} = resolve_permission_profile(permissions, profile_name)?;
let base_permissions = inherited_profile_names.iter().find_map(|name| {
match name.as_str() {
BUILT_IN_READ_ONLY_PROFILE => Some(PermissionProfile::read_only()),
BUILT_IN_WORKSPACE_PROFILE => Some(PermissionProfile::workspace_write()),
_ => None,
}
.map(|profile| profile.to_runtime_permissions())
});
let (mut file_system_sandbox_policy, base_network_sandbox_policy) = base_permissions
.unwrap_or_else(|| {
(
FileSystemSandboxPolicy::restricted(Vec::new()),
NetworkSandboxPolicy::Restricted,
)
});
let profile = resolve_permission_profile(permissions, profile_name)?;
let mut file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(Vec::new());
let base_network_sandbox_policy = NetworkSandboxPolicy::Restricted;
if let Some(filesystem) = profile.filesystem.as_ref() {
if filesystem.is_empty() && file_system_sandbox_policy.entries.is_empty() {
push_warning(
@@ -358,7 +447,7 @@ pub(crate) fn compile_permission_profile_workspace_roots(
})?;
let profile = resolve_permission_profile(permissions, profile_name)?;
Ok(compile_workspace_roots(
profile.profile.workspace_roots.as_ref(),
profile.workspace_roots.as_ref(),
policy_cwd,
))
}
@@ -693,6 +782,7 @@ fn parse_special_path(path: &str) -> Option<FileSystemSpecialPath> {
":minimal" => Some(FileSystemSpecialPath::Minimal),
":workspace_roots" => Some(FileSystemSpecialPath::project_roots(/*subpath*/ None)),
":tmpdir" => Some(FileSystemSpecialPath::Tmpdir),
":slash_tmp" => Some(FileSystemSpecialPath::SlashTmp),
_ if path.starts_with(':') => {
Some(FileSystemSpecialPath::unknown(path, /*subpath*/ None))
}
codex-rs/core/src/config/permissions_tests.rs
+1 -2
View File
@@ -326,8 +326,7 @@ allow_local_binding = true
)
.expect("expected profile should deserialize");
assert_eq!(resolved.profile, expected_profile);
assert_eq!(resolved.inherited_profile_names, vec!["base".to_string()]);
assert_eq!(resolved, expected_profile);
}
#[test]
codex-rs/core/src/network_proxy_loader.rs
+1 -1
View File
@@ -210,7 +210,7 @@ fn selected_network_from_tables(parsed: NetworkTablesToml) -> Result<Option<Netw
.context("default_permissions requires a `[permissions]` table for network settings")?;
let profile = resolve_permission_profile(&permissions, &default_permissions)
.map_err(anyhow::Error::from)?;
Ok(profile.profile.network)
Ok(profile.network)
}
#[cfg(test)]
codex-rs/protocol/src/models.rs
+2 -9
View File
@@ -374,16 +374,9 @@ impl Default for PermissionProfile {
impl PermissionProfile {
/// Managed read-only filesystem access with restricted network access.
pub fn read_only() -> Self {
let file_system = FileSystemSandboxPolicy::read_only();
Self::Managed {
file_system: ManagedFileSystemPermissions::Restricted {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
}],
glob_scan_max_depth: None,
},
file_system: ManagedFileSystemPermissions::from_sandbox_policy(&file_system),
network: NetworkSandboxPolicy::Restricted,
}
}
codex-rs/protocol/src/permissions.rs
+14 -10
View File
@@ -358,22 +358,26 @@ pub fn project_roots_glob_pattern(subpath: &Path) -> String {
format!("{PROJECT_ROOTS_GLOB_PATTERN_PREFIX}{}", subpath.display())
}
fn read_only_file_system_entries() -> Vec<FileSystemSandboxEntry> {
vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
}]
}
impl Default for FileSystemSandboxPolicy {
fn default() -> Self {
Self {
kind: FileSystemSandboxKind::Restricted,
glob_scan_max_depth: None,
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
}],
}
Self::read_only()
}
}
impl FileSystemSandboxPolicy {
pub fn read_only() -> Self {
Self::restricted(read_only_file_system_entries())
}
pub fn unrestricted() -> Self {
Self {
kind: FileSystemSandboxKind::Unrestricted,