Skip to content

permissions: constrain requirements as profiles #19736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bolinfest merged 1 commit into from
Apr 27, 2026
Merged

permissions: constrain requirements as profiles #19736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 94 additions & 65 deletions codex-rs/config/src/config_requirements.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::config_types::WebSearchMode;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Deserialize;
use serde::Serialize;
Expand Down Expand Up @@ -84,7 +84,7 @@ impl<T> std::ops::DerefMut for ConstrainedWithSource<T> {
pub struct ConfigRequirements {
pub approval_policy: ConstrainedWithSource<AskForApproval>,
pub approvals_reviewer: ConstrainedWithSource<ApprovalsReviewer>,
pub sandbox_policy: ConstrainedWithSource<SandboxPolicy>,
pub permission_profile: ConstrainedWithSource<PermissionProfile>,
pub web_search_mode: ConstrainedWithSource<WebSearchMode>,
pub feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
pub managed_hooks: Option<ConstrainedWithSource<ManagedHooksRequirementsToml>>,
Expand All @@ -110,8 +110,8 @@ impl Default for ConfigRequirements {
Constrained::allow_any_from_default(),
/*source*/ None,
),
sandbox_policy: ConstrainedWithSource::new(
Constrained::allow_any(SandboxPolicy::new_read_only_policy()),
permission_profile: ConstrainedWithSource::new(
Constrained::allow_any(PermissionProfile::read_only()),
/*source*/ None,
),
web_search_mode: ConstrainedWithSource::new(
Expand Down Expand Up @@ -967,15 +967,8 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
),
};

// TODO(gt): `ConfigRequirementsToml` should let the author specify the
// default `SandboxPolicy`? Should do this for `AskForApproval` too?
//
// Currently, we force ReadOnly as the default policy because two of
// the other variants (WorkspaceWrite, ExternalSandbox) require
// additional parameters. Ultimately, we should expand the config
// format to allow specifying those parameters.
let default_sandbox_policy = SandboxPolicy::new_read_only_policy();
let sandbox_policy = match allowed_sandbox_modes {
let default_permission_profile = PermissionProfile::read_only();
let permission_profile = match allowed_sandbox_modes {
Some(Sourced {
value: modes,
source: requirement_source,
Expand All @@ -984,23 +977,15 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
return Err(ConstraintError::InvalidValue {
field_name: "allowed_sandbox_modes",
candidate: format!("{modes:?}"),
allowed: "must include 'read-only' to allow any SandboxPolicy".to_string(),
allowed: "must include 'read-only' to allow any PermissionProfile"
.to_string(),
requirement_source,
});
};

let requirement_source_for_error = requirement_source.clone();
let constrained = Constrained::new(default_sandbox_policy, move |candidate| {
let mode = match candidate {
SandboxPolicy::ReadOnly { .. } => SandboxModeRequirement::ReadOnly,
SandboxPolicy::WorkspaceWrite { .. } => {
SandboxModeRequirement::WorkspaceWrite
}
SandboxPolicy::DangerFullAccess => SandboxModeRequirement::DangerFullAccess,
SandboxPolicy::ExternalSandbox { .. } => {
SandboxModeRequirement::ExternalSandbox
}
};
let constrained = Constrained::new(default_permission_profile, move |candidate| {
let mode = sandbox_mode_requirement_for_permission_profile(candidate);
if modes.contains(&mode) {
Ok(())
} else {
Expand All @@ -1014,12 +999,10 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
})?;
ConstrainedWithSource::new(constrained, Some(requirement_source))
}
None => {
ConstrainedWithSource::new(
Constrained::allow_any(default_sandbox_policy),
/*source*/ None,
)
}
None => ConstrainedWithSource::new(
Constrained::allow_any(default_permission_profile),
/*source*/ None,
),
};
let exec_policy = match rules {
Some(Sourced { value, source }) => {
Expand Down Expand Up @@ -1145,7 +1128,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
Ok(ConfigRequirements {
approval_policy,
approvals_reviewer,
sandbox_policy,
permission_profile,
web_search_mode,
feature_requirements,
managed_hooks,
Expand All @@ -1159,6 +1142,29 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
}
}

pub fn sandbox_mode_requirement_for_permission_profile(
permission_profile: &PermissionProfile,
) -> SandboxModeRequirement {
match permission_profile {
PermissionProfile::Disabled => SandboxModeRequirement::DangerFullAccess,
PermissionProfile::External { .. } => SandboxModeRequirement::ExternalSandbox,
PermissionProfile::Managed { .. } => {
let file_system_policy = permission_profile.file_system_sandbox_policy();
if file_system_policy.has_full_disk_write_access() {
SandboxModeRequirement::DangerFullAccess
} else if file_system_policy
.entries
.iter()
.any(|entry| entry.access.can_write())
{
SandboxModeRequirement::WorkspaceWrite
} else {
SandboxModeRequirement::ReadOnly
}
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -1168,6 +1174,7 @@ mod tests {
use codex_execpolicy::Evaluation;
use codex_execpolicy::RuleMatch;
use codex_protocol::protocol::NetworkAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::AbsolutePathBufGuard;
use pretty_assertions::assert_eq;
Expand All @@ -1183,6 +1190,10 @@ mod tests {
)?)
}

fn profile_from_sandbox_policy(sandbox_policy: &SandboxPolicy) -> PermissionProfile {
PermissionProfile::from_legacy_sandbox_policy(sandbox_policy)
}

fn with_unknown_source(toml: ConfigRequirementsToml) -> ConfigRequirementsWithSources {
let ConfigRequirementsToml {
allowed_approval_policies,
Expand Down Expand Up @@ -1724,8 +1735,10 @@ allowed_approvals_reviewers = ["user"]
);
assert_eq!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::DangerFullAccess),
.permission_profile
.can_set(&profile_from_sandbox_policy(
&SandboxPolicy::DangerFullAccess,
)),
Err(ConstraintError::InvalidValue {
field_name: "sandbox_mode",
candidate: "DangerFullAccess".into(),
Expand Down Expand Up @@ -1803,7 +1816,7 @@ allowed_approvals_reviewers = ["user"]
Some(source_location.clone())
);
assert_eq!(
requirements.sandbox_policy.source,
requirements.permission_profile.source,
Some(source_location.clone())
);
assert_eq!(
Expand Down Expand Up @@ -1869,8 +1882,10 @@ allowed_approvals_reviewers = ["user"]
);
assert!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::new_read_only_policy())
.permission_profile
.can_set(&profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy()
))
.is_ok()
);

Expand Down Expand Up @@ -1952,25 +1967,30 @@ allowed_approvals_reviewers = ["user"]
let root = if cfg!(windows) { "C:\\repo" } else { "/repo" };
assert!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::new_read_only_policy())
.permission_profile
.can_set(&profile_from_sandbox_policy(
&SandboxPolicy::new_read_only_policy()
))
.is_ok()
);
let workspace_write_policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?],
network_access: false,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
};
assert!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?],
network_access: false,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
})
.permission_profile
.can_set(&profile_from_sandbox_policy(&workspace_write_policy))
.is_ok()
);
assert_eq!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::DangerFullAccess),
.permission_profile
.can_set(&profile_from_sandbox_policy(
&SandboxPolicy::DangerFullAccess,
)),
Err(ConstraintError::InvalidValue {
field_name: "sandbox_mode",
candidate: "DangerFullAccess".into(),
Expand All @@ -1980,10 +2000,12 @@ allowed_approvals_reviewers = ["user"]
);
assert_eq!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Restricted,
}),
.permission_profile
.can_set(&profile_from_sandbox_policy(
&SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Restricted,
}
)),
Err(ConstraintError::InvalidValue {
field_name: "sandbox_mode",
candidate: "ExternalSandbox".into(),
Expand Down Expand Up @@ -2064,21 +2086,24 @@ allowed_approvals_reviewers = ["user"]

let requirements = ConfigRequirements::try_from(requirements_with_sources)?;
let root = if cfg!(windows) { "C:\\repo" } else { "/repo" };
let workspace_write_policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?],
network_access: false,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
};
assert!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?],
network_access: false,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
})
.permission_profile
.can_set(&profile_from_sandbox_policy(&workspace_write_policy))
.is_ok()
);
assert_eq!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::DangerFullAccess),
.permission_profile
.can_set(&profile_from_sandbox_policy(
&SandboxPolicy::DangerFullAccess,
)),
Err(ConstraintError::InvalidValue {
field_name: "sandbox_mode",
candidate: "DangerFullAccess".into(),
Expand Down Expand Up @@ -2108,8 +2133,10 @@ allowed_approvals_reviewers = ["user"]

assert_eq!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::DangerFullAccess),
.permission_profile
.can_set(&profile_from_sandbox_policy(
&SandboxPolicy::DangerFullAccess,
)),
Err(ConstraintError::InvalidValue {
field_name: "sandbox_mode",
candidate: "DangerFullAccess".into(),
Expand Down Expand Up @@ -2147,8 +2174,10 @@ allowed_approvals_reviewers = ["user"]

assert_eq!(
requirements
.sandbox_policy
.can_set(&SandboxPolicy::new_workspace_write_policy()),
.permission_profile
.can_set(&profile_from_sandbox_policy(
&SandboxPolicy::new_workspace_write_policy(),
)),
Err(ConstraintError::InvalidValue {
field_name: "sandbox_mode",
candidate: "WorkspaceWrite".into(),
Expand Down
11 changes: 7 additions & 4 deletions codex-rs/config/src/config_toml.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use codex_protocol::config_types::Verbosity;
use codex_protocol::config_types::WebSearchMode;
use codex_protocol::config_types::WebSearchToolConfig;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
Expand Down Expand Up @@ -647,7 +648,7 @@ impl ConfigToml {
profile_sandbox_mode: Option<SandboxMode>,
windows_sandbox_level: WindowsSandboxLevel,
active_project: Option<&ProjectConfig>,
sandbox_policy_constraint: Option<&crate::Constrained<SandboxPolicy>>,
permission_profile_constraint: Option<&crate::Constrained<PermissionProfile>>,
) -> SandboxPolicy {
let sandbox_mode_was_explicit = sandbox_mode_override.is_some()
|| profile_sandbox_mode.is_some()
Expand Down Expand Up @@ -707,14 +708,16 @@ impl ConfigToml {
downgrade_workspace_write_if_unsupported(&mut sandbox_policy);
}
if !sandbox_mode_was_explicit
&& let Some(constraint) = sandbox_policy_constraint
&& let Err(err) = constraint.can_set(&sandbox_policy)
&& let Some(constraint) = permission_profile_constraint
&& let Err(err) = constraint.can_set(&PermissionProfile::from_legacy_sandbox_policy(
&sandbox_policy,
))
{
tracing::warn!(
error = %err,
"default sandbox policy is disallowed by requirements; falling back to required default"
);
sandbox_policy = constraint.get().clone();
sandbox_policy = SandboxPolicy::new_read_only_policy();
downgrade_workspace_write_if_unsupported(&mut sandbox_policy);
}
sandbox_policy
Expand Down
1 change: 1 addition & 0 deletions codex-rs/config/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ pub use config_requirements::ResidencyRequirement;
pub use config_requirements::SandboxModeRequirement;
pub use config_requirements::Sourced;
pub use config_requirements::WebSearchModeRequirement;
pub use config_requirements::sandbox_mode_requirement_for_permission_profile;
pub use constraint::Constrained;
pub use constraint::ConstraintError;
pub use constraint::ConstraintResult;
Expand Down
Loading
Loading