mirror of
https://github.com/openai/codex.git
synced 2026-05-21 19:45:26 +00:00
permissions: derive config defaults as profiles (#19772)
## Why This continues the permissions migration by making legacy config default resolution produce the canonical `PermissionProfile` first. The legacy `SandboxPolicy` projection should stay available at compatibility boundaries, but config loading should not create a legacy policy just to immediately convert it back into a profile. Specifically, when `default_permissions` is not specified in `config.toml`, instead of creating a `SandboxPolicy` in `codex-rs/core/src/config/mod.rs` and then trying to derive a `PermissionProfile` from it, we use `derive_permission_profile()` to create a more faithful `PermissionProfile` using the values of `ConfigToml` directly. This also keeps the existing behavior of `sandbox_workspace_write` and extra writable roots after #19841 replaced `:cwd` with `:project_roots`. Legacy workspace-write defaults are represented as symbolic `:project_roots` write access plus symbolic project-root metadata carveouts. Extra absolute writable roots are still added directly and continue to get concrete metadata protections for paths that exist under those roots. The platform sandboxes differ when a symbolic project-root subpath does not exist yet. * **Seatbelt** can encode literal/subpath exclusions directly, so macOS emits project-root metadata subpath policies even if `.git`, `.agents`, or `.codex` do not exist. * **bwrap** has to materialize bind-mount targets. Binding `/dev/null` to a missing `.git` can create a host-visible placeholder that changes Git repo discovery. Binding missing `.agents` would not affect Git discovery, but it would still create a host-visible project metadata placeholder from an automatic compatibility carveout. Linux therefore skips only missing automatic `.git` and `.agents` read-only metadata masks; missing `.codex` remains protected so first-time project config creation goes through the protected-path approval flow. User-authored `read` and `none` subpath rules keep normal bwrap behavior, and `none` can still mask the first missing component to prevent creation under writable roots. ## What Changed - Adds profile-native helpers for legacy workspace-write semantics, including `PermissionProfile::workspace_write_with()`, `FileSystemSandboxPolicy::workspace_write()`, and `FileSystemSandboxPolicy::with_additional_legacy_workspace_writable_roots()`. - Makes `FileSystemSandboxPolicy::workspace_write()` the single legacy workspace-write constructor so both `from_legacy_sandbox_policy()` and `From<&SandboxPolicy>` include the project-root metadata carveouts. - Removes the no-carveout `legacy_workspace_write_base_policy()` path and the `prune_read_entries_under_writable_roots()` cleanup that was only needed by that split construction. - Adds `ConfigToml::derive_permission_profile()` for legacy sandbox-mode fallback resolution; named `default_permissions` profiles continue through the permissions profile pipeline instead of being reconstructed from `sandbox_mode`. - Updates `Config::load()` to start from the derived profile, validate that it still has a legacy compatibility projection, and apply additional writable roots directly to managed workspace-write filesystem policies. - Updates Linux bwrap argument construction so missing automatic `.git`/`.agents` symbolic project-root read-only carveouts are skipped before emitting bind args; missing `.codex`, user-authored `read`/`none` subpath rules, and existing missing writable-root behavior are preserved. - Adds coverage that legacy workspace-write config produces symbolic project-root metadata carveouts, extra legacy workspace writable roots still protect existing metadata paths such as `.git`, and bwrap skips missing `.git`/`.agents` project-root carveouts while preserving missing `.codex` and user-authored missing subpath rules. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19772). * #19776 * #19775 * #19774 * #19773 * __->__ #19772
This commit is contained in:
@@ -133,6 +133,34 @@ fn http_mcp(url: &str) -> McpServerConfig {
|
||||
}
|
||||
}
|
||||
|
||||
async fn derive_legacy_sandbox_policy_for_test(
|
||||
cfg: &ConfigToml,
|
||||
sandbox_mode_override: Option<SandboxMode>,
|
||||
profile_sandbox_mode: Option<SandboxMode>,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
active_project: Option<&ProjectConfig>,
|
||||
permission_profile_constraint: Option<&Constrained<PermissionProfile>>,
|
||||
) -> SandboxPolicy {
|
||||
let permission_profile = cfg
|
||||
.derive_permission_profile(
|
||||
sandbox_mode_override,
|
||||
profile_sandbox_mode,
|
||||
windows_sandbox_level,
|
||||
active_project,
|
||||
permission_profile_constraint,
|
||||
)
|
||||
.await;
|
||||
permission_profile
|
||||
.to_legacy_sandbox_policy(Path::new("/"))
|
||||
.unwrap_or_else(|err| {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"derived permission profile cannot be represented as a legacy sandbox policy; falling back to read-only"
|
||||
);
|
||||
SandboxPolicy::new_read_only_policy()
|
||||
})
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_config_normalizes_relative_cwd_override() -> std::io::Result<()> {
|
||||
let expected_cwd = AbsolutePathBuf::relative_to_current_dir("nested")?;
|
||||
@@ -1630,15 +1658,15 @@ network_access = false # This should be ignored.
|
||||
let sandbox_full_access_cfg = toml::from_str::<ConfigToml>(sandbox_full_access)
|
||||
.expect("TOML deserialization should succeed");
|
||||
let sandbox_mode_override = None;
|
||||
let resolution = sandbox_full_access_cfg
|
||||
.derive_sandbox_policy(
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&sandbox_full_access_cfg,
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
assert_eq!(resolution, SandboxPolicy::DangerFullAccess);
|
||||
|
||||
let sandbox_read_only = r#"
|
||||
@@ -1651,15 +1679,15 @@ network_access = true # This should be ignored.
|
||||
let sandbox_read_only_cfg = toml::from_str::<ConfigToml>(sandbox_read_only)
|
||||
.expect("TOML deserialization should succeed");
|
||||
let sandbox_mode_override = None;
|
||||
let resolution = sandbox_read_only_cfg
|
||||
.derive_sandbox_policy(
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&sandbox_read_only_cfg,
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
assert_eq!(resolution, SandboxPolicy::new_read_only_policy());
|
||||
|
||||
let writable_root = test_absolute_path("/my/workspace");
|
||||
@@ -1683,15 +1711,15 @@ trust_level = "trusted"
|
||||
let sandbox_workspace_write_cfg = toml::from_str::<ConfigToml>(&sandbox_workspace_write)
|
||||
.expect("TOML deserialization should succeed");
|
||||
let sandbox_mode_override = None;
|
||||
let resolution = sandbox_workspace_write_cfg
|
||||
.derive_sandbox_policy(
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&sandbox_workspace_write_cfg,
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(resolution, SandboxPolicy::new_read_only_policy());
|
||||
} else {
|
||||
@@ -1723,15 +1751,15 @@ exclude_slash_tmp = true
|
||||
let sandbox_workspace_write_cfg = toml::from_str::<ConfigToml>(&sandbox_workspace_write)
|
||||
.expect("TOML deserialization should succeed");
|
||||
let sandbox_mode_override = None;
|
||||
let resolution = sandbox_workspace_write_cfg
|
||||
.derive_sandbox_policy(
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&sandbox_workspace_write_cfg,
|
||||
sandbox_mode_override,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*active_project*/ None,
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(resolution, SandboxPolicy::new_read_only_policy());
|
||||
} else {
|
||||
@@ -1748,7 +1776,7 @@ exclude_slash_tmp = true
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn legacy_sandbox_mode_config_builds_split_policies_without_drift() -> std::io::Result<()> {
|
||||
async fn legacy_sandbox_mode_builds_profiles_with_compatible_projection() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
let extra_root = test_absolute_path("/tmp/legacy-extra-root");
|
||||
@@ -1793,26 +1821,91 @@ exclude_slash_tmp = true
|
||||
)
|
||||
.await?;
|
||||
|
||||
let sandbox_policy = &config.legacy_sandbox_policy();
|
||||
let sandbox_policy = config.legacy_sandbox_policy();
|
||||
let file_system_policy = config.permissions.file_system_sandbox_policy();
|
||||
let network_policy = config.permissions.network_sandbox_policy();
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(sandbox_policy, cwd.path()),
|
||||
"case `{name}` should preserve filesystem semantics from legacy config"
|
||||
);
|
||||
assert_eq!(
|
||||
config.permissions.network_sandbox_policy(),
|
||||
NetworkSandboxPolicy::from(sandbox_policy),
|
||||
network_policy,
|
||||
NetworkSandboxPolicy::from(&sandbox_policy),
|
||||
"case `{name}` should preserve network semantics from legacy config"
|
||||
);
|
||||
assert_eq!(
|
||||
config
|
||||
.permissions
|
||||
.file_system_sandbox_policy()
|
||||
.to_legacy_sandbox_policy(config.permissions.network_sandbox_policy(), cwd.path())
|
||||
file_system_policy
|
||||
.to_legacy_sandbox_policy(network_policy, cwd.path())
|
||||
.unwrap_or_else(|err| panic!("case `{name}` should round-trip: {err}")),
|
||||
sandbox_policy.clone(),
|
||||
"case `{name}` should round-trip through split policies without drift"
|
||||
sandbox_policy,
|
||||
"case `{name}` should preserve its legacy compatibility projection"
|
||||
);
|
||||
|
||||
match name.as_str() {
|
||||
"danger-full-access" | "read-only" => {
|
||||
assert_eq!(
|
||||
file_system_policy,
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(
|
||||
&sandbox_policy,
|
||||
cwd.path()
|
||||
),
|
||||
"case `{name}` should match the legacy filesystem projection exactly"
|
||||
);
|
||||
}
|
||||
"workspace-write" => {
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(
|
||||
sandbox_policy,
|
||||
SandboxPolicy::new_read_only_policy(),
|
||||
"legacy workspace-write should keep the existing Windows downgrade when \
|
||||
the experimental Windows sandbox is disabled"
|
||||
);
|
||||
assert_eq!(
|
||||
file_system_policy,
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(
|
||||
&sandbox_policy,
|
||||
cwd.path()
|
||||
),
|
||||
"downgraded workspace-write should match the legacy read-only projection"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
assert!(
|
||||
file_system_policy
|
||||
.entries
|
||||
.contains(&FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
})
|
||||
);
|
||||
assert!(
|
||||
file_system_policy
|
||||
.entries
|
||||
.contains(&FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: extra_root.clone(),
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
})
|
||||
);
|
||||
for subpath in [".git", ".agents", ".codex"] {
|
||||
assert!(
|
||||
file_system_policy
|
||||
.entries
|
||||
.contains(&FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(Some(
|
||||
subpath.into()
|
||||
)),
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
}),
|
||||
"case `{name}` should preserve `{subpath}` as a symbolic project-root \
|
||||
metadata carveout"
|
||||
);
|
||||
}
|
||||
}
|
||||
_ => unreachable!("unexpected test case `{name}`"),
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
@@ -6310,15 +6403,15 @@ trust_level = "untrusted"
|
||||
trust_level: Some(TrustLevel::Untrusted),
|
||||
};
|
||||
|
||||
let resolution = cfg
|
||||
.derive_sandbox_policy(
|
||||
/*sandbox_mode_override*/ None,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
Some(&active_project),
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&cfg,
|
||||
/*sandbox_mode_override*/ None,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
Some(&active_project),
|
||||
/*permission_profile_constraint*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
// Verify that untrusted projects get WorkspaceWrite (or ReadOnly on Windows due to downgrade)
|
||||
if cfg!(target_os = "windows") {
|
||||
@@ -6367,15 +6460,15 @@ async fn derive_sandbox_policy_falls_back_to_read_only_for_implicit_defaults() -
|
||||
}
|
||||
})?;
|
||||
|
||||
let resolution = cfg
|
||||
.derive_sandbox_policy(
|
||||
/*sandbox_mode_override*/ None,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
Some(&active_project),
|
||||
Some(&constrained),
|
||||
)
|
||||
.await;
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&cfg,
|
||||
/*sandbox_mode_override*/ None,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
Some(&active_project),
|
||||
Some(&constrained),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(resolution, SandboxPolicy::new_read_only_policy());
|
||||
Ok(())
|
||||
@@ -6423,15 +6516,15 @@ async fn derive_sandbox_policy_preserves_windows_downgrade_for_unsupported_fallb
|
||||
},
|
||||
)?;
|
||||
|
||||
let resolution = cfg
|
||||
.derive_sandbox_policy(
|
||||
/*sandbox_mode_override*/ None,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
Some(&active_project),
|
||||
Some(&constrained),
|
||||
)
|
||||
.await;
|
||||
let resolution = derive_legacy_sandbox_policy_for_test(
|
||||
&cfg,
|
||||
/*sandbox_mode_override*/ None,
|
||||
/*profile_sandbox_mode*/ None,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
Some(&active_project),
|
||||
Some(&constrained),
|
||||
)
|
||||
.await;
|
||||
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(resolution, SandboxPolicy::new_read_only_policy());
|
||||
|
||||
@@ -1976,8 +1976,13 @@ impl Config {
|
||||
)
|
||||
} else {
|
||||
let configured_network_proxy_config = NetworkProxyConfig::default();
|
||||
let mut sandbox_policy = cfg
|
||||
.derive_sandbox_policy(
|
||||
// No named `[permissions]` profile is active, but permissions
|
||||
// should still flow through the canonical profile representation.
|
||||
// Derive the old `sandbox_mode` defaults as a profile first, then
|
||||
// keep a legacy-compatible projection only for the remaining code
|
||||
// paths that still speak `SandboxPolicy`.
|
||||
let mut permission_profile = cfg
|
||||
.derive_permission_profile(
|
||||
sandbox_mode,
|
||||
config_profile.sandbox_mode,
|
||||
windows_sandbox_level,
|
||||
@@ -1985,24 +1990,46 @@ impl Config {
|
||||
Some(&constrained_permission_profile),
|
||||
)
|
||||
.await;
|
||||
if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy {
|
||||
for path in &additional_writable_roots {
|
||||
if !writable_roots.iter().any(|existing| existing == path) {
|
||||
writable_roots.push(path.clone());
|
||||
}
|
||||
}
|
||||
// The legacy-derived profiles above are expected to be
|
||||
// representable as `SandboxPolicy`. This guard keeps the old safe
|
||||
// fallback behavior if future changes make this branch derive a
|
||||
// profile with split-only filesystem semantics, such as root write
|
||||
// with carveouts or writes that are not expressible as
|
||||
// workspace-write roots.
|
||||
if let Err(err) = permission_profile.to_legacy_sandbox_policy(resolved_cwd.as_path()) {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"derived permission profile cannot be represented as a legacy sandbox policy; falling back to read-only"
|
||||
);
|
||||
permission_profile = PermissionProfile::read_only();
|
||||
}
|
||||
let (mut file_system_sandbox_policy, network_sandbox_policy) =
|
||||
permission_profile.to_runtime_permissions();
|
||||
// `additional_writable_roots` is a legacy workspace-write knob. It
|
||||
// only applies when the derived managed profile has workspace-style
|
||||
// write access to the project roots; read-only, disabled, external,
|
||||
// and future non-workspace profiles must not silently grow extra
|
||||
// write access.
|
||||
if matches!(permission_profile.enforcement(), SandboxEnforcement::Managed)
|
||||
&& file_system_sandbox_policy.can_write_path_with_cwd(
|
||||
resolved_cwd.as_path(),
|
||||
resolved_cwd.as_path(),
|
||||
)
|
||||
&& !file_system_sandbox_policy.has_full_disk_write_access()
|
||||
{
|
||||
// Keep legacy behavior for extra writable roots while storing
|
||||
// the result as the canonical permission profile. Explicit
|
||||
// extra roots are concrete paths, so their metadata carveouts
|
||||
// are also concrete rather than symbolic `:project_roots`
|
||||
// entries.
|
||||
file_system_sandbox_policy = file_system_sandbox_policy
|
||||
.with_additional_legacy_workspace_writable_roots(&additional_writable_roots);
|
||||
permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
permission_profile.enforcement(),
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
);
|
||||
}
|
||||
let file_system_sandbox_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(
|
||||
&sandbox_policy,
|
||||
resolved_cwd.as_path(),
|
||||
);
|
||||
let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy);
|
||||
let permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy),
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
);
|
||||
(
|
||||
configured_network_proxy_config,
|
||||
permission_profile,
|
||||
|
||||
@@ -111,7 +111,11 @@ fn read_only_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
|
||||
}
|
||||
|
||||
fn workspace_write_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&SandboxPolicy::new_workspace_write_policy())
|
||||
FileSystemSandboxPolicy::workspace_write(
|
||||
&[],
|
||||
/*exclude_tmpdir_env_var*/ false,
|
||||
/*exclude_slash_tmp*/ false,
|
||||
)
|
||||
}
|
||||
|
||||
fn unrestricted_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
|
||||
|
||||
Reference in New Issue
Block a user