feat: add support for /etc/codex/requirements.toml on UNIX (#8277)
This implements the new config design where config _requirements_ are loaded separately (and with a special schema) as compared to config _settings_. In particular, on UNIX, with this PR, you could define `/etc/codex/requirements.toml` with: ```toml allowed_approval_policies = ["never", "on-request"] ``` to enforce that `Config.approval_policy` must be one of those two values when Codex runs. We plan to expand the set of things that can be restricted by `/etc/codex/requirements.toml` in short order. Note that requirements can come from several sources: - new MDM key on macOS (not implemented yet) - `/etc/codex/requirements.toml` - re-interpretation of legacy MDM key on macOS (`com.openai.codex/config_toml_base64`) - re-interpretation of legacy `/etc/codex/managed_config.toml` So our resolution strategy is to load TOML data from those sources, in order. Later TOMLs are "merged" into previous TOMLs, but any field that is already set cannot be overwritten. See `ConfigRequirementsToml::merge_unset_fields()`.
This commit is contained in:
parent
4fb0b547d6
commit
2f048f2063
3 changed files with 186 additions and 16 deletions
|
|
@ -25,6 +25,26 @@ pub struct ConfigRequirementsToml {
|
|||
pub allowed_approval_policies: Option<Vec<AskForApproval>>,
|
||||
}
|
||||
|
||||
impl ConfigRequirementsToml {
|
||||
/// For every field in `other` that is `Some`, if the corresponding field in
|
||||
/// `self` is `None`, copy the value from `other` into `self`.
|
||||
pub fn merge_unset_fields(&mut self, mut other: ConfigRequirementsToml) {
|
||||
macro_rules! fill_missing_take {
|
||||
($base:expr, $other:expr, { $($field:ident),+ $(,)? }) => {
|
||||
$(
|
||||
if $base.$field.is_none() {
|
||||
if let Some(value) = $other.$field.take() {
|
||||
$base.$field = Some(value);
|
||||
}
|
||||
}
|
||||
)+
|
||||
};
|
||||
}
|
||||
|
||||
fill_missing_take!(self, other, { allowed_approval_policies });
|
||||
}
|
||||
}
|
||||
|
||||
impl TryFrom<ConfigRequirementsToml> for ConfigRequirements {
|
||||
type Error = ConstraintError;
|
||||
|
||||
|
|
@ -45,3 +65,43 @@ impl TryFrom<ConfigRequirementsToml> for ConfigRequirements {
|
|||
Ok(ConfigRequirements { approval_policy })
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use anyhow::Result;
|
||||
use pretty_assertions::assert_eq;
|
||||
use toml::from_str;
|
||||
|
||||
#[test]
|
||||
fn merge_unset_fields_only_fills_missing_values() -> Result<()> {
|
||||
let source: ConfigRequirementsToml = from_str(
|
||||
r#"
|
||||
allowed_approval_policies = ["on-request"]
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let mut empty_target: ConfigRequirementsToml = from_str(
|
||||
r#"
|
||||
# intentionally left unset
|
||||
"#,
|
||||
)?;
|
||||
empty_target.merge_unset_fields(source.clone());
|
||||
assert_eq!(
|
||||
empty_target.allowed_approval_policies,
|
||||
Some(vec![AskForApproval::OnRequest])
|
||||
);
|
||||
|
||||
let mut populated_target: ConfigRequirementsToml = from_str(
|
||||
r#"
|
||||
allowed_approval_policies = ["never"]
|
||||
"#,
|
||||
)?;
|
||||
populated_target.merge_unset_fields(source);
|
||||
assert_eq!(
|
||||
populated_target.allowed_approval_policies,
|
||||
Some(vec![AskForApproval::Never])
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ mod state;
|
|||
mod tests;
|
||||
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config_loader::config_requirements::ConfigRequirementsToml;
|
||||
use crate::config_loader::layer_io::LoadedConfigLayers;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
|
|
@ -26,6 +27,9 @@ pub use state::ConfigLayerEntry;
|
|||
pub use state::ConfigLayerStack;
|
||||
pub use state::LoaderOverrides;
|
||||
|
||||
/// On Unix systems, load requirements from this file path, if present.
|
||||
const DEFAULT_REQUIREMENTS_TOML_FILE_UNIX: &str = "/etc/codex/requirements.toml";
|
||||
|
||||
/// To build up the set of admin-enforced constraints, we build up from multiple
|
||||
/// configuration layers in the following order, but a constraint defined in an
|
||||
/// earlier layer cannot be overridden by a later layer:
|
||||
|
|
@ -55,10 +59,28 @@ pub async fn load_config_layers_state(
|
|||
cli_overrides: &[(String, TomlValue)],
|
||||
overrides: LoaderOverrides,
|
||||
) -> io::Result<ConfigLayerStack> {
|
||||
let loaded_config_layers = layer_io::load_config_layers_internal(codex_home, overrides).await?;
|
||||
let requirements = load_requirements_from_legacy_scheme(loaded_config_layers.clone()).await?;
|
||||
let mut config_requirements_toml = ConfigRequirementsToml::default();
|
||||
|
||||
// TODO(mbolin): Honor /etc/codex/requirements.toml.
|
||||
// TODO(mbolin): Support an entry in MDM for config requirements and use it
|
||||
// with `config_requirements_toml.merge_unset_fields(...)`, if present.
|
||||
|
||||
// Honor /etc/codex/requirements.toml.
|
||||
if cfg!(unix) {
|
||||
load_requirements_toml(
|
||||
&mut config_requirements_toml,
|
||||
DEFAULT_REQUIREMENTS_TOML_FILE_UNIX,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
// Make a best-effort to support the legacy `managed_config.toml` as a
|
||||
// requirements specification.
|
||||
let loaded_config_layers = layer_io::load_config_layers_internal(codex_home, overrides).await?;
|
||||
load_requirements_from_legacy_scheme(
|
||||
&mut config_requirements_toml,
|
||||
loaded_config_layers.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut layers = Vec::<ConfigLayerEntry>::new();
|
||||
|
||||
|
|
@ -133,23 +155,59 @@ pub async fn load_config_layers_state(
|
|||
));
|
||||
}
|
||||
|
||||
ConfigLayerStack::new(layers, requirements)
|
||||
ConfigLayerStack::new(layers, config_requirements_toml.try_into()?)
|
||||
}
|
||||
|
||||
/// If available, apply requirements from `/etc/codex/requirements.toml` to
|
||||
/// `config_requirements_toml` by filling in any unset fields.
|
||||
async fn load_requirements_toml(
|
||||
config_requirements_toml: &mut ConfigRequirementsToml,
|
||||
requirements_toml_file: impl AsRef<Path>,
|
||||
) -> io::Result<()> {
|
||||
match tokio::fs::read_to_string(&requirements_toml_file).await {
|
||||
Ok(contents) => {
|
||||
let requirements_config: ConfigRequirementsToml =
|
||||
toml::from_str(&contents).map_err(|e| {
|
||||
io::Error::new(
|
||||
io::ErrorKind::InvalidData,
|
||||
format!(
|
||||
"Error parsing requirements file {}: {e}",
|
||||
requirements_toml_file.as_ref().display(),
|
||||
),
|
||||
)
|
||||
})?;
|
||||
config_requirements_toml.merge_unset_fields(requirements_config);
|
||||
}
|
||||
Err(e) => {
|
||||
if e.kind() != io::ErrorKind::NotFound {
|
||||
return Err(io::Error::new(
|
||||
e.kind(),
|
||||
format!(
|
||||
"Failed to read requirements file {}: {e}",
|
||||
requirements_toml_file.as_ref().display(),
|
||||
),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn load_requirements_from_legacy_scheme(
|
||||
config_requirements_toml: &mut ConfigRequirementsToml,
|
||||
loaded_config_layers: LoadedConfigLayers,
|
||||
) -> io::Result<ConfigRequirements> {
|
||||
let mut config_requirements = ConfigRequirements::default();
|
||||
|
||||
// In this implementation, later layers override earlier layers, so list
|
||||
// managed_config_from_mdm last because it has the highest precedence.
|
||||
) -> io::Result<()> {
|
||||
// In this implementation, earlier layers cannot be overwritten by later
|
||||
// layers, so list managed_config_from_mdm first because it has the highest
|
||||
// precedence.
|
||||
let LoadedConfigLayers {
|
||||
managed_config,
|
||||
managed_config_from_mdm,
|
||||
} = loaded_config_layers;
|
||||
for config in [
|
||||
managed_config.map(|c| c.managed_config),
|
||||
managed_config_from_mdm,
|
||||
managed_config.map(|c| c.managed_config),
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
|
|
@ -162,14 +220,11 @@ async fn load_requirements_from_legacy_scheme(
|
|||
)
|
||||
})?;
|
||||
|
||||
let LegacyManagedConfigToml { approval_policy } = legacy_config;
|
||||
if let Some(approval_policy) = approval_policy {
|
||||
config_requirements.approval_policy =
|
||||
crate::config::Constrained::allow_only(approval_policy);
|
||||
}
|
||||
let new_requirements_toml = ConfigRequirementsToml::from(legacy_config);
|
||||
config_requirements_toml.merge_unset_fields(new_requirements_toml);
|
||||
}
|
||||
|
||||
Ok(config_requirements)
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The legacy mechanism for specifying admin-enforced configuration is to read
|
||||
|
|
@ -184,3 +239,16 @@ async fn load_requirements_from_legacy_scheme(
|
|||
struct LegacyManagedConfigToml {
|
||||
approval_policy: Option<AskForApproval>,
|
||||
}
|
||||
|
||||
impl From<LegacyManagedConfigToml> for ConfigRequirementsToml {
|
||||
fn from(legacy: LegacyManagedConfigToml) -> Self {
|
||||
let mut config_requirements_toml = ConfigRequirementsToml::default();
|
||||
|
||||
let LegacyManagedConfigToml { approval_policy } = legacy;
|
||||
if let Some(approval_policy) = approval_policy {
|
||||
config_requirements_toml.allowed_approval_policies = Some(vec![approval_policy]);
|
||||
}
|
||||
|
||||
config_requirements_toml
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,11 @@
|
|||
use super::LoaderOverrides;
|
||||
use super::load_config_layers_state;
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config_loader::ConfigRequirements;
|
||||
use crate::config_loader::config_requirements::ConfigRequirementsToml;
|
||||
use crate::config_loader::load_requirements_toml;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
|
|
@ -147,3 +152,40 @@ flag = true
|
|||
);
|
||||
assert_eq!(nested.get("flag"), Some(&TomlValue::Boolean(false)));
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn load_requirements_toml_produces_expected_constraints() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
let requirements_file = tmp.path().join("requirements.toml");
|
||||
tokio::fs::write(
|
||||
&requirements_file,
|
||||
r#"
|
||||
allowed_approval_policies = ["never", "on-request"]
|
||||
"#,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut config_requirements_toml = ConfigRequirementsToml::default();
|
||||
load_requirements_toml(&mut config_requirements_toml, &requirements_file).await?;
|
||||
|
||||
assert_eq!(
|
||||
config_requirements_toml.allowed_approval_policies,
|
||||
Some(vec![AskForApproval::Never, AskForApproval::OnRequest])
|
||||
);
|
||||
|
||||
let config_requirements: ConfigRequirements = config_requirements_toml.try_into()?;
|
||||
assert_eq!(
|
||||
config_requirements.approval_policy.value(),
|
||||
AskForApproval::OnRequest
|
||||
);
|
||||
config_requirements
|
||||
.approval_policy
|
||||
.can_set(&AskForApproval::Never)?;
|
||||
assert!(
|
||||
config_requirements
|
||||
.approval_policy
|
||||
.can_set(&AskForApproval::OnFailure)
|
||||
.is_err()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue