mirror of
https://github.com/openai/codex.git
synced 2026-05-04 11:26:33 +00:00
codex: support hooks in config.toml and requirements.toml (#18893)
## Summary Support the existing hooks schema in inline TOML so hooks can be configured from both `config.toml` and enterprise-managed `requirements.toml` without requiring a separate `hooks.json` payload. This gives enterprise admins a way to ship managed hook policy through the existing requirements channel while still leaving script delivery to MDM or other device-management tooling, and it keeps `hooks.json` working unchanged for existing users. This also lays the groundwork for follow-on managed filtering work such as #15937, while continuing to respect project trust gating from #14718. It does **not** implement `allow_managed_hooks_only` itself. NOTE: yes, it's a bit unfortunate that the toml isn't formatted as closely as normal to our default styling. This is because we're trying to stay compatible with the spec for plugins/hooks that we'll need to support & the main usecase here is embedding into requirements.toml ## What changed - moved the shared hook serde model out of `codex-rs/hooks` into `codex-rs/config` so the same schema can power `hooks.json`, inline `config.toml` hooks, and managed `requirements.toml` hooks - added `hooks` support to both `ConfigToml` and `ConfigRequirementsToml`, including requirements-side `managed_dir` / `windows_managed_dir` - treated requirements-managed hooks as one constrained value via `Constrained`, so managed hook policy is merged atomically and cannot drift across requirement sources - updated hook discovery to load requirements-managed hooks first, then per-layer `hooks.json`, then per-layer inline TOML hooks, with a warning when a single layer defines both representations - threaded managed hook metadata through discovered handlers and exposed requirements hooks in app-server responses, generated schemas, and `/debug-config` - added hook/config coverage in `codex-rs/config`, `codex-rs/hooks`, `codex-rs/core/src/config_loader/tests.rs`, and `codex-rs/core/tests/suite/hooks.rs` ## Testing - `cargo test -p codex-config` - `cargo test -p codex-hooks` - `cargo test -p codex-app-server config_api` ## Documentation Companion updates are needed in the developers website repo for: - the hooks guide - the config reference, sample, basic, and advanced pages - the enterprise managed configuration guide --------- Co-authored-by: Michael Bolin <mbolin@openai.com>
This commit is contained in:
@@ -17,6 +17,7 @@ use super::requirements_exec_policy::RequirementsExecPolicy;
|
||||
use super::requirements_exec_policy::RequirementsExecPolicyToml;
|
||||
use crate::Constrained;
|
||||
use crate::ConstraintError;
|
||||
use crate::ManagedHooksRequirementsToml;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum RequirementSource {
|
||||
@@ -86,6 +87,7 @@ pub struct ConfigRequirements {
|
||||
pub sandbox_policy: ConstrainedWithSource<SandboxPolicy>,
|
||||
pub web_search_mode: ConstrainedWithSource<WebSearchMode>,
|
||||
pub feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
|
||||
pub managed_hooks: Option<ConstrainedWithSource<ManagedHooksRequirementsToml>>,
|
||||
pub mcp_servers: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
pub exec_policy: Option<Sourced<RequirementsExecPolicy>>,
|
||||
pub enforce_residency: ConstrainedWithSource<Option<ResidencyRequirement>>,
|
||||
@@ -117,6 +119,7 @@ impl Default for ConfigRequirements {
|
||||
/*source*/ None,
|
||||
),
|
||||
feature_requirements: None,
|
||||
managed_hooks: None,
|
||||
mcp_servers: None,
|
||||
exec_policy: None,
|
||||
enforce_residency: ConstrainedWithSource::new(
|
||||
@@ -628,6 +631,7 @@ pub struct ConfigRequirementsToml {
|
||||
pub allowed_web_search_modes: Option<Vec<WebSearchModeRequirement>>,
|
||||
#[serde(rename = "features", alias = "feature_requirements")]
|
||||
pub feature_requirements: Option<FeatureRequirementsToml>,
|
||||
pub hooks: Option<ManagedHooksRequirementsToml>,
|
||||
pub mcp_servers: Option<BTreeMap<String, McpServerRequirement>>,
|
||||
pub apps: Option<AppsRequirementsToml>,
|
||||
pub rules: Option<RequirementsExecPolicyToml>,
|
||||
@@ -673,6 +677,7 @@ pub struct ConfigRequirementsWithSources {
|
||||
pub allowed_sandbox_modes: Option<Sourced<Vec<SandboxModeRequirement>>>,
|
||||
pub allowed_web_search_modes: Option<Sourced<Vec<WebSearchModeRequirement>>>,
|
||||
pub feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
|
||||
pub hooks: Option<Sourced<ManagedHooksRequirementsToml>>,
|
||||
pub mcp_servers: Option<Sourced<BTreeMap<String, McpServerRequirement>>>,
|
||||
pub apps: Option<Sourced<AppsRequirementsToml>>,
|
||||
pub rules: Option<Sourced<RequirementsExecPolicyToml>>,
|
||||
@@ -707,6 +712,7 @@ impl ConfigRequirementsWithSources {
|
||||
remote_sandbox_config: _,
|
||||
allowed_web_search_modes: _,
|
||||
feature_requirements: _,
|
||||
hooks: _,
|
||||
mcp_servers: _,
|
||||
apps: _,
|
||||
rules: _,
|
||||
@@ -734,6 +740,7 @@ impl ConfigRequirementsWithSources {
|
||||
allowed_sandbox_modes,
|
||||
allowed_web_search_modes,
|
||||
feature_requirements,
|
||||
hooks,
|
||||
mcp_servers,
|
||||
rules,
|
||||
enforce_residency,
|
||||
@@ -759,6 +766,7 @@ impl ConfigRequirementsWithSources {
|
||||
allowed_sandbox_modes,
|
||||
allowed_web_search_modes,
|
||||
feature_requirements,
|
||||
hooks,
|
||||
mcp_servers,
|
||||
apps,
|
||||
rules,
|
||||
@@ -774,6 +782,7 @@ impl ConfigRequirementsWithSources {
|
||||
remote_sandbox_config: None,
|
||||
allowed_web_search_modes: allowed_web_search_modes.map(|sourced| sourced.value),
|
||||
feature_requirements: feature_requirements.map(|sourced| sourced.value),
|
||||
hooks: hooks.map(|sourced| sourced.value),
|
||||
mcp_servers: mcp_servers.map(|sourced| sourced.value),
|
||||
apps: apps.map(|sourced| sourced.value),
|
||||
rules: rules.map(|sourced| sourced.value),
|
||||
@@ -858,6 +867,10 @@ impl ConfigRequirementsToml {
|
||||
.feature_requirements
|
||||
.as_ref()
|
||||
.is_none_or(FeatureRequirementsToml::is_empty)
|
||||
&& self
|
||||
.hooks
|
||||
.as_ref()
|
||||
.is_none_or(ManagedHooksRequirementsToml::is_empty)
|
||||
&& self.mcp_servers.is_none()
|
||||
&& self
|
||||
.apps
|
||||
@@ -884,6 +897,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
allowed_sandbox_modes,
|
||||
allowed_web_search_modes,
|
||||
feature_requirements,
|
||||
hooks,
|
||||
mcp_servers,
|
||||
apps: _apps,
|
||||
rules,
|
||||
@@ -1064,6 +1078,34 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
};
|
||||
let feature_requirements =
|
||||
feature_requirements.filter(|requirements| !requirements.value.is_empty());
|
||||
let managed_hooks = hooks
|
||||
.filter(|managed_hooks| managed_hooks.value.handler_count() > 0)
|
||||
.map(|sourced_hooks| {
|
||||
let Sourced {
|
||||
value,
|
||||
source: requirement_source,
|
||||
} = sourced_hooks;
|
||||
let allowed = value;
|
||||
let allowed_for_error = format!("{allowed:?}");
|
||||
let requirement_source_for_error = requirement_source.clone();
|
||||
let constrained = Constrained::new(allowed.clone(), move |candidate| {
|
||||
if candidate == &allowed {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(ConstraintError::InvalidValue {
|
||||
field_name: "hooks",
|
||||
candidate: format!("{candidate:?}"),
|
||||
allowed: allowed_for_error.clone(),
|
||||
requirement_source: requirement_source_for_error.clone(),
|
||||
})
|
||||
}
|
||||
})?;
|
||||
Ok(ConstrainedWithSource::new(
|
||||
constrained,
|
||||
Some(requirement_source),
|
||||
))
|
||||
})
|
||||
.transpose()?;
|
||||
|
||||
let enforce_residency = match enforce_residency {
|
||||
Some(Sourced {
|
||||
@@ -1106,6 +1148,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
sandbox_policy,
|
||||
web_search_mode,
|
||||
feature_requirements,
|
||||
managed_hooks,
|
||||
mcp_servers,
|
||||
exec_policy,
|
||||
enforce_residency,
|
||||
@@ -1119,6 +1162,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::HookEventsToml;
|
||||
use anyhow::Result;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
@@ -1147,6 +1191,7 @@ mod tests {
|
||||
remote_sandbox_config: _,
|
||||
allowed_web_search_modes,
|
||||
feature_requirements,
|
||||
hooks,
|
||||
mcp_servers,
|
||||
apps,
|
||||
rules,
|
||||
@@ -1166,6 +1211,7 @@ mod tests {
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
feature_requirements: feature_requirements
|
||||
.map(|value| Sourced::new(value, RequirementSource::Unknown)),
|
||||
hooks: hooks.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)),
|
||||
@@ -1210,6 +1256,7 @@ mod tests {
|
||||
remote_sandbox_config: None,
|
||||
allowed_web_search_modes: Some(allowed_web_search_modes.clone()),
|
||||
feature_requirements: Some(feature_requirements.clone()),
|
||||
hooks: None,
|
||||
mcp_servers: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
@@ -1241,6 +1288,7 @@ mod tests {
|
||||
feature_requirements,
|
||||
enforce_source.clone(),
|
||||
)),
|
||||
hooks: None,
|
||||
mcp_servers: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
@@ -1278,6 +1326,7 @@ mod tests {
|
||||
allowed_sandbox_modes: None,
|
||||
allowed_web_search_modes: None,
|
||||
feature_requirements: None,
|
||||
hooks: None,
|
||||
mcp_servers: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
@@ -1323,6 +1372,7 @@ mod tests {
|
||||
allowed_sandbox_modes: None,
|
||||
allowed_web_search_modes: None,
|
||||
feature_requirements: None,
|
||||
hooks: None,
|
||||
mcp_servers: None,
|
||||
apps: None,
|
||||
rules: None,
|
||||
@@ -2232,6 +2282,124 @@ allowed_approvals_reviewers = ["user"]
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserialize_managed_hooks_requirements() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
managed_dir = "/enterprise/hooks"
|
||||
windows_managed_dir = 'C:\enterprise\hooks'
|
||||
|
||||
[[PreToolUse]]
|
||||
matcher = "^Bash$"
|
||||
|
||||
[[PreToolUse.hooks]]
|
||||
type = "command"
|
||||
command = "python3 /enterprise/hooks/pre.py"
|
||||
timeout = 10
|
||||
statusMessage = "checking"
|
||||
"#;
|
||||
let hooks: ManagedHooksRequirementsToml = from_str(toml_str)?;
|
||||
|
||||
assert_eq!(
|
||||
hooks.managed_dir.as_deref(),
|
||||
Some(std::path::Path::new("/enterprise/hooks"))
|
||||
);
|
||||
assert_eq!(hooks.handler_count(), 1);
|
||||
assert_eq!(hooks.hooks.pre_tool_use.len(), 1);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_unset_fields_does_not_overwrite_existing_hooks() -> Result<()> {
|
||||
let mut target = ConfigRequirementsWithSources::default();
|
||||
target.merge_unset_fields(
|
||||
RequirementSource::CloudRequirements,
|
||||
from_str::<ConfigRequirementsToml>(
|
||||
r#"
|
||||
[hooks]
|
||||
managed_dir = "/cloud/hooks"
|
||||
|
||||
[[hooks.PreToolUse]]
|
||||
matcher = "^Bash$"
|
||||
|
||||
[[hooks.PreToolUse.hooks]]
|
||||
type = "command"
|
||||
command = "python3 /cloud/hooks/pre.py"
|
||||
"#,
|
||||
)?,
|
||||
);
|
||||
target.merge_unset_fields(
|
||||
RequirementSource::SystemRequirementsToml {
|
||||
file: system_requirements_toml_file_for_test()?,
|
||||
},
|
||||
from_str::<ConfigRequirementsToml>(
|
||||
r#"
|
||||
[hooks]
|
||||
managed_dir = "/system/hooks"
|
||||
|
||||
[[hooks.PreToolUse]]
|
||||
matcher = "^Bash$"
|
||||
|
||||
[[hooks.PreToolUse.hooks]]
|
||||
type = "command"
|
||||
command = "python3 /system/hooks/pre.py"
|
||||
"#,
|
||||
)?,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
target
|
||||
.hooks
|
||||
.as_ref()
|
||||
.and_then(|hooks| hooks.value.managed_dir.as_ref())
|
||||
.map(std::path::PathBuf::as_path),
|
||||
Some(std::path::Path::new("/cloud/hooks"))
|
||||
);
|
||||
assert_eq!(
|
||||
target.hooks.as_ref().map(|hooks| hooks.source.clone()),
|
||||
Some(RequirementSource::CloudRequirements)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_hooks_constraint_rejects_drift() -> Result<()> {
|
||||
let config: ConfigRequirementsToml = from_str(
|
||||
r#"
|
||||
[hooks]
|
||||
managed_dir = "/enterprise/hooks"
|
||||
|
||||
[[hooks.PreToolUse]]
|
||||
matcher = "^Bash$"
|
||||
|
||||
[[hooks.PreToolUse.hooks]]
|
||||
type = "command"
|
||||
command = "python3 /enterprise/hooks/pre.py"
|
||||
"#,
|
||||
)?;
|
||||
let requirements: ConfigRequirements = with_unknown_source(config).try_into()?;
|
||||
let mut managed_hooks = requirements
|
||||
.managed_hooks
|
||||
.expect("expected managed hooks requirements");
|
||||
|
||||
let err = managed_hooks
|
||||
.set(ManagedHooksRequirementsToml {
|
||||
managed_dir: Some(std::path::PathBuf::from("/other/hooks")),
|
||||
windows_managed_dir: None,
|
||||
hooks: HookEventsToml::default(),
|
||||
})
|
||||
.expect_err("managed hooks should reject drift");
|
||||
|
||||
assert!(matches!(
|
||||
err,
|
||||
ConstraintError::InvalidValue {
|
||||
field_name: "hooks",
|
||||
requirement_source: RequirementSource::Unknown,
|
||||
..
|
||||
}
|
||||
));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn network_requirements_are_preserved_as_constraints_with_source() -> Result<()> {
|
||||
let toml_str = r#"
|
||||
|
||||
Reference in New Issue
Block a user