mirror of
https://github.com/openai/codex.git
synced 2026-11-04 00:24:43 +00:00
permissions: derive compatibility policies from profiles (#19392)
## Why After #19391, `PermissionProfile` and the split filesystem/network policies could still be stored in parallel. That creates drift risk: a profile can preserve deny globs, external enforcement, or split filesystem entries while a cached projection silently loses those details. This PR makes the profile the runtime source and derives compatibility views from it. ## What Changed - Removes stored filesystem/network sandbox projections from `Permissions` and `SessionConfiguration`; their accessors now derive from the canonical `PermissionProfile`. - Derives legacy `SandboxPolicy` snapshots from profiles only where an older API still needs that field. - Updates MCP connection and elicitation state to track `PermissionProfile` instead of `SandboxPolicy` for auto-approval decisions. - Adds semantic filesystem-policy comparison so cwd changes can preserve richer profiles while still recognizing equivalent legacy projections independent of entry ordering. - Updates config/session tests to assert profile-derived projections instead of parallel stored fields. ## Verification - `cargo test -p codex-core direct_write_roots` - `cargo test -p codex-core runtime_roots_to_legacy_projection` - `cargo test -p codex-app-server requested_permissions_trust_project_uses_permission_profile_intent` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19392). * #19395 * #19394 * #19393 * __->__ #19392
This commit is contained in:
1 parent
4d7ce3447d
commit
deaa307fb2
39 files changed
+568
-439
No files matched your search
@@ -776,7 +776,7 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::
|
||||
|
||||
let memories_root = codex_home.path().join("memories").abs();
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy,
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
@@ -814,7 +814,7 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
config.permissions.network_sandbox_policy,
|
||||
config.permissions.network_sandbox_policy(),
|
||||
NetworkSandboxPolicy::Restricted
|
||||
);
|
||||
Ok(())
|
||||
@@ -1082,7 +1082,7 @@ async fn project_root_glob_none_compiles_to_filesystem_pattern_entry() -> std::i
|
||||
assert_eq!(
|
||||
config
|
||||
.permissions
|
||||
.file_system_sandbox_policy
|
||||
.file_system_sandbox_policy()
|
||||
.glob_scan_max_depth,
|
||||
Some(2)
|
||||
);
|
||||
@@ -1092,7 +1092,7 @@ async fn project_root_glob_none_compiles_to_filesystem_pattern_entry() -> std::i
|
||||
assert!(
|
||||
config
|
||||
.permissions
|
||||
.file_system_sandbox_policy
|
||||
.file_system_sandbox_policy()
|
||||
.entries
|
||||
.contains(&FileSystemSandboxEntry {
|
||||
path: FileSystemPath::GlobPattern {
|
||||
@@ -1104,7 +1104,7 @@ async fn project_root_glob_none_compiles_to_filesystem_pattern_entry() -> std::i
|
||||
assert!(
|
||||
!config
|
||||
.permissions
|
||||
.file_system_sandbox_policy
|
||||
.file_system_sandbox_policy()
|
||||
.entries
|
||||
.iter()
|
||||
.any(|entry| matches!(
|
||||
@@ -1304,7 +1304,7 @@ async fn permissions_profiles_allow_unknown_special_paths() -> std::io::Result<(
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy,
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::unknown(
|
||||
@@ -1350,7 +1350,7 @@ async fn permissions_profiles_allow_unknown_special_paths_with_nested_entries()
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy,
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::unknown(":future_special_path", Some("docs".into())),
|
||||
@@ -1377,7 +1377,7 @@ async fn permissions_profiles_allow_missing_filesystem_with_warning() -> std::io
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy,
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
FileSystemSandboxPolicy::restricted(Vec::new())
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -1408,7 +1408,7 @@ async fn permissions_profiles_allow_empty_filesystem_with_warning() -> std::io::
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy,
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
FileSystemSandboxPolicy::restricted(Vec::new())
|
||||
);
|
||||
assert!(
|
||||
@@ -1505,7 +1505,7 @@ async fn permissions_profiles_allow_network_enablement() -> std::io::Result<()>
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
config.permissions.network_sandbox_policy.is_enabled(),
|
||||
config.permissions.network_sandbox_policy().is_enabled(),
|
||||
"expected network sandbox policy to be enabled",
|
||||
);
|
||||
assert!(
|
||||
@@ -1800,20 +1800,20 @@ exclude_slash_tmp = true
|
||||
|
||||
let sandbox_policy = config.permissions.sandbox_policy.get();
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy,
|
||||
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,
|
||||
config.permissions.network_sandbox_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_sandbox_policy()
|
||||
.to_legacy_sandbox_policy(config.permissions.network_sandbox_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"
|
||||
@@ -5449,10 +5449,6 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
||||
approval_policy: Constrained::allow_any(AskForApproval::Never),
|
||||
permission_profile: Constrained::allow_any(PermissionProfile::read_only()),
|
||||
sandbox_policy: Constrained::allow_any(SandboxPolicy::new_read_only_policy()),
|
||||
file_system_sandbox_policy: FileSystemSandboxPolicy::from(
|
||||
&SandboxPolicy::new_read_only_policy(),
|
||||
),
|
||||
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
|
||||
network: None,
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
@@ -5647,10 +5643,6 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
approval_policy: Constrained::allow_any(AskForApproval::UnlessTrusted),
|
||||
permission_profile: Constrained::allow_any(PermissionProfile::read_only()),
|
||||
sandbox_policy: Constrained::allow_any(SandboxPolicy::new_read_only_policy()),
|
||||
file_system_sandbox_policy: FileSystemSandboxPolicy::from(
|
||||
&SandboxPolicy::new_read_only_policy(),
|
||||
),
|
||||
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
|
||||
network: None,
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
@@ -5799,10 +5791,6 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
||||
approval_policy: Constrained::allow_any(AskForApproval::OnFailure),
|
||||
permission_profile: Constrained::allow_any(PermissionProfile::read_only()),
|
||||
sandbox_policy: Constrained::allow_any(SandboxPolicy::new_read_only_policy()),
|
||||
file_system_sandbox_policy: FileSystemSandboxPolicy::from(
|
||||
&SandboxPolicy::new_read_only_policy(),
|
||||
),
|
||||
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
|
||||
network: None,
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
@@ -5936,10 +5924,6 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
approval_policy: Constrained::allow_any(AskForApproval::OnFailure),
|
||||
permission_profile: Constrained::allow_any(PermissionProfile::read_only()),
|
||||
sandbox_policy: Constrained::allow_any(SandboxPolicy::new_read_only_policy()),
|
||||
file_system_sandbox_policy: FileSystemSandboxPolicy::from(
|
||||
&SandboxPolicy::new_read_only_policy(),
|
||||
),
|
||||
network_sandbox_policy: NetworkSandboxPolicy::Restricted,
|
||||
network: None,
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
|
||||
@@ -200,18 +200,6 @@ pub struct Permissions {
|
||||
/// Legacy projection retained while runtime call sites migrate to
|
||||
/// `permission_profile`.
|
||||
pub sandbox_policy: Constrained<SandboxPolicy>,
|
||||
/// Effective filesystem sandbox policy, including entries that cannot yet
|
||||
/// be fully represented by the legacy [`SandboxPolicy`] projection.
|
||||
///
|
||||
/// Runtime projection retained while callers migrate to
|
||||
/// `permission_profile`.
|
||||
pub file_system_sandbox_policy: FileSystemSandboxPolicy,
|
||||
/// Effective network sandbox policy split out from the legacy
|
||||
/// [`SandboxPolicy`] projection.
|
||||
///
|
||||
/// Runtime projection retained while callers migrate to
|
||||
/// `permission_profile`.
|
||||
pub network_sandbox_policy: NetworkSandboxPolicy,
|
||||
/// Effective network configuration applied to all spawned processes.
|
||||
pub network: Option<NetworkProxySpec>,
|
||||
/// Whether the model may request a login shell for shell-based tools.
|
||||
@@ -239,14 +227,14 @@ impl Permissions {
|
||||
self.permission_profile.get().clone()
|
||||
}
|
||||
|
||||
/// Effective filesystem sandbox policy projection.
|
||||
/// Effective filesystem sandbox policy derived from the canonical profile.
|
||||
pub fn file_system_sandbox_policy(&self) -> FileSystemSandboxPolicy {
|
||||
self.file_system_sandbox_policy.clone()
|
||||
self.permission_profile.get().file_system_sandbox_policy()
|
||||
}
|
||||
|
||||
/// Effective network sandbox policy projection.
|
||||
/// Effective network sandbox policy derived from the canonical profile.
|
||||
pub fn network_sandbox_policy(&self) -> NetworkSandboxPolicy {
|
||||
self.network_sandbox_policy
|
||||
self.permission_profile.get().network_sandbox_policy()
|
||||
}
|
||||
|
||||
/// Replace permissions from a legacy sandbox policy and keep every
|
||||
@@ -269,8 +257,6 @@ impl Permissions {
|
||||
|
||||
self.sandbox_policy.set(sandbox_policy)?;
|
||||
self.permission_profile.set(permission_profile)?;
|
||||
self.file_system_sandbox_policy = file_system_sandbox_policy;
|
||||
self.network_sandbox_policy = network_sandbox_policy;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -294,8 +280,6 @@ impl Permissions {
|
||||
|
||||
self.permission_profile.set(permission_profile)?;
|
||||
self.sandbox_policy.set(sandbox_policy)?;
|
||||
self.file_system_sandbox_policy = file_system_sandbox_policy;
|
||||
self.network_sandbox_policy = network_sandbox_policy;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
@@ -2493,8 +2477,6 @@ impl Config {
|
||||
approval_policy: constrained_approval_policy.value,
|
||||
permission_profile: constrained_permission_profile,
|
||||
sandbox_policy: constrained_sandbox_policy.value,
|
||||
file_system_sandbox_policy: effective_file_system_sandbox_policy,
|
||||
network_sandbox_policy: effective_network_sandbox_policy,
|
||||
network,
|
||||
allow_login_shell,
|
||||
shell_environment_policy,
|
||||
|
||||
@@ -89,7 +89,7 @@ async fn restricted_read_implicitly_allows_helper_executables() -> std::io::Resu
|
||||
let expected_zsh = AbsolutePathBuf::try_from(zsh_path)?;
|
||||
let expected_allowed_arg0_dir = AbsolutePathBuf::try_from(allowed_arg0_dir)?;
|
||||
let expected_sibling_arg0_dir = AbsolutePathBuf::try_from(sibling_arg0_dir)?;
|
||||
let policy = &config.permissions.file_system_sandbox_policy;
|
||||
let policy = config.permissions.file_system_sandbox_policy();
|
||||
|
||||
assert!(
|
||||
policy.can_read_path_with_cwd(expected_zsh.as_path(), &cwd),
|
||||
|
||||
Reference in new issue
Block a user