agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Add allow_managed_hooks_only hook requirement (#20319)

Browse Source 
## Why

Enterprise-managed hook policy needs a narrow way to require Codex to
ignore user-controlled lifecycle hooks without adopting the broader
trust-precedence model from earlier hook work. This keeps the policy
anchored in `requirements.toml`, so admins can opt into managed hooks
only while normal `config.toml` files cannot enable the restriction
themselves.

## What changed

- Added `allow_managed_hooks_only` to the requirements data flow and
preserved explicit `false` values.
- Also adds it to /debug-config
- Marked MDM, system, and legacy managed config layers as managed for
hook discovery.
- Updated hook discovery so `allow_managed_hooks_only = true`:
  - keeps managed requirements hooks and managed config-layer hooks,
- skips user/project/session `hooks.json` and `[hooks]` entries with
concise startup warnings,
  - skips current unmanaged plugin hooks,
- ignores any `allow_managed_hooks_only` key placed in ordinary
`config.toml` layers.
This commit is contained in:
Andrei Eternal
2026-05-12 19:05:25 -07:00
committed by GitHub
Unverified
parent fbfbfe5fc5
commit 913aad4d3c
17 changed files with 650 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/hooks/src/engine/discovery.rs
+75 -44
View File
@@ -1,3 +1,4 @@
use std::collections::HashMap;
use std::fs;
use std::path::Path;
@@ -19,7 +20,6 @@ use codex_plugin::PluginHookSource;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Deserialize;
use serde::Serialize;
use std::collections::HashMap;
use super::ConfiguredHandler;
use super::HookListEntry;
@@ -46,6 +46,17 @@ struct HookHandlerSource<'a> {
plugin_id: Option<String>,
}
#[derive(Clone, Copy)]
struct HookDiscoveryPolicy {
allow_managed_hooks_only: bool,
}
impl HookDiscoveryPolicy {
fn allows(self, source: &HookHandlerSource<'_>) -> bool {
!self.allow_managed_hooks_only || source.is_managed
}
}
pub(crate) fn discover_handlers(
config_layer_stack: Option<&ConfigLayerStack>,
plugin_hook_sources: Vec<PluginHookSource>,
@@ -56,6 +67,15 @@ pub(crate) fn discover_handlers(
let mut warnings = plugin_hook_load_warnings;
let mut display_order = 0_i64;
let hook_states = hook_states_from_stack(config_layer_stack);
let policy = HookDiscoveryPolicy {
allow_managed_hooks_only: config_layer_stack.is_some_and(|config_layer_stack| {
config_layer_stack
.requirements()
.allow_managed_hooks_only
.as_ref()
.is_some_and(|requirement| requirement.value)
}),
};
if let Some(config_layer_stack) = config_layer_stack {
append_managed_requirement_handlers(
@@ -65,6 +85,7 @@ pub(crate) fn discover_handlers(
&mut display_order,
config_layer_stack,
&hook_states,
policy,
);
for layer in config_layer_stack.get_layers(
@@ -72,6 +93,19 @@ pub(crate) fn discover_handlers(
/*include_disabled*/ false,
) {
let (hook_source, is_managed) = hook_metadata_for_config_layer_source(&layer.name);
let policy_path = config_toml_source_path(layer);
let policy_source = HookHandlerSource {
path: &policy_path,
key_source: policy_path.display().to_string(),
source: hook_source,
is_managed,
hook_states: &hook_states,
env: HashMap::new(),
plugin_id: None,
};
if !policy.allows(&policy_source) {
continue;
}
let json_hooks = load_hooks_json(layer.config_folder().as_deref(), &mut warnings);
let toml_hooks = load_toml_hooks_from_layer(layer, &mut warnings);
@@ -103,6 +137,7 @@ pub(crate) fn discover_handlers(
plugin_id: None,
},
hook_events,
policy,
);
}
}
@@ -115,6 +150,7 @@ pub(crate) fn discover_handlers(
&mut display_order,
plugin_hook_sources,
&hook_states,
policy,
);
DiscoveryResult {
@@ -131,15 +167,12 @@ fn append_managed_requirement_handlers(
display_order: &mut i64,
config_layer_stack: &ConfigLayerStack,
hook_states: &HashMap<String, HookStateToml>,
policy: HookDiscoveryPolicy,
) {
let Some(managed_hooks) = config_layer_stack.requirements().managed_hooks.as_ref() else {
return;
};
let Some(source_path) =
managed_hooks_source_path(managed_hooks.get(), managed_hooks.source.as_ref(), warnings)
else {
return;
};
let source_path = managed_hooks_source_path(managed_hooks.get(), managed_hooks.source.as_ref());
append_hook_events(
handlers,
hook_entries,
@@ -155,6 +188,7 @@ fn append_managed_requirement_handlers(
plugin_id: None,
},
managed_hooks.get().hooks.clone(),
policy,
);
}
@@ -165,6 +199,7 @@ fn append_plugin_hook_sources(
display_order: &mut i64,
plugin_hook_sources: Vec<PluginHookSource>,
hook_states: &HashMap<String, HookStateToml>,
policy: HookDiscoveryPolicy,
) {
for source in plugin_hook_sources {
let PluginHookSource {
@@ -203,6 +238,7 @@ fn append_plugin_hook_sources(
plugin_id: Some(plugin_id),
},
hooks,
policy,
);
}
}
@@ -210,45 +246,35 @@ fn append_plugin_hook_sources(
fn managed_hooks_source_path(
managed_hooks: &ManagedHooksRequirementsToml,
requirement_source: Option<&RequirementSource>,
warnings: &mut Vec<String>,
) -> Option<AbsolutePathBuf> {
let source = requirement_source
.map(ToString::to_string)
.unwrap_or_else(|| "managed requirements".to_string());
let Some(source_path) = managed_hooks.managed_dir_for_current_platform() else {
warnings.push(format!(
"skipping managed hooks from {source}: no managed hook directory is configured for this platform"
));
return None;
};
) -> AbsolutePathBuf {
if let Some(source_path) = managed_hooks.managed_dir_for_current_platform()
&& source_path.is_absolute()
&& let Ok(source_path) = AbsolutePathBuf::from_absolute_path(source_path)
{
return source_path;
}
if !source_path.is_absolute() {
warnings.push(format!(
"skipping managed hooks from {source}: managed hook directory {} is not absolute",
source_path.display()
));
None
} else if !source_path.exists() {
warnings.push(format!(
"skipping managed hooks from {source}: managed hook directory {} does not exist",
source_path.display()
));
None
} else if !source_path.is_dir() {
warnings.push(format!(
"skipping managed hooks from {source}: managed hook directory {} is not a directory",
source_path.display()
));
None
} else {
AbsolutePathBuf::from_absolute_path(source_path)
.inspect_err(|err| {
warnings.push(format!(
"skipping managed hooks from {source}: could not normalize managed hook directory {}: {err}",
source_path.display()
));
})
.ok()
fallback_managed_hooks_source_path(requirement_source)
}
fn fallback_managed_hooks_source_path(
requirement_source: Option<&RequirementSource>,
) -> AbsolutePathBuf {
match requirement_source {
Some(RequirementSource::SystemRequirementsToml { file })
| Some(RequirementSource::LegacyManagedConfigTomlFromFile { file }) => file.clone(),
Some(RequirementSource::MdmManagedPreferences { domain, key }) => {
synthetic_layer_path(&format!("<mdm:{domain}:{key}>/requirements.toml"))
}
Some(RequirementSource::CloudRequirements) => {
synthetic_layer_path("<cloud-requirements>/requirements.toml")
}
Some(RequirementSource::LegacyManagedConfigTomlFromMdm) => {
synthetic_layer_path("<legacy-managed-config.toml-mdm>/managed_config.toml")
}
Some(RequirementSource::Unknown) | None => {
synthetic_layer_path("<managed-requirements>/requirements.toml")
}
}
}
@@ -350,7 +376,12 @@ fn append_hook_events(
display_order: &mut i64,
source: HookHandlerSource<'_>,
hook_events: HookEventsToml,
policy: HookDiscoveryPolicy,
) {
if !policy.allows(&source) {
return;
}
for (event_name, groups) in hook_events.into_matcher_groups() {
append_matcher_groups(
handlers,
codex-rs/hooks/src/engine/mod_tests.rs
+369 -22
View File
@@ -15,6 +15,7 @@ use codex_config::HookHandlerConfig;
use codex_config::ManagedHooksRequirementsToml;
use codex_config::MatcherGroup;
use codex_config::RequirementSource;
use codex_config::Sourced;
use codex_config::TomlValue;
use codex_plugin::PluginHookSource;
use codex_plugin::PluginId;
@@ -55,6 +56,88 @@ fn managed_hooks_for_current_platform(
}
}
fn pre_tool_use_hook_events(command: impl Into<String>) -> HookEventsToml {
HookEventsToml {
pre_tool_use: vec![MatcherGroup {
matcher: Some("^Bash$".to_string()),
hooks: vec![HookHandlerConfig::Command {
command: command.into(),
command_windows: None,
timeout_sec: Some(10),
r#async: false,
status_message: Some("checking".to_string()),
}],
}],
..Default::default()
}
}
fn config_toml_with_pre_tool_use(command: &str) -> TomlValue {
let mut config_toml = TomlValue::Table(Default::default());
let TomlValue::Table(config_table) = &mut config_toml else {
unreachable!("config TOML root should be a table");
};
let mut hooks_table = TomlValue::Table(Default::default());
let TomlValue::Table(hooks_entries) = &mut hooks_table else {
unreachable!("hooks entry should be a table");
};
let mut pre_tool_use_group = TomlValue::Table(Default::default());
let TomlValue::Table(pre_tool_use_group_entries) = &mut pre_tool_use_group else {
unreachable!("PreToolUse group should be a table");
};
pre_tool_use_group_entries.insert(
"matcher".to_string(),
TomlValue::String("^Bash$".to_string()),
);
let mut handler = TomlValue::Table(Default::default());
let TomlValue::Table(handler_entries) = &mut handler else {
unreachable!("PreToolUse handler should be a table");
};
handler_entries.insert("type".to_string(), TomlValue::String("command".to_string()));
handler_entries.insert(
"command".to_string(),
TomlValue::String(command.to_string()),
);
handler_entries.insert("timeout".to_string(), TomlValue::Integer(10));
handler_entries.insert(
"statusMessage".to_string(),
TomlValue::String("checking".to_string()),
);
pre_tool_use_group_entries.insert("hooks".to_string(), TomlValue::Array(vec![handler]));
hooks_entries.insert(
"PreToolUse".to_string(),
TomlValue::Array(vec![pre_tool_use_group]),
);
config_table.insert("hooks".to_string(), hooks_table);
config_toml
}
fn requirements_with_managed_hooks_only(
allow_managed_hooks_only: bool,
managed_hooks: Option<ManagedHooksRequirementsToml>,
) -> (ConfigRequirements, ConfigRequirementsToml) {
(
ConfigRequirements {
allow_managed_hooks_only: Some(Sourced::new(
allow_managed_hooks_only,
RequirementSource::CloudRequirements,
)),
managed_hooks: managed_hooks.clone().map(|hooks| {
ConstrainedWithSource::new(
Constrained::allow_any(hooks),
Some(RequirementSource::CloudRequirements),
)
}),
..ConfigRequirements::default()
},
ConfigRequirementsToml {
allow_managed_hooks_only: Some(allow_managed_hooks_only),
hooks: managed_hooks,
..ConfigRequirementsToml::default()
},
)
}
#[tokio::test]
async fn requirements_managed_hooks_execute_from_managed_dir() {
let temp = tempdir().expect("create temp dir");
@@ -535,7 +618,7 @@ fn trusted_plugin_hook_stack(
}
#[test]
fn requirements_managed_hooks_warn_when_managed_dir_is_missing() {
fn requirements_managed_hooks_load_when_managed_dir_is_missing() {
let temp = tempdir().expect("create temp dir");
let missing_dir = temp.path().join("missing-managed-hooks");
let managed_hooks = managed_hooks_for_current_platform(
@@ -544,7 +627,7 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() {
pre_tool_use: vec![MatcherGroup {
matcher: Some("^Bash$".to_string()),
hooks: vec![HookHandlerConfig::Command {
command: format!("python3 {}", missing_dir.join("pre.py").display()),
command: "echo hi".to_string(),
command_windows: None,
timeout_sec: Some(10),
r#async: false,
@@ -581,30 +664,294 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() {
},
);
assert!(engine.warnings().iter().any(|warning| {
warning.contains("managed hook directory")
&& warning.contains("does not exist")
&& warning.contains(&missing_dir.display().to_string())
}));
assert!(engine.warnings().is_empty());
let cwd = cwd();
assert!(
engine
.preview_pre_tool_use(&PreToolUseRequest {
session_id: ThreadId::new(),
turn_id: "turn-1".to_string(),
cwd,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
tool_name: "Bash".to_string(),
matcher_aliases: Vec::new(),
tool_use_id: "tool-1".to_string(),
tool_input: serde_json::json!({ "command": "echo hello" }),
})
.is_empty()
let preview = engine.preview_pre_tool_use(&PreToolUseRequest {
session_id: ThreadId::new(),
turn_id: "turn-1".to_string(),
cwd,
transcript_path: None,
model: "gpt-test".to_string(),
permission_mode: "default".to_string(),
tool_name: "Bash".to_string(),
matcher_aliases: Vec::new(),
tool_use_id: "tool-1".to_string(),
tool_input: serde_json::json!({ "command": "echo hello" }),
});
assert_eq!(preview.len(), 1);
assert_eq!(engine.handlers[0].command, "echo hi");
assert_eq!(
engine.handlers[0].source_path,
AbsolutePathBuf::try_from(missing_dir).expect("absolute missing dir")
);
}
#[test]
fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() {
let temp = tempdir().expect("create temp dir");
let config_path =
AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path");
let (requirements, requirements_toml) = requirements_with_managed_hooks_only(
/*allow_managed_hooks_only*/ false, /*managed_hooks*/ None,
);
let config_layer_stack = ConfigLayerStack::new(
vec![ConfigLayerEntry::new(
ConfigLayerSource::User { file: config_path },
config_toml_with_pre_tool_use("python3 /tmp/user-hook.py"),
)],
requirements,
requirements_toml,
)
.expect("config layer stack");
let engine = ClaudeHooksEngine::new(
/*enabled*/ true,
Some(&config_layer_stack),
Vec::new(),
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
},
);
assert!(engine.warnings().is_empty());
assert!(engine.handlers.is_empty());
let discovered =
super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new());
assert_eq!(discovered.hook_entries.len(), 1);
assert!(!discovered.hook_entries[0].is_managed);
assert_eq!(
discovered.hook_entries[0].command.as_deref(),
Some("python3 /tmp/user-hook.py")
);
}
#[test]
fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() {
let temp = tempdir().expect("create temp dir");
let config_path =
AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path");
let mut config_toml = config_toml_with_pre_tool_use("python3 /tmp/user-hook.py");
let TomlValue::Table(config_table) = &mut config_toml else {
unreachable!("config TOML root should be a table");
};
config_table.insert(
"allow_managed_hooks_only".to_string(),
TomlValue::Boolean(true),
);
let config_layer_stack = ConfigLayerStack::new(
vec![ConfigLayerEntry::new(
ConfigLayerSource::User { file: config_path },
config_toml,
)],
ConfigRequirements::default(),
ConfigRequirementsToml::default(),
)
.expect("config layer stack");
let engine = ClaudeHooksEngine::new(
/*enabled*/ true,
Some(&config_layer_stack),
Vec::new(),
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
},
);
assert!(engine.warnings().is_empty());
assert!(engine.handlers.is_empty());
let discovered =
super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new());
assert_eq!(discovered.hook_entries.len(), 1);
assert!(!discovered.hook_entries[0].is_managed);
assert_eq!(
discovered.hook_entries[0].command.as_deref(),
Some("python3 /tmp/user-hook.py")
);
}
#[test]
fn allow_managed_hooks_only_skips_unmanaged_json_and_toml_hooks() {
let temp = tempdir().expect("create temp dir");
let config_path =
AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path");
let hooks_json_path =
AbsolutePathBuf::try_from(temp.path().join("hooks.json")).expect("absolute hooks path");
fs::write(
hooks_json_path.as_path(),
r#"{
"hooks": {
"PreToolUse": [
{
"matcher": "^Bash$",
"hooks": [
{
"type": "command",
"command": "python3 /tmp/json-hook.py"
}
]
}
]
}
}"#,
)
.expect("write hooks.json");
let (requirements, requirements_toml) = requirements_with_managed_hooks_only(
/*allow_managed_hooks_only*/ true, /*managed_hooks*/ None,
);
let config_layer_stack = ConfigLayerStack::new(
vec![ConfigLayerEntry::new(
ConfigLayerSource::User { file: config_path },
config_toml_with_pre_tool_use("python3 /tmp/toml-hook.py"),
)],
requirements,
requirements_toml,
)
.expect("config layer stack");
let engine = ClaudeHooksEngine::new(
/*enabled*/ true,
Some(&config_layer_stack),
Vec::new(),
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
},
);
assert!(engine.handlers.is_empty());
assert!(engine.warnings().is_empty());
}
#[test]
fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() {
let temp = tempdir().expect("create temp dir");
let plugin_root =
AbsolutePathBuf::try_from(temp.path().join("demo-plugin")).expect("plugin root");
let plugin_data_root =
AbsolutePathBuf::try_from(temp.path().join("plugin-data")).expect("plugin data root");
let source_path = plugin_root.join("hooks/hooks.json");
let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id");
let plugin_hook_sources = vec![PluginHookSource {
plugin_id,
plugin_root,
plugin_data_root,
source_path,
source_relative_path: "hooks/hooks.json".to_string(),
hooks: pre_tool_use_hook_events("python3 /tmp/plugin-hook.py"),
}];
let (requirements, requirements_toml) = requirements_with_managed_hooks_only(
/*allow_managed_hooks_only*/ true, /*managed_hooks*/ None,
);
let config_layer_stack = ConfigLayerStack::new(Vec::new(), requirements, requirements_toml)
.expect("config layer stack");
let engine = ClaudeHooksEngine::new(
/*enabled*/ true,
Some(&config_layer_stack),
plugin_hook_sources,
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
},
);
assert!(engine.handlers.is_empty());
assert!(engine.warnings().is_empty());
}
#[test]
fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() {
let temp = tempdir().expect("create temp dir");
let managed_dir =
AbsolutePathBuf::try_from(temp.path().join("managed-hooks")).expect("absolute path");
fs::create_dir_all(managed_dir.as_path()).expect("create managed hooks dir");
let system_config_path =
AbsolutePathBuf::try_from(temp.path().join("system").join("config.toml"))
.expect("absolute system config path");
let system_parent = system_config_path
.as_path()
.parent()
.expect("system config parent");
fs::create_dir_all(system_parent).expect("create system config dir");
let legacy_config_path = AbsolutePathBuf::try_from(temp.path().join("managed_config.toml"))
.expect("absolute legacy config path");
let managed_hooks = managed_hooks_for_current_platform(
managed_dir,
pre_tool_use_hook_events("python3 /tmp/requirements-hook.py"),
);
let (requirements, requirements_toml) = requirements_with_managed_hooks_only(
/*allow_managed_hooks_only*/ true,
Some(managed_hooks),
);
let config_layer_stack = ConfigLayerStack::new(
vec![
ConfigLayerEntry::new(
ConfigLayerSource::Mdm {
domain: "com.openai.codex".to_string(),
key: "config".to_string(),
},
config_toml_with_pre_tool_use("python3 /tmp/mdm-hook.py"),
),
ConfigLayerEntry::new(
ConfigLayerSource::System {
file: system_config_path,
},
config_toml_with_pre_tool_use("python3 /tmp/system-hook.py"),
),
ConfigLayerEntry::new(
ConfigLayerSource::LegacyManagedConfigTomlFromFile {
file: legacy_config_path,
},
config_toml_with_pre_tool_use("python3 /tmp/legacy-file-hook.py"),
),
ConfigLayerEntry::new(
ConfigLayerSource::LegacyManagedConfigTomlFromMdm,
config_toml_with_pre_tool_use("python3 /tmp/legacy-mdm-hook.py"),
),
],
requirements,
requirements_toml,
)
.expect("config layer stack");
let engine = ClaudeHooksEngine::new(
/*enabled*/ true,
Some(&config_layer_stack),
Vec::new(),
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
},
);
assert!(engine.warnings().is_empty());
assert_eq!(
engine
.handlers
.iter()
.map(|handler| handler.command.as_str())
.collect::<Vec<_>>(),
vec![
"python3 /tmp/requirements-hook.py",
"python3 /tmp/mdm-hook.py",
"python3 /tmp/system-hook.py",
"python3 /tmp/legacy-file-hook.py",
"python3 /tmp/legacy-mdm-hook.py",
]
);
let discovered =
super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new());
assert!(discovered.hook_entries.iter().all(|entry| entry.is_managed));
}
#[test]
fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
let temp = tempdir().expect("create temp dir");