mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Enforce managed shell environment policy
This commit is contained in:
@@ -1,5 +1,7 @@
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::config_types::SandboxMode;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicyInherit;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicyRule;
|
||||
use codex_protocol::config_types::WebSearchMode;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -10,6 +12,7 @@ use serde::de::Error as _;
|
||||
use serde::de::value::Error as ValueDeserializerError;
|
||||
use serde::de::value::StrDeserializer;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::fmt;
|
||||
use wildmatch::WildMatchPattern;
|
||||
|
||||
@@ -155,6 +158,7 @@ pub struct ConfigRequirements {
|
||||
pub check_for_update_on_startup: Option<Sourced<bool>>,
|
||||
pub otel: Option<Sourced<OtelConfigToml>>,
|
||||
pub allow_login_shell: Option<Sourced<bool>>,
|
||||
pub shell_environment_policy: Option<Sourced<ShellEnvironmentPolicyRequirementsToml>>,
|
||||
pub feedback: Option<Sourced<FeedbackConfigToml>>,
|
||||
pub approval_policy: ConstrainedWithSource<AskForApproval>,
|
||||
pub approvals_reviewer: ConstrainedWithSource<ApprovalsReviewer>,
|
||||
@@ -193,6 +197,7 @@ impl Default for ConfigRequirements {
|
||||
check_for_update_on_startup: None,
|
||||
otel: None,
|
||||
allow_login_shell: None,
|
||||
shell_environment_policy: None,
|
||||
feedback: None,
|
||||
approval_policy: ConstrainedWithSource::new(
|
||||
Constrained::allow_any_from_default(),
|
||||
@@ -859,6 +864,7 @@ pub struct ConfigRequirementsToml {
|
||||
pub check_for_update_on_startup: Option<bool>,
|
||||
pub otel: Option<OtelConfigToml>,
|
||||
pub allow_login_shell: Option<bool>,
|
||||
pub shell_environment_policy: Option<ShellEnvironmentPolicyRequirementsToml>,
|
||||
pub feedback: Option<FeedbackConfigToml>,
|
||||
pub allowed_approval_policies: Option<Vec<AskForApproval>>,
|
||||
pub allowed_approvals_reviewers: Option<Vec<ApprovalsReviewer>>,
|
||||
@@ -886,6 +892,22 @@ pub struct ConfigRequirementsToml {
|
||||
pub guardian_policy_config: Option<String>,
|
||||
}
|
||||
|
||||
/// Managed shell environment policy fields accepted by `requirements.toml`.
|
||||
///
|
||||
/// Unlike ordinary `config.toml`, requirements intentionally accept only the
|
||||
/// keyed `rules` form. This makes include/exclude actions compose by pattern,
|
||||
/// while config arrays remain a migration-only compatibility path that can be
|
||||
/// deprecated independently later.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct ShellEnvironmentPolicyRequirementsToml {
|
||||
pub inherit: Option<ShellEnvironmentPolicyInherit>,
|
||||
pub ignore_default_excludes: Option<bool>,
|
||||
pub r#set: Option<HashMap<String, String>>,
|
||||
pub rules: Option<BTreeMap<String, ShellEnvironmentPolicyRule>>,
|
||||
pub experimental_use_profile: Option<bool>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone, PartialEq)]
|
||||
pub struct RemoteSandboxConfigToml {
|
||||
pub hostname_patterns: Vec<String>,
|
||||
@@ -926,6 +948,7 @@ pub struct ConfigRequirementsWithSources {
|
||||
pub check_for_update_on_startup: Option<Sourced<bool>>,
|
||||
pub otel: Option<Sourced<OtelConfigToml>>,
|
||||
pub allow_login_shell: Option<Sourced<bool>>,
|
||||
pub shell_environment_policy: Option<Sourced<ShellEnvironmentPolicyRequirementsToml>>,
|
||||
pub feedback: Option<Sourced<FeedbackConfigToml>>,
|
||||
pub allowed_approval_policies: Option<Sourced<Vec<AskForApproval>>>,
|
||||
pub allowed_approvals_reviewers: Option<Sourced<Vec<ApprovalsReviewer>>>,
|
||||
@@ -979,6 +1002,7 @@ impl ConfigRequirementsWithSources {
|
||||
check_for_update_on_startup: _,
|
||||
otel: _,
|
||||
allow_login_shell: _,
|
||||
shell_environment_policy: _,
|
||||
feedback: _,
|
||||
allowed_approval_policies: _,
|
||||
allowed_approvals_reviewers: _,
|
||||
@@ -1027,6 +1051,7 @@ impl ConfigRequirementsWithSources {
|
||||
check_for_update_on_startup,
|
||||
otel,
|
||||
allow_login_shell,
|
||||
shell_environment_policy,
|
||||
feedback,
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
@@ -1072,6 +1097,7 @@ impl ConfigRequirementsWithSources {
|
||||
check_for_update_on_startup,
|
||||
otel,
|
||||
allow_login_shell,
|
||||
shell_environment_policy,
|
||||
feedback,
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
@@ -1106,6 +1132,7 @@ impl ConfigRequirementsWithSources {
|
||||
check_for_update_on_startup: check_for_update_on_startup.map(|sourced| sourced.value),
|
||||
otel: otel.map(|sourced| sourced.value),
|
||||
allow_login_shell: allow_login_shell.map(|sourced| sourced.value),
|
||||
shell_environment_policy: shell_environment_policy.map(|sourced| sourced.value),
|
||||
feedback: feedback.map(|sourced| sourced.value),
|
||||
allowed_approval_policies: allowed_approval_policies.map(|sourced| sourced.value),
|
||||
allowed_approvals_reviewers: allowed_approvals_reviewers.map(|sourced| sourced.value),
|
||||
@@ -1210,6 +1237,10 @@ impl ConfigRequirementsToml {
|
||||
.as_ref()
|
||||
.is_none_or(|otel| otel == &OtelConfigToml::default())
|
||||
&& self.allow_login_shell.is_none()
|
||||
&& self
|
||||
.shell_environment_policy
|
||||
.as_ref()
|
||||
.is_none_or(|policy| policy == &ShellEnvironmentPolicyRequirementsToml::default())
|
||||
&& self
|
||||
.feedback
|
||||
.as_ref()
|
||||
@@ -1278,6 +1309,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
check_for_update_on_startup,
|
||||
otel,
|
||||
allow_login_shell,
|
||||
shell_environment_policy,
|
||||
feedback,
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
@@ -1618,6 +1650,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
check_for_update_on_startup,
|
||||
otel,
|
||||
allow_login_shell,
|
||||
shell_environment_policy,
|
||||
feedback,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
@@ -1720,6 +1753,7 @@ mod tests {
|
||||
check_for_update_on_startup,
|
||||
otel,
|
||||
allow_login_shell,
|
||||
shell_environment_policy,
|
||||
feedback,
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
@@ -1762,6 +1796,8 @@ mod tests {
|
||||
otel: otel.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
allow_login_shell: allow_login_shell
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
shell_environment_policy: shell_environment_policy
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
feedback: feedback.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
allowed_approval_policies: allowed_approval_policies
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
@@ -3779,4 +3815,40 @@ command = "python3 /enterprise/hooks/pre.py"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_environment_policy_requires_rules_for_list_fields() -> Result<()> {
|
||||
let requirements: ConfigRequirementsToml = from_str(
|
||||
r#"
|
||||
[shell_environment_policy.rules]
|
||||
"HOME" = "include"
|
||||
"SECRET_*" = "exclude"
|
||||
"#,
|
||||
)?;
|
||||
|
||||
assert_eq!(
|
||||
requirements.shell_environment_policy,
|
||||
Some(ShellEnvironmentPolicyRequirementsToml {
|
||||
rules: Some(BTreeMap::from([
|
||||
("HOME".to_string(), ShellEnvironmentPolicyRule::Include,),
|
||||
("SECRET_*".to_string(), ShellEnvironmentPolicyRule::Exclude,),
|
||||
])),
|
||||
..Default::default()
|
||||
})
|
||||
);
|
||||
|
||||
for field in ["exclude", "include_only"] {
|
||||
let error = from_str::<ConfigRequirementsToml>(&format!(
|
||||
"[shell_environment_policy]\n{field} = [\"LEGACY_*\"]\n"
|
||||
))
|
||||
.expect_err("legacy requirements.toml arrays should be rejected");
|
||||
assert!(error.to_string().contains(field));
|
||||
}
|
||||
let error = from_str::<ConfigRequirementsToml>(
|
||||
"[shell_environment_policy.rules]\n\"PATH\" = \"keep\"\n",
|
||||
)
|
||||
.expect_err("unknown rule actions should be rejected");
|
||||
assert!(error.to_string().contains("keep"));
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -74,6 +74,7 @@ pub use config_requirements::RemoteSandboxConfigToml;
|
||||
pub use config_requirements::RequirementSource;
|
||||
pub use config_requirements::ResidencyRequirement;
|
||||
pub use config_requirements::SandboxModeRequirement;
|
||||
pub use config_requirements::ShellEnvironmentPolicyRequirementsToml;
|
||||
pub use config_requirements::Sourced;
|
||||
pub use config_requirements::WebSearchModeRequirement;
|
||||
pub use config_requirements::WindowsRequirementsToml;
|
||||
|
||||
@@ -184,6 +184,7 @@ fn populate_merged_regular_fields_with_sources(
|
||||
check_for_update_on_startup,
|
||||
otel: _,
|
||||
allow_login_shell,
|
||||
shell_environment_policy,
|
||||
feedback,
|
||||
allowed_approval_policies,
|
||||
allowed_approvals_reviewers,
|
||||
@@ -221,6 +222,7 @@ fn populate_merged_regular_fields_with_sources(
|
||||
&["check_for_update_on_startup"]
|
||||
);
|
||||
set_sourced!(allow_login_shell, &["allow_login_shell"]);
|
||||
set_sourced!(shell_environment_policy, &["shell_environment_policy"]);
|
||||
set_sourced!(feedback, &["feedback"]);
|
||||
set_sourced!(allowed_approval_policies, &["allowed_approval_policies"]);
|
||||
set_sourced!(
|
||||
|
||||
@@ -6,7 +6,9 @@ use super::compose_requirements_for_hostname_and_hook_directory;
|
||||
use crate::ConfigRequirementsToml;
|
||||
use crate::ConfigRequirementsWithSources;
|
||||
use crate::RequirementSource;
|
||||
use crate::ShellEnvironmentPolicyRequirementsToml;
|
||||
use crate::Sourced;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicyRule;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -1074,3 +1076,53 @@ shared = "high"
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_environment_rules_compose_by_pattern() {
|
||||
let high_source = RequirementSource::EnterpriseManaged {
|
||||
id: "req_high".to_string(),
|
||||
name: "High".to_string(),
|
||||
};
|
||||
let low_source = RequirementSource::EnterpriseManaged {
|
||||
id: "req_low".to_string(),
|
||||
name: "Low".to_string(),
|
||||
};
|
||||
let composed = compose_requirements_for_hostname(
|
||||
vec![
|
||||
RequirementsLayerEntry::from_toml(
|
||||
low_source.clone(),
|
||||
r#"
|
||||
[shell_environment_policy.rules]
|
||||
"FLIP_*" = "exclude"
|
||||
"KEEP_*" = "include"
|
||||
"#,
|
||||
),
|
||||
RequirementsLayerEntry::from_toml(
|
||||
high_source.clone(),
|
||||
r#"
|
||||
[shell_environment_policy.rules]
|
||||
"ADD_*" = "exclude"
|
||||
"FLIP_*" = "include"
|
||||
"#,
|
||||
),
|
||||
],
|
||||
/*hostname*/ None,
|
||||
)
|
||||
.expect("compose requirements")
|
||||
.expect("requirements present");
|
||||
|
||||
assert_eq!(
|
||||
composed.shell_environment_policy,
|
||||
Some(Sourced::new(
|
||||
ShellEnvironmentPolicyRequirementsToml {
|
||||
rules: Some(BTreeMap::from([
|
||||
("ADD_*".to_string(), ShellEnvironmentPolicyRule::Exclude,),
|
||||
("FLIP_*".to_string(), ShellEnvironmentPolicyRule::Include,),
|
||||
("KEEP_*".to_string(), ShellEnvironmentPolicyRule::Include,),
|
||||
])),
|
||||
..Default::default()
|
||||
},
|
||||
RequirementSource::composite([high_source, low_source]),
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -8340,6 +8340,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset()
|
||||
check_for_update_on_startup: None,
|
||||
otel: None,
|
||||
allow_login_shell: None,
|
||||
shell_environment_policy: None,
|
||||
feedback: None,
|
||||
allowed_approval_policies: None,
|
||||
allowed_approvals_reviewers: None,
|
||||
@@ -11170,3 +11171,61 @@ zz_overflow = {configured_overflow:?}
|
||||
}));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_shell_environment_policy_overrides_matching_patterns() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"
|
||||
[shell_environment_policy]
|
||||
ignore_default_excludes = true
|
||||
exclude = ["USER_*", "FLIP_TO_INCLUDE"]
|
||||
include_only = ["HOME", "FLIP_TO_EXCLUDE"]
|
||||
|
||||
[shell_environment_policy.set]
|
||||
CONFIGURED = "kept"
|
||||
MANAGED = "old"
|
||||
"#,
|
||||
)?;
|
||||
let config = load_with_enterprise_requirement(
|
||||
&codex_home,
|
||||
r#"
|
||||
[shell_environment_policy]
|
||||
ignore_default_excludes = false
|
||||
experimental_use_profile = true
|
||||
|
||||
[shell_environment_policy.rules]
|
||||
"FLIP_TO_EXCLUDE" = "exclude"
|
||||
"FLIP_TO_INCLUDE" = "include"
|
||||
"MANAGED_*" = "exclude"
|
||||
|
||||
[shell_environment_policy.set]
|
||||
MANAGED = "required"
|
||||
"#,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let expected: codex_protocol::config_types::ShellEnvironmentPolicy =
|
||||
codex_config::types::ShellEnvironmentPolicyToml {
|
||||
ignore_default_excludes: Some(false),
|
||||
exclude: Some(vec![
|
||||
"USER_*".to_string(),
|
||||
"FLIP_TO_EXCLUDE".to_string(),
|
||||
"MANAGED_*".to_string(),
|
||||
]),
|
||||
include_only: Some(vec!["HOME".to_string(), "FLIP_TO_INCLUDE".to_string()]),
|
||||
experimental_use_profile: Some(true),
|
||||
r#set: Some(HashMap::from([
|
||||
("CONFIGURED".to_string(), "kept".to_string()),
|
||||
("MANAGED".to_string(), "required".to_string()),
|
||||
])),
|
||||
..Default::default()
|
||||
}
|
||||
.into();
|
||||
assert_eq!(config.permissions.shell_environment_policy, expected);
|
||||
assert!(config.startup_warnings.iter().any(|warning| {
|
||||
warning.contains("Configured values under `shell_environment_policy` are overridden")
|
||||
}));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -2716,6 +2716,7 @@ impl Config {
|
||||
check_for_update_on_startup: _,
|
||||
otel: _,
|
||||
allow_login_shell: _,
|
||||
shell_environment_policy: _,
|
||||
feedback: _,
|
||||
approval_policy: mut constrained_approval_policy,
|
||||
approvals_reviewer: mut constrained_approvals_reviewer,
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::ShellEnvironmentPolicyRequirementsToml;
|
||||
use codex_config::Sourced;
|
||||
use codex_config::config_toml::ConfigToml;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_config::types::FeedbackConfigToml;
|
||||
use codex_config::types::ShellEnvironmentPolicyToml;
|
||||
use codex_protocol::config_types::ForcedLoginMethod;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicyRule;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use super::otel;
|
||||
|
||||
@@ -42,6 +46,11 @@ pub(super) fn apply_to_config(
|
||||
apply_exact!(model_catalog_json);
|
||||
apply_exact!(check_for_update_on_startup);
|
||||
apply_exact!(allow_login_shell);
|
||||
apply_shell_environment_policy_requirement(
|
||||
&mut config.shell_environment_policy,
|
||||
requirements.shell_environment_policy.as_ref(),
|
||||
startup_warnings,
|
||||
);
|
||||
if let Some(requirement) = requirements.otel.as_ref() {
|
||||
otel::apply_requirement(&mut config.otel, requirement, startup_warnings)?;
|
||||
}
|
||||
@@ -122,6 +131,97 @@ pub(super) fn replace_required_leaf<T: Clone + PartialEq>(
|
||||
conflict
|
||||
}
|
||||
|
||||
fn apply_shell_environment_policy_requirement(
|
||||
configured: &mut ShellEnvironmentPolicyToml,
|
||||
requirement: Option<&Sourced<ShellEnvironmentPolicyRequirementsToml>>,
|
||||
startup_warnings: &mut Vec<String>,
|
||||
) {
|
||||
let Some(Sourced { value, source }) = requirement else {
|
||||
return;
|
||||
};
|
||||
let ShellEnvironmentPolicyRequirementsToml {
|
||||
inherit,
|
||||
ignore_default_excludes,
|
||||
r#set: required_set,
|
||||
rules,
|
||||
experimental_use_profile,
|
||||
} = value;
|
||||
let mut conflict = false;
|
||||
|
||||
conflict |= replace_required_leaf(&mut configured.inherit, inherit);
|
||||
conflict |= replace_required_leaf(
|
||||
&mut configured.ignore_default_excludes,
|
||||
ignore_default_excludes,
|
||||
);
|
||||
conflict |= apply_required_pattern_rules(configured, rules);
|
||||
conflict |= replace_required_leaf(
|
||||
&mut configured.experimental_use_profile,
|
||||
experimental_use_profile,
|
||||
);
|
||||
|
||||
if let Some(required) = required_set.as_ref() {
|
||||
let configured_set = configured.r#set.get_or_insert_default();
|
||||
conflict |= required.iter().any(|(key, value)| {
|
||||
configured_set
|
||||
.get(key)
|
||||
.is_some_and(|current| current != value)
|
||||
});
|
||||
configured_set.extend(required.clone());
|
||||
}
|
||||
|
||||
push_structured_requirement_override_warning(
|
||||
"shell_environment_policy",
|
||||
conflict,
|
||||
source,
|
||||
startup_warnings,
|
||||
);
|
||||
}
|
||||
|
||||
fn apply_required_pattern_rules(
|
||||
configured: &mut ShellEnvironmentPolicyToml,
|
||||
required: &Option<BTreeMap<String, ShellEnvironmentPolicyRule>>,
|
||||
) -> bool {
|
||||
let Some(required) = required else {
|
||||
return false;
|
||||
};
|
||||
let conflict = required.iter().any(|(pattern, required_action)| {
|
||||
let configured_action = configured
|
||||
.rules
|
||||
.as_ref()
|
||||
.and_then(|rules| rules.get(pattern).copied())
|
||||
.or_else(|| {
|
||||
configured
|
||||
.exclude
|
||||
.as_ref()
|
||||
.is_some_and(|patterns| patterns.contains(pattern))
|
||||
.then_some(ShellEnvironmentPolicyRule::Exclude)
|
||||
})
|
||||
.or_else(|| {
|
||||
configured
|
||||
.include_only
|
||||
.as_ref()
|
||||
.is_some_and(|patterns| patterns.contains(pattern))
|
||||
.then_some(ShellEnvironmentPolicyRule::Include)
|
||||
});
|
||||
configured_action.is_some_and(|action| action != *required_action)
|
||||
});
|
||||
|
||||
for pattern in required.keys() {
|
||||
if let Some(exclude) = configured.exclude.as_mut() {
|
||||
exclude.retain(|candidate| candidate != pattern);
|
||||
}
|
||||
if let Some(include_only) = configured.include_only.as_mut() {
|
||||
include_only.retain(|candidate| candidate != pattern);
|
||||
}
|
||||
}
|
||||
configured
|
||||
.rules
|
||||
.get_or_insert_default()
|
||||
.extend(required.clone());
|
||||
|
||||
conflict
|
||||
}
|
||||
|
||||
fn apply_feedback_requirement(
|
||||
configured: &mut Option<FeedbackConfigToml>,
|
||||
requirement: Option<&Sourced<FeedbackConfigToml>>,
|
||||
|
||||
Reference in New Issue
Block a user