mirror of
https://github.com/openai/codex.git
synced 2026-05-04 19:36:45 +00:00
Override local apps settings with requirements.toml settings (#14304)
This PR changes app and connector enablement when `requirements.toml` is present locally or via remote configuration. For apps.* entries: - `enabled = false` in `requirements.toml` overrides the user’s local `config.toml` and forces the app to be disabled. - `enabled = true` in `requirements.toml` does not re-enable an app the user has disabled in config.toml. This behavior applies whether or not the user has an explicit entry for that app in `config.toml`. It also applies to cloud-managed policies and configurations when the admin sets the override through `requirements.toml`. Scenarios tested and verified: - Remote managed, user config (present) override - Admin-defined policies & configurations include a connector override: `[apps.<appID>] enabled = false` - User's config.toml has the same connector configured with `enabled = true` - TUI/App should show connector as disabled - Connector should be unavailable for use in the composer - Remote managed, user config (absent) override - Admin-defined policies & configurations include a connector override: `[apps.<appID>] enabled = false` - User's config.toml has no entry for the the same connector - TUI/App should show connector as disabled - Connector should be unavailable for use in the composer - Locally managed, user config (present) override - Local requirements.toml includes a connector override: `[apps.<appID>] enabled = false` - User's config.toml has the same connector configured with `enabled = true` - TUI/App should show connector as disabled - Connector should be unavailable for use in the composer - Locally managed, user config (absent) override - Local requirements.toml includes a connector override: `[apps.<appID>] enabled = false` - User's config.toml has no entry for the the same connector - TUI/App should show connector as disabled - Connector should be unavailable for use in the composer <img width="1446" height="753" alt="image" src="https://github.com/user-attachments/assets/61c714ca-dcca-4952-8ad2-0afc16ff3835" /> <img width="595" height="233" alt="image" src="https://github.com/user-attachments/assets/7c8ab147-8fd7-429a-89fb-591c21c15621" />
This commit is contained in:
@@ -245,6 +245,43 @@ impl FeatureRequirementsToml {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct AppRequirementToml {
|
||||
pub enabled: Option<bool>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct AppsRequirementsToml {
|
||||
#[serde(default, flatten)]
|
||||
pub apps: BTreeMap<String, AppRequirementToml>,
|
||||
}
|
||||
|
||||
impl AppsRequirementsToml {
|
||||
pub fn is_empty(&self) -> bool {
|
||||
self.apps.values().all(|app| app.enabled.is_none())
|
||||
}
|
||||
}
|
||||
|
||||
/// Merge `enabled` configs from a lower-precedence source into an existing higher-precedence set.
|
||||
/// This lets managed sources (for example Cloud/MDM) enforce setting disablement across layers.
|
||||
/// Implemented with AppsRequirementsToml for now, could be abstracted if we have more enablement-style configs in the future.
|
||||
pub(crate) fn merge_enablement_settings_descending(
|
||||
base: &mut AppsRequirementsToml,
|
||||
incoming: AppsRequirementsToml,
|
||||
) {
|
||||
for (app_id, incoming_requirement) in incoming.apps {
|
||||
let base_requirement = base.apps.entry(app_id).or_default();
|
||||
let higher_precedence = base_requirement.enabled;
|
||||
let lower_precedence = incoming_requirement.enabled;
|
||||
base_requirement.enabled =
|
||||
if higher_precedence == Some(false) || lower_precedence == Some(false) {
|
||||
Some(false)
|
||||
} else {
|
||||
higher_precedence.or(lower_precedence)
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
/// Base config deserialized from system `requirements.toml` or MDM.
|
||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq)]
|
||||
pub struct ConfigRequirementsToml {
|
||||
@@ -254,6 +291,7 @@ pub struct ConfigRequirementsToml {
|
||||
#[serde(rename = "features", alias = "feature_requirements")]
|
||||
pub feature_requirements: Option<FeatureRequirementsToml>,
|
||||
pub mcp_servers: Option<BTreeMap<String, McpServerRequirement>>,
|
||||
pub apps: Option<AppsRequirementsToml>,
|
||||
pub rules: Option<RequirementsExecPolicyToml>,
|
||||
pub enforce_residency: Option<ResidencyRequirement>,
|
||||
#[serde(rename = "experimental_network")]
|
||||
@@ -289,6 +327,7 @@ pub struct ConfigRequirementsWithSources {
|
||||
pub allowed_web_search_modes: Option<Sourced<Vec<WebSearchModeRequirement>>>,
|
||||
pub feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
|
||||
pub mcp_servers: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
pub apps: Option<Sourced<AppsRequirementsToml>>,
|
||||
pub rules: Option<Sourced<RequirementsExecPolicyToml>>,
|
||||
pub enforce_residency: Option<Sourced<ResidencyRequirement>>,
|
||||
pub network: Option<Sourced<NetworkRequirementsToml>>,
|
||||
@@ -300,10 +339,6 @@ impl ConfigRequirementsWithSources {
|
||||
// in `self` is `None`, copy the value from `other` into `self`.
|
||||
macro_rules! fill_missing_take {
|
||||
($base:expr, $other:expr, $source:expr, { $($field:ident),+ $(,)? }) => {
|
||||
// Destructure without `..` so adding fields to `ConfigRequirementsToml`
|
||||
// forces this merge logic to be updated.
|
||||
let ConfigRequirementsToml { $($field: _,)+ } = &$other;
|
||||
|
||||
$(
|
||||
if $base.$field.is_none()
|
||||
&& let Some(value) = $other.$field.take()
|
||||
@@ -314,6 +349,20 @@ impl ConfigRequirementsWithSources {
|
||||
};
|
||||
}
|
||||
|
||||
// Destructure without `..` so adding fields to `ConfigRequirementsToml`
|
||||
// forces this merge logic to be updated.
|
||||
let ConfigRequirementsToml {
|
||||
allowed_approval_policies: _,
|
||||
allowed_sandbox_modes: _,
|
||||
allowed_web_search_modes: _,
|
||||
feature_requirements: _,
|
||||
mcp_servers: _,
|
||||
apps: _,
|
||||
rules: _,
|
||||
enforce_residency: _,
|
||||
network: _,
|
||||
} = &other;
|
||||
|
||||
let mut other = other;
|
||||
fill_missing_take!(
|
||||
self,
|
||||
@@ -330,6 +379,14 @@ impl ConfigRequirementsWithSources {
|
||||
network,
|
||||
}
|
||||
);
|
||||
|
||||
if let Some(incoming_apps) = other.apps.take() {
|
||||
if let Some(existing_apps) = self.apps.as_mut() {
|
||||
merge_enablement_settings_descending(&mut existing_apps.value, incoming_apps);
|
||||
} else {
|
||||
self.apps = Some(Sourced::new(incoming_apps, source));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn into_toml(self) -> ConfigRequirementsToml {
|
||||
@@ -339,6 +396,7 @@ impl ConfigRequirementsWithSources {
|
||||
allowed_web_search_modes,
|
||||
feature_requirements,
|
||||
mcp_servers,
|
||||
apps,
|
||||
rules,
|
||||
enforce_residency,
|
||||
network,
|
||||
@@ -349,6 +407,7 @@ impl ConfigRequirementsWithSources {
|
||||
allowed_web_search_modes: allowed_web_search_modes.map(|sourced| sourced.value),
|
||||
feature_requirements: feature_requirements.map(|sourced| sourced.value),
|
||||
mcp_servers: mcp_servers.map(|sourced| sourced.value),
|
||||
apps: apps.map(|sourced| sourced.value),
|
||||
rules: rules.map(|sourced| sourced.value),
|
||||
enforce_residency: enforce_residency.map(|sourced| sourced.value),
|
||||
network: network.map(|sourced| sourced.value),
|
||||
@@ -399,6 +458,10 @@ impl ConfigRequirementsToml {
|
||||
.as_ref()
|
||||
.is_none_or(FeatureRequirementsToml::is_empty)
|
||||
&& self.mcp_servers.is_none()
|
||||
&& self
|
||||
.apps
|
||||
.as_ref()
|
||||
.is_none_or(AppsRequirementsToml::is_empty)
|
||||
&& self.rules.is_none()
|
||||
&& self.enforce_residency.is_none()
|
||||
&& self.network.is_none()
|
||||
@@ -415,6 +478,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
allowed_web_search_modes,
|
||||
feature_requirements,
|
||||
mcp_servers,
|
||||
apps: _apps,
|
||||
rules,
|
||||
enforce_residency,
|
||||
network,
|
||||
@@ -622,6 +686,7 @@ mod tests {
|
||||
allowed_web_search_modes,
|
||||
feature_requirements,
|
||||
mcp_servers,
|
||||
apps,
|
||||
rules,
|
||||
enforce_residency,
|
||||
network,
|
||||
@@ -636,6 +701,7 @@ mod tests {
|
||||
feature_requirements: feature_requirements
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
mcp_servers: mcp_servers.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
apps: apps.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
rules: rules.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
enforce_residency: enforce_residency
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
@@ -671,6 +737,7 @@ mod tests {
|
||||
allowed_web_search_modes: Some(allowed_web_search_modes.clone()),
|
||||
feature_requirements: Some(feature_requirements.clone()),
|
||||
mcp_servers: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
enforce_residency: Some(enforce_residency),
|
||||
network: None,
|
||||
@@ -695,6 +762,7 @@ mod tests {
|
||||
enforce_source.clone(),
|
||||
)),
|
||||
mcp_servers: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
enforce_residency: Some(Sourced::new(enforce_residency, enforce_source)),
|
||||
network: None,
|
||||
@@ -728,6 +796,7 @@ mod tests {
|
||||
allowed_web_search_modes: None,
|
||||
feature_requirements: None,
|
||||
mcp_servers: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
@@ -769,6 +838,7 @@ mod tests {
|
||||
allowed_web_search_modes: None,
|
||||
feature_requirements: None,
|
||||
mcp_servers: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
@@ -777,6 +847,174 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_apps_requirements() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
[apps.connector_123123]
|
||||
enabled = false
|
||||
"#;
|
||||
let requirements: ConfigRequirementsToml = from_str(toml_str)?;
|
||||
|
||||
assert_eq!(
|
||||
requirements.apps,
|
||||
Some(AppsRequirementsToml {
|
||||
apps: BTreeMap::from([(
|
||||
"connector_123123".to_string(),
|
||||
AppRequirementToml {
|
||||
enabled: Some(false),
|
||||
},
|
||||
)]),
|
||||
})
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn apps_requirements(entries: &[(&str, Option<bool>)]) -> AppsRequirementsToml {
|
||||
AppsRequirementsToml {
|
||||
apps: entries
|
||||
.iter()
|
||||
.map(|(app_id, enabled)| {
|
||||
(
|
||||
(*app_id).to_string(),
|
||||
AppRequirementToml { enabled: *enabled },
|
||||
)
|
||||
})
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_enablement_settings_descending_unions_distinct_apps() {
|
||||
let mut merged = apps_requirements(&[("connector_high", Some(false))]);
|
||||
let lower = apps_requirements(&[("connector_low", Some(true))]);
|
||||
|
||||
merge_enablement_settings_descending(&mut merged, lower);
|
||||
|
||||
assert_eq!(
|
||||
merged,
|
||||
apps_requirements(&[
|
||||
("connector_high", Some(false)),
|
||||
("connector_low", Some(true))
|
||||
]),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_enablement_settings_descending_prefers_false_from_lower_precedence() {
|
||||
let mut merged = apps_requirements(&[("connector_123123", Some(true))]);
|
||||
let lower = apps_requirements(&[("connector_123123", Some(false))]);
|
||||
|
||||
merge_enablement_settings_descending(&mut merged, lower);
|
||||
|
||||
assert_eq!(
|
||||
merged,
|
||||
apps_requirements(&[("connector_123123", Some(false))]),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_enablement_settings_descending_keeps_higher_true_when_lower_is_unset() {
|
||||
let mut merged = apps_requirements(&[("connector_123123", Some(true))]);
|
||||
let lower = apps_requirements(&[("connector_123123", None)]);
|
||||
|
||||
merge_enablement_settings_descending(&mut merged, lower);
|
||||
|
||||
assert_eq!(
|
||||
merged,
|
||||
apps_requirements(&[("connector_123123", Some(true))]),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_enablement_settings_descending_uses_lower_value_when_higher_missing() {
|
||||
let mut merged = apps_requirements(&[]);
|
||||
let lower = apps_requirements(&[("connector_123123", Some(true))]);
|
||||
|
||||
merge_enablement_settings_descending(&mut merged, lower);
|
||||
|
||||
assert_eq!(
|
||||
merged,
|
||||
apps_requirements(&[("connector_123123", Some(true))]),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_enablement_settings_descending_preserves_higher_false_when_lower_missing_app() {
|
||||
let mut merged = apps_requirements(&[("connector_123123", Some(false))]);
|
||||
let lower = apps_requirements(&[]);
|
||||
|
||||
merge_enablement_settings_descending(&mut merged, lower);
|
||||
|
||||
assert_eq!(
|
||||
merged,
|
||||
apps_requirements(&[("connector_123123", Some(false))]),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_unset_fields_merges_apps_across_sources_with_enabled_evaluation() {
|
||||
let higher_source = RequirementSource::CloudRequirements;
|
||||
let lower_source = RequirementSource::LegacyManagedConfigTomlFromMdm;
|
||||
let mut target = ConfigRequirementsWithSources::default();
|
||||
|
||||
target.merge_unset_fields(
|
||||
higher_source.clone(),
|
||||
ConfigRequirementsToml {
|
||||
apps: Some(apps_requirements(&[
|
||||
("connector_high", Some(true)),
|
||||
("connector_shared", Some(true)),
|
||||
])),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
target.merge_unset_fields(
|
||||
lower_source,
|
||||
ConfigRequirementsToml {
|
||||
apps: Some(apps_requirements(&[
|
||||
("connector_low", Some(false)),
|
||||
("connector_shared", Some(false)),
|
||||
])),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
|
||||
let apps = target.apps.expect("apps should be present");
|
||||
assert_eq!(
|
||||
apps.value,
|
||||
apps_requirements(&[
|
||||
("connector_high", Some(true)),
|
||||
("connector_low", Some(false)),
|
||||
("connector_shared", Some(false)),
|
||||
])
|
||||
);
|
||||
assert_eq!(apps.source, higher_source);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_unset_fields_apps_empty_higher_source_does_not_block_lower_disables() {
|
||||
let mut target = ConfigRequirementsWithSources::default();
|
||||
|
||||
target.merge_unset_fields(
|
||||
RequirementSource::CloudRequirements,
|
||||
ConfigRequirementsToml {
|
||||
apps: Some(apps_requirements(&[])),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
target.merge_unset_fields(
|
||||
RequirementSource::LegacyManagedConfigTomlFromMdm,
|
||||
ConfigRequirementsToml {
|
||||
apps: Some(apps_requirements(&[("connector_123123", Some(false))])),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
target.apps.map(|apps| apps.value),
|
||||
Some(apps_requirements(&[("connector_123123", Some(false))])),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn constraint_error_includes_requirement_source() -> Result<()> {
|
||||
let source: ConfigRequirementsToml = from_str(
|
||||
|
||||
Reference in New Issue
Block a user