mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
permissions: enforce managed permission profile allowlists (#24852)
## Why Permission profile allowlists are an enterprise security boundary, but they also need to compose across the managed requirements layers added in #24620. A map representation lets each requirements layer add, allow, or revoke individual profiles without replacing an entire array. ## Managed Contract Administrators configure the mergeable allow map with `allowed_permission_profiles`. A recommended enterprise configuration explicitly lists every built-in and custom profile users should be able to select: ```toml default_permissions = "review_only" [allowed_permission_profiles] ":read-only" = true ":workspace" = true review_only = true # ":danger-full-access" is intentionally omitted, so it is denied. [permissions.review_only] extends = ":read-only" ``` - Profiles whose effective merged value is `true` are allowed. - Missing profiles and profiles set to `false` are denied. - This is a closed allowlist: built-in profiles and profiles introduced in future versions are denied unless explicitly allowed. - Explicitly list each built-in profile the enterprise wants to make available. Omit built-ins such as `:danger-full-access` when they should remain unavailable. - Set `default_permissions` explicitly to the allowed profile users should receive when they have no local selection. - Higher-precedence layers override only the profile keys they define. - `false` is only needed when a higher-precedence layer must revoke a `true` inherited from a lower layer. - Explicit keys must refer to known built-in or managed profiles. A custom or narrowed allowlist requires an allowed `default_permissions`. For compatibility, if both `:workspace` and `:read-only` are explicitly allowed, an omitted default resolves to `:workspace`; customer configurations should still set the intended default explicitly. When `allowed_permission_profiles` is absent, existing implicit permission and legacy `sandbox_mode` behavior is unchanged. ## What Changed - Add `allowed_permission_profiles` as a `BTreeMap<String, bool>` that merges per profile across requirements layers. - Enforce managed defaults, strict denial of omitted profiles, and the explicitly allowed standard-pair fallback. - Expose `allowedPermissionProfiles` through `configRequirements/read` and regenerate its schemas. - Add regression coverage for map composition and revocation, managed defaults, strict denial of omitted built-ins, and API output. ## Verification - Focused `codex-config` coverage for layered map composition and revocation - Focused `codex-core` coverage for managed defaults, invalid defaults, strict denial of omitted built-ins, and the standard built-in pair - Focused `codex-app-server` coverage for requirements API output - Scoped Clippy for `codex-config`, `codex-core`, `codex-app-server-protocol`, and `codex-app-server` ## Documentation The managed `requirements.toml` documentation should introduce `allowed_permission_profiles` as a closed permission-profile allowlist before this setting is published on developers.openai.com. --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
954e2878bb
commit
2f108f9fd9
@@ -822,7 +822,8 @@ pub struct ConfigRequirementsToml {
|
||||
pub allowed_approval_policies: Option<Vec<AskForApproval>>,
|
||||
pub allowed_approvals_reviewers: Option<Vec<ApprovalsReviewer>>,
|
||||
pub allowed_sandbox_modes: Option<Vec<SandboxModeRequirement>>,
|
||||
pub allowed_permissions: Option<Vec<String>>,
|
||||
pub allowed_permission_profiles: Option<BTreeMap<String, bool>>,
|
||||
pub default_permissions: Option<String>,
|
||||
pub remote_sandbox_config: Option<Vec<RemoteSandboxConfigToml>>,
|
||||
pub allowed_web_search_modes: Option<Vec<WebSearchModeRequirement>>,
|
||||
pub allow_managed_hooks_only: Option<bool>,
|
||||
@@ -876,7 +877,8 @@ pub struct ConfigRequirementsWithSources {
|
||||
pub allowed_approval_policies: Option<Sourced<Vec<AskForApproval>>>,
|
||||
pub allowed_approvals_reviewers: Option<Sourced<Vec<ApprovalsReviewer>>>,
|
||||
pub allowed_sandbox_modes: Option<Sourced<Vec<SandboxModeRequirement>>>,
|
||||
pub allowed_permissions: Option<Sourced<Vec<String>>>,
|
||||
pub allowed_permission_profiles: Option<Sourced<BTreeMap<String, bool>>>,
|
||||
pub default_permissions: Option<Sourced<String>>,
|
||||
pub allowed_web_search_modes: Option<Sourced<Vec<WebSearchModeRequirement>>>,
|
||||
pub allow_managed_hooks_only: Option<Sourced<bool>>,
|
||||
pub allow_appshots: Option<Sourced<bool>>,
|
||||
@@ -916,7 +918,8 @@ impl ConfigRequirementsWithSources {
|
||||
allowed_approval_policies: _,
|
||||
allowed_approvals_reviewers: _,
|
||||
allowed_sandbox_modes: _,
|
||||
allowed_permissions: _,
|
||||
allowed_permission_profiles: _,
|
||||
default_permissions: _,
|
||||
remote_sandbox_config: _,
|
||||
allowed_web_search_modes: _,
|
||||
allow_managed_hooks_only: _,
|
||||
@@ -951,7 +954,8 @@ impl ConfigRequirementsWithSources {
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
allowed_sandbox_modes,
|
||||
allowed_permissions,
|
||||
allowed_permission_profiles,
|
||||
default_permissions,
|
||||
allowed_web_search_modes,
|
||||
allow_managed_hooks_only,
|
||||
allow_appshots,
|
||||
@@ -983,7 +987,8 @@ impl ConfigRequirementsWithSources {
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
allowed_sandbox_modes,
|
||||
allowed_permissions,
|
||||
allowed_permission_profiles,
|
||||
default_permissions,
|
||||
allowed_web_search_modes,
|
||||
allow_managed_hooks_only,
|
||||
allow_appshots,
|
||||
@@ -1004,7 +1009,8 @@ impl ConfigRequirementsWithSources {
|
||||
allowed_approval_policies: allowed_approval_policies.map(|sourced| sourced.value),
|
||||
allowed_approvals_reviewers: allowed_approvals_reviewers.map(|sourced| sourced.value),
|
||||
allowed_sandbox_modes: allowed_sandbox_modes.map(|sourced| sourced.value),
|
||||
allowed_permissions: allowed_permissions.map(|sourced| sourced.value),
|
||||
allowed_permission_profiles: allowed_permission_profiles.map(|sourced| sourced.value),
|
||||
default_permissions: default_permissions.map(|sourced| sourced.value),
|
||||
remote_sandbox_config: None,
|
||||
allowed_web_search_modes: allowed_web_search_modes.map(|sourced| sourced.value),
|
||||
allow_managed_hooks_only: allow_managed_hooks_only.map(|sourced| sourced.value),
|
||||
@@ -1092,7 +1098,8 @@ impl ConfigRequirementsToml {
|
||||
self.allowed_approval_policies.is_none()
|
||||
&& self.allowed_approvals_reviewers.is_none()
|
||||
&& self.allowed_sandbox_modes.is_none()
|
||||
&& self.allowed_permissions.is_none()
|
||||
&& self.allowed_permission_profiles.is_none()
|
||||
&& self.default_permissions.is_none()
|
||||
&& self.remote_sandbox_config.is_none()
|
||||
&& self.allowed_web_search_modes.is_none()
|
||||
&& self.allow_managed_hooks_only.is_none()
|
||||
@@ -1144,7 +1151,8 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
allowed_sandbox_modes,
|
||||
allowed_permissions: _,
|
||||
allowed_permission_profiles: _,
|
||||
default_permissions: _,
|
||||
allowed_web_search_modes,
|
||||
allow_managed_hooks_only,
|
||||
allow_appshots,
|
||||
@@ -1511,7 +1519,8 @@ mod tests {
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
allowed_sandbox_modes,
|
||||
allowed_permissions,
|
||||
allowed_permission_profiles,
|
||||
default_permissions,
|
||||
remote_sandbox_config: _,
|
||||
allowed_web_search_modes,
|
||||
allow_managed_hooks_only,
|
||||
@@ -1536,7 +1545,9 @@ mod tests {
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
allowed_sandbox_modes: allowed_sandbox_modes
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
allowed_permissions: allowed_permissions
|
||||
allowed_permission_profiles: allowed_permission_profiles
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
default_permissions: default_permissions
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
allowed_web_search_modes: allowed_web_search_modes
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
@@ -1592,7 +1603,11 @@ mod tests {
|
||||
fn deserialize_managed_permission_profiles() -> Result<()> {
|
||||
let requirements: ConfigRequirementsToml = from_str(
|
||||
r#"
|
||||
allowed_permissions = ["managed-standard", "managed-build"]
|
||||
default_permissions = "managed-standard"
|
||||
|
||||
[allowed_permission_profiles]
|
||||
managed-standard = true
|
||||
managed-build = true
|
||||
|
||||
[permissions.managed-standard]
|
||||
extends = ":workspace"
|
||||
@@ -1603,11 +1618,15 @@ mod tests {
|
||||
)?;
|
||||
|
||||
assert_eq!(
|
||||
requirements.allowed_permissions,
|
||||
Some(vec![
|
||||
"managed-standard".to_string(),
|
||||
"managed-build".to_string(),
|
||||
])
|
||||
requirements.allowed_permission_profiles,
|
||||
Some(BTreeMap::from([
|
||||
("managed-build".to_string(), true),
|
||||
("managed-standard".to_string(), true),
|
||||
]))
|
||||
);
|
||||
assert_eq!(
|
||||
requirements.default_permissions,
|
||||
Some("managed-standard".to_string())
|
||||
);
|
||||
let permissions = requirements
|
||||
.permissions
|
||||
@@ -1720,7 +1739,8 @@ mod tests {
|
||||
allowed_approval_policies: Some(allowed_approval_policies.clone()),
|
||||
allowed_approvals_reviewers: Some(allowed_approvals_reviewers.clone()),
|
||||
allowed_sandbox_modes: Some(allowed_sandbox_modes.clone()),
|
||||
allowed_permissions: Some(vec!["managed".to_string()]),
|
||||
allowed_permission_profiles: Some(BTreeMap::from([("managed".to_string(), true)])),
|
||||
default_permissions: Some("managed".to_string()),
|
||||
remote_sandbox_config: None,
|
||||
allowed_web_search_modes: Some(allowed_web_search_modes.clone()),
|
||||
allow_managed_hooks_only: Some(true),
|
||||
@@ -1753,10 +1773,11 @@ mod tests {
|
||||
source.clone(),
|
||||
)),
|
||||
allowed_sandbox_modes: Some(Sourced::new(allowed_sandbox_modes, source.clone(),)),
|
||||
allowed_permissions: Some(Sourced::new(
|
||||
vec!["managed".to_string()],
|
||||
allowed_permission_profiles: Some(Sourced::new(
|
||||
BTreeMap::from([("managed".to_string(), true)]),
|
||||
source.clone(),
|
||||
)),
|
||||
default_permissions: Some(Sourced::new("managed".to_string(), source.clone(),)),
|
||||
allowed_web_search_modes: Some(Sourced::new(
|
||||
allowed_web_search_modes,
|
||||
enforce_source.clone(),
|
||||
@@ -1809,7 +1830,8 @@ mod tests {
|
||||
)),
|
||||
allowed_approvals_reviewers: None,
|
||||
allowed_sandbox_modes: None,
|
||||
allowed_permissions: None,
|
||||
allowed_permission_profiles: None,
|
||||
default_permissions: None,
|
||||
allowed_web_search_modes: None,
|
||||
allow_managed_hooks_only: None,
|
||||
allow_appshots: None,
|
||||
@@ -1861,7 +1883,8 @@ mod tests {
|
||||
)),
|
||||
allowed_approvals_reviewers: None,
|
||||
allowed_sandbox_modes: None,
|
||||
allowed_permissions: None,
|
||||
allowed_permission_profiles: None,
|
||||
default_permissions: None,
|
||||
allowed_web_search_modes: None,
|
||||
allow_managed_hooks_only: None,
|
||||
allow_appshots: None,
|
||||
|
||||
@@ -176,7 +176,8 @@ fn populate_merged_regular_fields_with_sources(
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
allowed_sandbox_modes,
|
||||
allowed_permissions,
|
||||
allowed_permission_profiles,
|
||||
default_permissions,
|
||||
remote_sandbox_config: _,
|
||||
allowed_web_search_modes,
|
||||
allow_managed_hooks_only,
|
||||
@@ -201,7 +202,11 @@ fn populate_merged_regular_fields_with_sources(
|
||||
&["allowed_approvals_reviewers"]
|
||||
);
|
||||
set_sourced!(allowed_sandbox_modes, &["allowed_sandbox_modes"]);
|
||||
set_sourced!(allowed_permissions, &["allowed_permissions"]);
|
||||
set_sourced!(
|
||||
allowed_permission_profiles,
|
||||
&["allowed_permission_profiles"]
|
||||
);
|
||||
set_sourced!(default_permissions, &["default_permissions"]);
|
||||
set_sourced!(allowed_web_search_modes, &["allowed_web_search_modes"]);
|
||||
set_sourced!(allow_managed_hooks_only, &["allow_managed_hooks_only"]);
|
||||
set_sourced!(allow_appshots, &["allow_appshots"]);
|
||||
|
||||
@@ -62,6 +62,11 @@ fn top_level_values_use_toml_priority() {
|
||||
r#"
|
||||
allowed_approval_policies = ["on-request"]
|
||||
allowed_sandbox_modes = ["workspace-write"]
|
||||
default_permissions = ":workspace"
|
||||
|
||||
[allowed_permission_profiles]
|
||||
":read-only" = true
|
||||
":workspace" = true
|
||||
"#,
|
||||
),
|
||||
layer(
|
||||
@@ -70,6 +75,11 @@ allowed_sandbox_modes = ["workspace-write"]
|
||||
r#"
|
||||
allowed_approval_policies = ["never"]
|
||||
allowed_sandbox_modes = ["read-only"]
|
||||
default_permissions = ":read-only"
|
||||
|
||||
[allowed_permission_profiles]
|
||||
":danger-full-access" = false
|
||||
":workspace" = false
|
||||
"#,
|
||||
),
|
||||
])
|
||||
@@ -82,6 +92,12 @@ allowed_sandbox_modes = ["read-only"]
|
||||
r#"
|
||||
allowed_approval_policies = ["never"]
|
||||
allowed_sandbox_modes = ["read-only"]
|
||||
default_permissions = ":read-only"
|
||||
|
||||
[allowed_permission_profiles]
|
||||
":danger-full-access" = false
|
||||
":read-only" = true
|
||||
":workspace" = false
|
||||
"#
|
||||
)
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user