mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
add --dangerously-bypass-hook-trust CLI flag (#21768)
# Why Hook trust happens through the TUI in `/hooks` so it can block non-interactive use cases. This flag will allow users that are using codex headlessly to bypass hooks when they want to. # What This adds one invocation-scoped escape hatch. - the CLI flag sets a runtime-only `bypass_hook_trust` override; there is no durable `config.toml` setting - hook discovery still respects normal enablement, so explicitly disabled hooks remain disabled - we show a `--dangerously-bypass-hook-trust is enabled. Enabled hooks may run without review for this invocation.` message on startup so accidental use is visible in both interactive and exec flows This keeps “enabled” and “trusted” as separate concepts in the normal path, while giving CI/E2E callers a stable way to opt into the exceptional path when they already control the hook set.
This commit is contained in:
@@ -41,6 +41,7 @@ struct HookHandlerSource<'a> {
|
||||
key_source: String,
|
||||
source: HookSource,
|
||||
is_managed: bool,
|
||||
bypass_hook_trust: bool,
|
||||
hook_states: &'a HashMap<String, HookStateToml>,
|
||||
env: HashMap<String, String>,
|
||||
plugin_id: Option<String>,
|
||||
@@ -49,6 +50,7 @@ struct HookHandlerSource<'a> {
|
||||
#[derive(Clone, Copy)]
|
||||
struct HookDiscoveryPolicy {
|
||||
allow_managed_hooks_only: bool,
|
||||
bypass_hook_trust: bool,
|
||||
}
|
||||
|
||||
impl HookDiscoveryPolicy {
|
||||
@@ -61,6 +63,7 @@ pub(crate) fn discover_handlers(
|
||||
config_layer_stack: Option<&ConfigLayerStack>,
|
||||
plugin_hook_sources: Vec<PluginHookSource>,
|
||||
plugin_hook_load_warnings: Vec<String>,
|
||||
bypass_hook_trust: bool,
|
||||
) -> DiscoveryResult {
|
||||
let mut handlers = Vec::new();
|
||||
let mut hook_entries = Vec::new();
|
||||
@@ -75,6 +78,7 @@ pub(crate) fn discover_handlers(
|
||||
.as_ref()
|
||||
.is_some_and(|requirement| requirement.value)
|
||||
}),
|
||||
bypass_hook_trust,
|
||||
};
|
||||
|
||||
if let Some(config_layer_stack) = config_layer_stack {
|
||||
@@ -99,6 +103,7 @@ pub(crate) fn discover_handlers(
|
||||
key_source: policy_path.display().to_string(),
|
||||
source: hook_source,
|
||||
is_managed,
|
||||
bypass_hook_trust: false,
|
||||
hook_states: &hook_states,
|
||||
env: HashMap::new(),
|
||||
plugin_id: None,
|
||||
@@ -132,6 +137,7 @@ pub(crate) fn discover_handlers(
|
||||
key_source: source_path.display().to_string(),
|
||||
source: hook_source,
|
||||
is_managed,
|
||||
bypass_hook_trust: policy.bypass_hook_trust,
|
||||
hook_states: &hook_states,
|
||||
env: HashMap::new(),
|
||||
plugin_id: None,
|
||||
@@ -183,6 +189,7 @@ fn append_managed_requirement_handlers(
|
||||
key_source: source_path.display().to_string(),
|
||||
source: hook_source_for_requirement_source(managed_hooks.source.as_ref()),
|
||||
is_managed: true,
|
||||
bypass_hook_trust: false,
|
||||
hook_states,
|
||||
env: HashMap::new(),
|
||||
plugin_id: None,
|
||||
@@ -233,6 +240,7 @@ fn append_plugin_hook_sources(
|
||||
),
|
||||
source: HookSource::Plugin,
|
||||
is_managed: false,
|
||||
bypass_hook_trust: policy.bypass_hook_trust,
|
||||
hook_states,
|
||||
env,
|
||||
plugin_id: Some(plugin_id),
|
||||
@@ -485,10 +493,11 @@ fn append_matcher_groups(
|
||||
trust_status,
|
||||
});
|
||||
if enabled
|
||||
&& matches!(
|
||||
trust_status,
|
||||
HookTrustStatus::Managed | HookTrustStatus::Trusted
|
||||
)
|
||||
&& (source.bypass_hook_trust
|
||||
|| matches!(
|
||||
trust_status,
|
||||
HookTrustStatus::Managed | HookTrustStatus::Trusted
|
||||
))
|
||||
{
|
||||
handlers.push(ConfiguredHandler {
|
||||
event_name,
|
||||
@@ -620,6 +629,7 @@ mod tests {
|
||||
use codex_config::HookStateToml;
|
||||
use codex_config::MatcherGroup;
|
||||
use codex_config::TomlValue;
|
||||
use codex_protocol::protocol::HookTrustStatus;
|
||||
|
||||
fn source_path() -> AbsolutePathBuf {
|
||||
test_path_buf("/tmp/hooks.json").abs()
|
||||
@@ -638,6 +648,24 @@ mod tests {
|
||||
key_source: path.display().to_string(),
|
||||
source: hook_source(),
|
||||
is_managed: true,
|
||||
bypass_hook_trust: false,
|
||||
hook_states,
|
||||
env: std::collections::HashMap::new(),
|
||||
plugin_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
fn unmanaged_hook_handler_source<'a>(
|
||||
path: &'a AbsolutePathBuf,
|
||||
hook_states: &'a std::collections::HashMap<String, HookStateToml>,
|
||||
bypass_hook_trust: bool,
|
||||
) -> super::HookHandlerSource<'a> {
|
||||
super::HookHandlerSource {
|
||||
path,
|
||||
key_source: path.display().to_string(),
|
||||
source: HookSource::User,
|
||||
is_managed: false,
|
||||
bypass_hook_trust,
|
||||
hook_states,
|
||||
env: std::collections::HashMap::new(),
|
||||
plugin_id: None,
|
||||
@@ -727,6 +755,72 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bypass_hook_trust_allows_enabled_untrusted_handlers() {
|
||||
let mut handlers = Vec::new();
|
||||
let mut hook_entries = Vec::new();
|
||||
let mut warnings = Vec::new();
|
||||
let mut display_order = 0;
|
||||
let source_path = source_path();
|
||||
let hook_states = std::collections::HashMap::new();
|
||||
|
||||
append_matcher_groups(
|
||||
&mut handlers,
|
||||
&mut hook_entries,
|
||||
&mut warnings,
|
||||
&mut display_order,
|
||||
&unmanaged_hook_handler_source(
|
||||
&source_path,
|
||||
&hook_states,
|
||||
/*bypass_hook_trust*/ true,
|
||||
),
|
||||
HookEventName::PreToolUse,
|
||||
vec![command_group(Some("Bash"))],
|
||||
);
|
||||
|
||||
assert_eq!(warnings, Vec::<String>::new());
|
||||
assert_eq!(handlers.len(), 1);
|
||||
assert_eq!(hook_entries.len(), 1);
|
||||
assert_eq!(hook_entries[0].trust_status, HookTrustStatus::Untrusted);
|
||||
assert_eq!(hook_entries[0].enabled, true);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bypass_hook_trust_respects_disabled_handlers() {
|
||||
let mut handlers = Vec::new();
|
||||
let mut hook_entries = Vec::new();
|
||||
let mut warnings = Vec::new();
|
||||
let mut display_order = 0;
|
||||
let source_path = source_path();
|
||||
let hook_states = std::collections::HashMap::from([(
|
||||
format!("{}:pre_tool_use:0:0", source_path.display()),
|
||||
HookStateToml {
|
||||
enabled: Some(false),
|
||||
trusted_hash: None,
|
||||
},
|
||||
)]);
|
||||
|
||||
append_matcher_groups(
|
||||
&mut handlers,
|
||||
&mut hook_entries,
|
||||
&mut warnings,
|
||||
&mut display_order,
|
||||
&unmanaged_hook_handler_source(
|
||||
&source_path,
|
||||
&hook_states,
|
||||
/*bypass_hook_trust*/ true,
|
||||
),
|
||||
HookEventName::PreToolUse,
|
||||
vec![command_group(Some("Bash"))],
|
||||
);
|
||||
|
||||
assert_eq!(warnings, Vec::<String>::new());
|
||||
assert_eq!(handlers, Vec::<ConfiguredHandler>::new());
|
||||
assert_eq!(hook_entries.len(), 1);
|
||||
assert_eq!(hook_entries[0].trust_status, HookTrustStatus::Untrusted);
|
||||
assert_eq!(hook_entries[0].enabled, false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pre_tool_use_treats_star_matcher_as_match_all() {
|
||||
let mut handlers = Vec::new();
|
||||
|
||||
@@ -106,6 +106,7 @@ pub(crate) struct ClaudeHooksEngine {
|
||||
impl ClaudeHooksEngine {
|
||||
pub(crate) fn new(
|
||||
enabled: bool,
|
||||
bypass_hook_trust: bool,
|
||||
config_layer_stack: Option<&ConfigLayerStack>,
|
||||
plugin_hook_sources: Vec<PluginHookSource>,
|
||||
plugin_hook_load_warnings: Vec<String>,
|
||||
@@ -125,6 +126,7 @@ impl ClaudeHooksEngine {
|
||||
config_layer_stack,
|
||||
plugin_hook_sources,
|
||||
plugin_hook_load_warnings,
|
||||
bypass_hook_trust,
|
||||
);
|
||||
Self {
|
||||
handlers: discovered.handlers,
|
||||
|
||||
@@ -196,6 +196,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -211,6 +212,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
|
||||
let listed = crate::list_hooks(crate::HooksConfig {
|
||||
legacy_notify_argv: None,
|
||||
feature_enabled: true,
|
||||
bypass_hook_trust: false,
|
||||
config_layer_stack: Some(config_layer_stack.clone()),
|
||||
plugin_hook_sources: Vec::new(),
|
||||
plugin_hook_load_warnings: Vec::new(),
|
||||
@@ -295,6 +297,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -372,6 +375,7 @@ fn unknown_requirement_source_hooks_stay_managed() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -383,8 +387,12 @@ fn unknown_requirement_source_hooks_stay_managed() {
|
||||
|
||||
assert_eq!(engine.handlers.len(), 1);
|
||||
assert_eq!(engine.handlers[0].source, HookSource::Unknown);
|
||||
let discovered =
|
||||
super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new());
|
||||
let discovered = super::discovery::discover_handlers(
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
/*bypass_hook_trust*/ false,
|
||||
);
|
||||
assert_eq!(discovered.hook_entries.len(), 1);
|
||||
assert_eq!(discovered.hook_entries[0].source, HookSource::Unknown);
|
||||
assert_eq!(discovered.hook_entries[0].enabled, true);
|
||||
@@ -446,6 +454,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -457,8 +466,12 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() {
|
||||
|
||||
assert_eq!(engine.handlers.len(), 1);
|
||||
assert_eq!(engine.handlers[0].source, HookSource::CloudRequirements);
|
||||
let discovered =
|
||||
super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new());
|
||||
let discovered = super::discovery::discover_handlers(
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
/*bypass_hook_trust*/ false,
|
||||
);
|
||||
assert_eq!(discovered.hook_entries.len(), 2);
|
||||
assert_eq!(discovered.hook_entries[0].key, managed_disabled_key);
|
||||
assert_eq!(discovered.hook_entries[0].enabled, true);
|
||||
@@ -503,6 +516,7 @@ fn user_disablement_does_not_filter_managed_layer_hooks() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -517,8 +531,12 @@ fn user_disablement_does_not_filter_managed_layer_hooks() {
|
||||
engine.handlers[0].source,
|
||||
HookSource::LegacyManagedConfigFile
|
||||
);
|
||||
let discovered =
|
||||
super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new());
|
||||
let discovered = super::discovery::discover_handlers(
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
/*bypass_hook_trust*/ false,
|
||||
);
|
||||
assert_eq!(discovered.hook_entries.len(), 1);
|
||||
assert_eq!(discovered.hook_entries[0].key, managed_key);
|
||||
assert_eq!(discovered.hook_entries[0].enabled, true);
|
||||
@@ -586,6 +604,7 @@ fn trusted_plugin_hook_stack(
|
||||
/*config_layer_stack*/ None,
|
||||
plugin_hook_sources.to_vec(),
|
||||
Vec::new(),
|
||||
/*bypass_hook_trust*/ false,
|
||||
);
|
||||
let state = discovered
|
||||
.hook_entries
|
||||
@@ -655,6 +674,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -706,6 +726,7 @@ fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -717,8 +738,12 @@ fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() {
|
||||
|
||||
assert!(engine.warnings().is_empty());
|
||||
assert!(engine.handlers.is_empty());
|
||||
let discovered =
|
||||
super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new());
|
||||
let discovered = super::discovery::discover_handlers(
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
/*bypass_hook_trust*/ false,
|
||||
);
|
||||
assert_eq!(discovered.hook_entries.len(), 1);
|
||||
assert!(!discovered.hook_entries[0].is_managed);
|
||||
assert_eq!(
|
||||
@@ -752,6 +777,7 @@ fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -763,8 +789,12 @@ fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() {
|
||||
|
||||
assert!(engine.warnings().is_empty());
|
||||
assert!(engine.handlers.is_empty());
|
||||
let discovered =
|
||||
super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new());
|
||||
let discovered = super::discovery::discover_handlers(
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
/*bypass_hook_trust*/ false,
|
||||
);
|
||||
assert_eq!(discovered.hook_entries.len(), 1);
|
||||
assert!(!discovered.hook_entries[0].is_managed);
|
||||
assert_eq!(
|
||||
@@ -814,6 +844,7 @@ fn allow_managed_hooks_only_skips_unmanaged_json_and_toml_hooks() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -852,6 +883,7 @@ fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
plugin_hook_sources,
|
||||
Vec::new(),
|
||||
@@ -923,6 +955,7 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -947,8 +980,12 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() {
|
||||
"python3 /tmp/legacy-mdm-hook.py",
|
||||
]
|
||||
);
|
||||
let discovered =
|
||||
super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new());
|
||||
let discovered = super::discovery::discover_handlers(
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
/*bypass_hook_trust*/ false,
|
||||
);
|
||||
assert!(discovered.hook_entries.iter().all(|entry| entry.is_managed));
|
||||
}
|
||||
|
||||
@@ -1028,6 +1065,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
@@ -1119,6 +1157,7 @@ print(json.dumps({
|
||||
);
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
plugin_hook_sources.clone(),
|
||||
Vec::new(),
|
||||
@@ -1146,6 +1185,7 @@ print(json.dumps({
|
||||
let listed = crate::list_hooks(crate::HooksConfig {
|
||||
legacy_notify_argv: None,
|
||||
feature_enabled: true,
|
||||
bypass_hook_trust: false,
|
||||
config_layer_stack: None,
|
||||
plugin_hook_sources,
|
||||
plugin_hook_load_warnings: Vec::new(),
|
||||
@@ -1229,6 +1269,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
|
||||
);
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
plugin_hook_sources,
|
||||
Vec::new(),
|
||||
@@ -1272,6 +1313,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
|
||||
fn plugin_hook_load_warnings_are_startup_warnings() {
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
/*config_layer_stack*/ None,
|
||||
Vec::new(),
|
||||
vec!["failed plugin hook".to_string()],
|
||||
|
||||
@@ -30,6 +30,7 @@ use crate::types::HookResponse;
|
||||
pub struct HooksConfig {
|
||||
pub legacy_notify_argv: Option<Vec<String>>,
|
||||
pub feature_enabled: bool,
|
||||
pub bypass_hook_trust: bool,
|
||||
pub config_layer_stack: Option<ConfigLayerStack>,
|
||||
pub plugin_hook_sources: Vec<PluginHookSource>,
|
||||
pub plugin_hook_load_warnings: Vec<String>,
|
||||
@@ -65,6 +66,7 @@ impl Hooks {
|
||||
.collect();
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
config.feature_enabled,
|
||||
config.bypass_hook_trust,
|
||||
config.config_layer_stack.as_ref(),
|
||||
config.plugin_hook_sources,
|
||||
config.plugin_hook_load_warnings,
|
||||
@@ -212,6 +214,7 @@ pub fn list_hooks(config: HooksConfig) -> HookListOutcome {
|
||||
config.config_layer_stack.as_ref(),
|
||||
config.plugin_hook_sources,
|
||||
config.plugin_hook_load_warnings,
|
||||
config.bypass_hook_trust,
|
||||
);
|
||||
HookListOutcome {
|
||||
hooks: discovered.hook_entries,
|
||||
|
||||
Reference in New Issue
Block a user