permissions: store only constrained permission profiles (#19735)

This commit is contained in:
Michael Bolin
2026-04-26 20:59:58 -07:00
committed by GitHub
Unverified
parent 8033b6a449
commit 0ccd659b4b
32 changed files with 242 additions and 215 deletions
+1 -1
View File
@@ -574,7 +574,7 @@ writable_roots = ["./sandbox-root"]
false
);
match &*config.permissions.sandbox_policy {
match &config.legacy_sandbox_policy() {
SandboxPolicy::WorkspaceWrite { network_access, .. } => {
assert_eq!(*network_access, true);
}
@@ -526,7 +526,7 @@ writable_roots = ["~/code"]
.await?;
let expected_root = AbsolutePathBuf::from_absolute_path(home.join("code"))?;
match config.permissions.sandbox_policy.get() {
match &config.legacy_sandbox_policy() {
SandboxPolicy::WorkspaceWrite { writable_roots, .. } => {
assert_eq!(
writable_roots
+23 -36
View File
@@ -806,7 +806,7 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::
]),
);
assert_eq!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![memories_root],
network_access: false,
@@ -840,7 +840,7 @@ async fn permission_profile_override_populates_runtime_permissions() -> std::io:
assert_eq!(config.permissions.permission_profile(), permission_profile);
assert_eq!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
&SandboxPolicy::DangerFullAccess
);
Ok(())
@@ -869,7 +869,7 @@ async fn permission_profile_override_preserves_managed_unrestricted_filesystem()
assert_eq!(config.permissions.permission_profile(), permission_profile);
assert_eq!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
&SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Restricted,
}
@@ -898,7 +898,7 @@ async fn managed_unrestricted_permission_profile_still_enables_network_requireme
)
.await?;
assert_eq!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
&SandboxPolicy::DangerFullAccess,
"the legacy projection is intentionally lossy for managed unrestricted profiles"
);
@@ -974,7 +974,7 @@ async fn permission_profile_override_applies_runtime_roots_to_legacy_projection(
.can_write_path_with_cwd(memories_root.as_path(), cwd.path())
);
assert_eq!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![memories_root],
network_access: false,
@@ -1209,7 +1209,7 @@ async fn permissions_profiles_allow_direct_write_roots_outside_workspace_root()
.can_write_path_with_cwd(external_write_path.as_path(), cwd.path())
);
assert_eq!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![external_write_path, memories_root],
network_access: false,
@@ -1317,7 +1317,7 @@ async fn permissions_profiles_allow_unknown_special_paths() -> std::io::Result<(
}]),
);
assert_eq!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
&SandboxPolicy::ReadOnly {
network_access: false,
}
@@ -1382,7 +1382,7 @@ async fn permissions_profiles_allow_missing_filesystem_with_warning() -> std::io
FileSystemSandboxPolicy::restricted(Vec::new())
);
assert_eq!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
&SandboxPolicy::ReadOnly {
network_access: false,
}
@@ -1509,13 +1509,7 @@ async fn permissions_profiles_allow_network_enablement() -> std::io::Result<()>
config.permissions.network_sandbox_policy().is_enabled(),
"expected network sandbox policy to be enabled",
);
assert!(
config
.permissions
.sandbox_policy
.get()
.has_full_network_access()
);
assert!(config.legacy_sandbox_policy().has_full_network_access());
Ok(())
}
@@ -1799,7 +1793,7 @@ exclude_slash_tmp = true
)
.await?;
let sandbox_policy = config.permissions.sandbox_policy.get();
let sandbox_policy = &config.legacy_sandbox_policy();
assert_eq!(
config.permissions.file_system_sandbox_policy(),
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(sandbox_policy, cwd.path()),
@@ -1982,12 +1976,12 @@ async fn add_dir_override_extends_workspace_writable_roots() -> std::io::Result<
let expected_backend = backend.abs();
if cfg!(target_os = "windows") {
match config.permissions.sandbox_policy.get() {
match &config.legacy_sandbox_policy() {
SandboxPolicy::ReadOnly { .. } => {}
other => panic!("expected read-only policy on Windows, got {other:?}"),
}
} else {
match config.permissions.sandbox_policy.get() {
match &config.legacy_sandbox_policy() {
SandboxPolicy::WorkspaceWrite { writable_roots, .. } => {
assert_eq!(
writable_roots
@@ -2045,7 +2039,7 @@ async fn workspace_write_always_includes_memories_root_once() -> std::io::Result
.await?;
if cfg!(target_os = "windows") {
match config.permissions.sandbox_policy.get() {
match &config.legacy_sandbox_policy() {
SandboxPolicy::ReadOnly { .. } => {}
other => panic!("expected read-only policy on Windows, got {other:?}"),
}
@@ -2056,7 +2050,7 @@ async fn workspace_write_always_includes_memories_root_once() -> std::io::Result
memories_root.display()
);
let expected_memories_root = memories_root.abs();
match config.permissions.sandbox_policy.get() {
match &config.legacy_sandbox_policy() {
SandboxPolicy::WorkspaceWrite { writable_roots, .. } => {
assert_eq!(
writable_roots
@@ -2375,7 +2369,7 @@ async fn profile_sandbox_mode_overrides_base() -> std::io::Result<()> {
.await?;
assert!(matches!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
&SandboxPolicy::DangerFullAccess
));
@@ -2409,12 +2403,12 @@ async fn cli_override_takes_precedence_over_profile_sandbox_mode() -> std::io::R
if cfg!(target_os = "windows") {
assert!(matches!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
SandboxPolicy::ReadOnly { .. }
));
} else {
assert!(matches!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
SandboxPolicy::WorkspaceWrite { .. }
));
}
@@ -5448,7 +5442,6 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
permissions: Permissions {
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()),
network: None,
allow_login_shell: true,
shell_environment_policy: ShellEnvironmentPolicy::default(),
@@ -5642,7 +5635,6 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
permissions: Permissions {
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()),
network: None,
allow_login_shell: true,
shell_environment_policy: ShellEnvironmentPolicy::default(),
@@ -5790,7 +5782,6 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
permissions: Permissions {
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()),
network: None,
allow_login_shell: true,
shell_environment_policy: ShellEnvironmentPolicy::default(),
@@ -5923,7 +5914,6 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
permissions: Permissions {
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()),
network: None,
allow_login_shell: true,
shell_environment_policy: ShellEnvironmentPolicy::default(),
@@ -6660,7 +6650,7 @@ async fn test_untrusted_project_gets_unless_trusted_approval_policy() -> anyhow:
if cfg!(target_os = "windows") {
assert!(
matches!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
SandboxPolicy::ReadOnly { .. }
),
"Expected ReadOnly on Windows"
@@ -6668,7 +6658,7 @@ async fn test_untrusted_project_gets_unless_trusted_approval_policy() -> anyhow:
} else {
assert!(
matches!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
SandboxPolicy::WorkspaceWrite { .. }
),
"Expected WorkspaceWrite sandbox for untrusted project"
@@ -6694,7 +6684,7 @@ async fn requirements_disallowing_default_sandbox_falls_back_to_required_default
.build()
.await?;
assert_eq!(
*config.permissions.sandbox_policy.get(),
config.legacy_sandbox_policy(),
SandboxPolicy::new_read_only_policy()
);
Ok(())
@@ -6735,7 +6725,7 @@ async fn explicit_sandbox_mode_falls_back_when_disallowed_by_requirements() -> s
.build()
.await?;
assert_eq!(
*config.permissions.sandbox_policy.get(),
config.legacy_sandbox_policy(),
SandboxPolicy::new_read_only_policy()
);
Ok(())
@@ -6764,10 +6754,7 @@ async fn permission_profile_override_falls_back_when_disallowed_by_requirements(
.await?;
let expected_sandbox_policy = SandboxPolicy::new_read_only_policy();
assert_eq!(
*config.permissions.sandbox_policy.get(),
expected_sandbox_policy
);
assert_eq!(config.legacy_sandbox_policy(), expected_sandbox_policy);
assert_eq!(
config.permissions.permission_profile(),
PermissionProfile::read_only()
@@ -6821,7 +6808,7 @@ async fn permission_profile_override_preserves_split_write_roots() -> std::io::R
.can_write_path_with_cwd(outside_root.as_path(), config.cwd.as_path())
);
assert!(matches!(
config.permissions.sandbox_policy.get(),
&config.legacy_sandbox_policy(),
SandboxPolicy::WorkspaceWrite { .. }
));
assert_eq!(
+14 -22
View File
@@ -195,11 +195,6 @@ pub struct Permissions {
/// Canonical effective runtime permissions after config requirements and
/// runtime readable-root additions have been applied.
pub permission_profile: Constrained<PermissionProfile>,
/// Effective sandbox policy used for shell/unified exec.
///
/// Legacy projection retained while runtime call sites migrate to
/// `permission_profile`.
pub sandbox_policy: Constrained<SandboxPolicy>,
/// Effective network configuration applied to all spawned processes.
pub network: Option<NetworkProxySpec>,
/// Whether the model may request a login shell for shell-based tools.
@@ -250,13 +245,12 @@ impl Permissions {
}
/// Check whether a legacy sandbox policy can be applied to this permission
/// set under both legacy and canonical profile constraints.
/// set after projecting it into the canonical permission profile.
pub fn can_set_legacy_sandbox_policy(
&self,
sandbox_policy: &SandboxPolicy,
cwd: &Path,
) -> ConstraintResult<()> {
self.sandbox_policy.can_set(sandbox_policy)?;
let file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(sandbox_policy, cwd);
let network_sandbox_policy = NetworkSandboxPolicy::from(sandbox_policy);
@@ -285,31 +279,18 @@ impl Permissions {
network_sandbox_policy,
);
self.sandbox_policy.set(sandbox_policy)?;
self.permission_profile.set(permission_profile)?;
Ok(())
}
/// Replace permissions from the canonical profile and update compatibility
/// projections for legacy consumers.
/// Replace permissions from the canonical profile.
pub fn set_permission_profile(
&mut self,
permission_profile: PermissionProfile,
cwd: &Path,
) -> ConstraintResult<()> {
let (file_system_sandbox_policy, network_sandbox_policy) =
permission_profile.to_runtime_permissions();
let sandbox_policy = compatibility_sandbox_policy_for_permission_profile(
&permission_profile,
&file_system_sandbox_policy,
network_sandbox_policy,
cwd,
);
self.permission_profile.can_set(&permission_profile)?;
self.sandbox_policy.can_set(&sandbox_policy)?;
self.permission_profile.set(permission_profile)?;
self.sandbox_policy.set(sandbox_policy)?;
Ok(())
}
}
@@ -915,6 +896,18 @@ impl ConfigBuilder {
}
impl Config {
pub fn legacy_sandbox_policy(&self) -> SandboxPolicy {
self.permissions.legacy_sandbox_policy(self.cwd.as_path())
}
pub fn set_legacy_sandbox_policy(
&mut self,
sandbox_policy: SandboxPolicy,
) -> ConstraintResult<()> {
self.permissions
.set_legacy_sandbox_policy(sandbox_policy, self.cwd.as_path())
}
pub fn to_models_manager_config(&self) -> ModelsManagerConfig {
ModelsManagerConfig {
model_context_window: self.model_context_window,
@@ -2484,7 +2477,6 @@ impl Config {
permissions: Permissions {
approval_policy: constrained_approval_policy.value,
permission_profile: constrained_permission_profile,
sandbox_policy: constrained_sandbox_policy.value,
network,
allow_login_shell,
shell_environment_policy,
@@ -848,7 +848,6 @@ pub(crate) fn build_guardian_review_session_config(
guardian_config.permissions.permission_profile = Constrained::allow_only(
PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy),
);
guardian_config.permissions.sandbox_policy = Constrained::allow_only(sandbox_policy.clone());
guardian_config
.permissions
.set_legacy_sandbox_policy(sandbox_policy, guardian_config.cwd.as_path())
+4 -2
View File
@@ -1984,8 +1984,10 @@ async fn guardian_review_session_config_preserves_parent_network_proxy() {
Constrained::allow_only(AskForApproval::Never)
);
assert_eq!(
guardian_config.permissions.sandbox_policy,
Constrained::allow_only(SandboxPolicy::new_read_only_policy())
guardian_config.permissions.permission_profile,
Constrained::allow_only(PermissionProfile::from_legacy_sandbox_policy(
&SandboxPolicy::new_read_only_policy(),
))
);
}
+4 -3
View File
@@ -489,7 +489,7 @@ mod phase2 {
);
config
.permissions
.set_permission_profile(permission_profile, config.cwd.as_path())
.set_permission_profile(permission_profile)
.expect("permissions are configurable");
configure(&mut config);
let config = Arc::new(config);
@@ -935,8 +935,9 @@ mod phase2 {
.await
.expect("enqueue global consolidation");
let mut constrained_config = harness.config.as_ref().clone();
constrained_config.permissions.sandbox_policy =
Constrained::allow_only(SandboxPolicy::DangerFullAccess);
constrained_config.permissions.permission_profile = Constrained::allow_only(
PermissionProfile::from_legacy_sandbox_policy(&SandboxPolicy::DangerFullAccess),
);
phase2::run(&harness.session, Arc::new(constrained_config)).await;
+4 -2
View File
@@ -1515,7 +1515,9 @@ async fn session_configured_reports_permission_profile_for_external_sandbox() ->
};
let expected_sandbox_policy = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.sandbox_policy = codex_config::Constrained::allow_any(sandbox_policy);
config
.set_legacy_sandbox_policy(sandbox_policy)
.expect("set sandbox policy");
config.permissions.permission_profile =
codex_config::Constrained::allow_any(PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::external_sandbox(),
@@ -4187,7 +4189,7 @@ async fn user_turn_updates_approvals_reviewer() {
cwd: config.cwd.to_path_buf(),
approval_policy: config.permissions.approval_policy.value(),
approvals_reviewer: Some(codex_config::types::ApprovalsReviewer::AutoReview),
sandbox_policy: config.permissions.sandbox_policy.get().clone(),
sandbox_policy: config.legacy_sandbox_policy(),
permission_profile: None,
model: turn_context.model_info.slug.clone(),
effort: config.model_reasoning_effort,
@@ -383,8 +383,6 @@ impl Session {
per_turn_config.approvals_reviewer = session_configuration.approvals_reviewer;
per_turn_config.permissions.permission_profile =
session_configuration.permission_profile.clone();
let sandbox_policy = session_configuration.sandbox_policy();
per_turn_config.permissions.sandbox_policy = Constrained::allow_only(sandbox_policy);
let permission_profile = session_configuration.permission_profile();
let resolved_web_search_mode =
resolve_web_search_mode_for_turn(&per_turn_config.web_search_mode, &permission_profile);
@@ -269,7 +269,7 @@ pub(crate) fn apply_spawn_agent_runtime_overrides(
config.cwd = turn.cwd.clone();
config
.permissions
.set_permission_profile(turn.permission_profile(), turn.cwd.as_path())
.set_permission_profile(turn.permission_profile())
.map_err(|err| {
FunctionCallError::RespondToModel(format!("permission_profile is invalid: {err}"))
})?;
@@ -2086,7 +2086,7 @@ async fn spawn_agent_reapplies_runtime_sandbox_after_role_config() {
let (mut session, mut turn) = make_session_and_context().await;
let manager = thread_manager();
session.services.agent_control = manager.agent_control();
let expected_sandbox = turn.config.permissions.sandbox_policy.get().clone();
let expected_sandbox = turn.config.legacy_sandbox_policy();
let mut expected_file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&expected_sandbox, &turn.cwd);
expected_file_system_sandbox_policy
@@ -3585,8 +3585,9 @@ async fn tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtr
#[tokio::test]
async fn build_agent_spawn_config_uses_turn_context_values() {
fn pick_allowed_sandbox_policy(
constraint: &crate::config::Constrained<SandboxPolicy>,
constraint: &crate::config::Constrained<PermissionProfile>,
base: SandboxPolicy,
cwd: &std::path::Path,
) -> SandboxPolicy {
let candidates = [
SandboxPolicy::new_read_only_policy(),
@@ -3595,7 +3596,21 @@ async fn build_agent_spawn_config_uses_turn_context_values() {
];
candidates
.into_iter()
.find(|candidate| *candidate != base && constraint.can_set(candidate).is_ok())
.find(|candidate| {
if *candidate == base {
return false;
}
let file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(candidate, cwd);
let network_sandbox_policy = NetworkSandboxPolicy::from(candidate);
let permission_profile =
PermissionProfile::from_runtime_permissions_with_enforcement(
SandboxEnforcement::from_legacy_sandbox_policy(candidate),
&file_system_sandbox_policy,
network_sandbox_policy,
);
constraint.can_set(&permission_profile).is_ok()
})
.unwrap_or(base)
}
@@ -3613,8 +3628,9 @@ async fn build_agent_spawn_config_uses_turn_context_values() {
turn.cwd = temp_dir.abs();
turn.codex_linux_sandbox_exe = Some(PathBuf::from("/bin/echo"));
let sandbox_policy = pick_allowed_sandbox_policy(
&turn.config.permissions.sandbox_policy,
turn.config.permissions.sandbox_policy.get().clone(),
&turn.config.permissions.permission_profile,
turn.config.legacy_sandbox_policy(),
turn.cwd.as_path(),
);
let file_system_sandbox_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&sandbox_policy, &turn.cwd);
@@ -3648,7 +3664,7 @@ async fn build_agent_spawn_config_uses_turn_context_values() {
.expect("approval policy set");
expected
.permissions
.set_permission_profile(permission_profile, turn.cwd.as_path())
.set_permission_profile(permission_profile)
.expect("permission profile set");
assert_eq!(config, expected);
}
@@ -3699,8 +3715,7 @@ async fn build_agent_resume_config_clears_base_instructions() {
.expect("approval policy set");
expected
.permissions
.sandbox_policy
.set(turn.sandbox_policy())
.expect("sandbox policy set");
.set_permission_profile(turn.permission_profile())
.expect("permission profile set");
assert_eq!(config, expected);
}