From 5fb5e47767ac4910f7c208481c23bf4d2fe5cf62 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 25 May 2026 09:44:21 -0700 Subject: [PATCH] Respect hook trust bypass during TUI startup (#24317) Fixes #24093. ## Why `--dangerously-bypass-hook-trust` is a supported CLI flag intended for headless or automated runs where enabled hooks should be allowed to run without requiring persisted trust. In the TUI, startup hook review still opened whenever hooks looked untrusted, so a launch using the bypass could block on the interactive "Hooks need review" prompt. The tricky case is persistent app-server resume: a resume may attach to an already-running thread, where resume config overrides are ignored. In that path, hiding the startup review would be wrong because the existing hook engine may still filter untrusted hooks. ## What Changed - Startup hook review now skips the prompt only when hook trust bypass is actually safe for that launch. - The TUI forwards `bypass_hook_trust` through the app-server request config for fresh thread start/resume/fork paths, and the app-server applies it as a runtime-only `ConfigOverrides` value rather than treating it like a `config.toml` setting. - Persistent app-server resumes keep the startup review prompt so users still have a chance to trust hooks when the running thread cannot receive the bypass override. ## Verification - Added focused coverage for startup hook review with and without `bypass_hook_trust`. - Extended existing TUI/app-server config override tests to cover forwarding and applying `bypass_hook_trust`. --- codex-rs/app-server/src/config_manager.rs | 12 ++++++++-- .../thread_processor_tests.rs | 2 ++ codex-rs/tui/src/app_server_session.rs | 7 +++++- codex-rs/tui/src/lib.rs | 24 +++++++++++++++---- codex-rs/tui/src/startup_hooks_review.rs | 18 +++++++++++++- 5 files changed, 54 insertions(+), 9 deletions(-) diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs index 25fdc5c0cb..2cfd53b130 100644 --- a/codex-rs/app-server/src/config_manager.rs +++ b/codex-rs/app-server/src/config_manager.rs @@ -216,15 +216,23 @@ impl ConfigManager { &self, cli_overrides: &[(String, TomlValue)], request_overrides: Option>, - typesafe_overrides: ConfigOverrides, + mut typesafe_overrides: ConfigOverrides, fallback_cwd: Option, ) -> std::io::Result { + let mut request_overrides = request_overrides.unwrap_or_default(); + if let Some(value) = request_overrides.remove("bypass_hook_trust") { + typesafe_overrides.bypass_hook_trust = Some(value.as_bool().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + "`bypass_hook_trust` override must be a boolean", + ) + })?); + } let merged_cli_overrides = cli_overrides .iter() .cloned() .chain( request_overrides - .unwrap_or_default() .into_iter() .map(|(key, value)| (key, json_to_toml(value))), ) diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index c6b5541aa2..ab8f52ffc0 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -610,6 +610,7 @@ mod thread_processor_behavior_tests { Some(HashMap::from([ ("model_provider".to_string(), json!("request")), ("features.plugins".to_string(), json!(true)), + ("bypass_hook_trust".to_string(), json!(true)), ( "model_providers.session".to_string(), json!({ @@ -626,6 +627,7 @@ mod thread_processor_behavior_tests { assert_eq!(config.model_provider_id, "session"); assert_eq!(config.model_provider, session_provider); assert!(!config.features.enabled(Feature::Plugins)); + assert!(config.bypass_hook_trust); Ok(()) } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 37dc255df5..91ed9176ff 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -1259,6 +1259,9 @@ fn config_request_overrides_from_config( "web_search", Some(config.web_search_mode.value().to_string()), ); + if config.bypass_hook_trust { + overrides.insert("bypass_hook_trust".to_string(), true.into()); + } Some(overrides) } @@ -2131,7 +2134,7 @@ mod tests { } #[tokio::test] - async fn thread_lifecycle_params_forward_model_reasoning_and_service_tier() { + async fn thread_lifecycle_params_forward_config_overrides_and_service_tier() { let temp_dir = tempfile::tempdir().expect("tempdir"); let mut config = build_config(&temp_dir).await; config.model_reasoning_effort = Some(ReasoningEffort::High); @@ -2142,6 +2145,7 @@ mod tests { .web_search_mode .set(WebSearchMode::Disabled) .expect("test web search mode should be allowed"); + config.bypass_hook_trust = true; config.service_tier = Some(ServiceTier::Fast.request_value().to_string()); let thread_id = ThreadId::new(); @@ -2175,6 +2179,7 @@ mod tests { ("model_verbosity".to_string(), string("low")), ("personality".to_string(), string("pragmatic")), ("web_search".to_string(), string("disabled")), + ("bypass_hook_trust".to_string(), true.into()), ]); assert_eq!(start.config, Some(expected_config.clone())); assert_eq!(resume.config, Some(expected_config.clone())); diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 7348e3b452..2683d3796a 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1706,11 +1706,25 @@ async fn run_ratatui_app( }, }; - let startup_hooks_browser = - match maybe_run_startup_hooks_review(&mut app_server, &mut tui, &config).await? { - StartupHooksReviewOutcome::Continue => None, - StartupHooksReviewOutcome::OpenHooksBrowser(data) => Some(data), - }; + // Persistent app-server resumes may attach to an already-running thread, + // where resume config overrides are ignored. + let is_persistent_resume = !matches!(&app_server_target, AppServerTarget::Embedded) + && matches!( + &session_selection, + resume_picker::SessionSelection::Resume(_) + ); + let bypass_hook_trust_for_startup_review = config.bypass_hook_trust && !is_persistent_resume; + let startup_hooks_browser = match maybe_run_startup_hooks_review( + &mut app_server, + &mut tui, + &config, + bypass_hook_trust_for_startup_review, + ) + .await? + { + StartupHooksReviewOutcome::Continue => None, + StartupHooksReviewOutcome::OpenHooksBrowser(data) => Some(data), + }; let app_result = App::run( &mut tui, diff --git a/codex-rs/tui/src/startup_hooks_review.rs b/codex-rs/tui/src/startup_hooks_review.rs index 7c12524896..559c796aa0 100644 --- a/codex-rs/tui/src/startup_hooks_review.rs +++ b/codex-rs/tui/src/startup_hooks_review.rs @@ -46,6 +46,7 @@ pub(crate) async fn maybe_run_startup_hooks_review( app_server: &mut AppServerSession, tui: &mut Tui, config: &Config, + bypass_hook_trust: bool, ) -> Result { let cwd = config.cwd.to_path_buf(); let response = match fetch_hooks_list(app_server.request_handle(), cwd.clone()).await { @@ -56,7 +57,7 @@ pub(crate) async fn maybe_run_startup_hooks_review( } }; let entry = hooks_list_entry_for_cwd(response, &cwd); - if review_needed_count(&entry) == 0 { + if !review_is_needed(bypass_hook_trust, &entry) { return Ok(StartupHooksReviewOutcome::Continue); } @@ -223,6 +224,10 @@ fn review_needed_count(entry: &HooksListEntry) -> usize { .count() } +fn review_is_needed(bypass_hook_trust: bool, entry: &HooksListEntry) -> bool { + !bypass_hook_trust && review_needed_count(entry) > 0 +} + fn selection_item(name: &str, is_disabled: bool) -> SelectionItem { SelectionItem { name: name.to_string(), @@ -259,6 +264,7 @@ impl WidgetRef for &StandaloneSelectionView<'_> { #[cfg(test)] mod tests { + use super::review_is_needed; use super::selection_view; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -333,6 +339,16 @@ mod tests { .join("\n") } + #[test] + fn bypass_hook_trust_suppresses_startup_review() { + assert!(!review_is_needed(/*bypass_hook_trust*/ true, &entry())); + } + + #[test] + fn untrusted_hooks_need_review_without_bypass() { + assert!(review_is_needed(/*bypass_hook_trust*/ false, &entry())); + } + #[test] fn renders_prompt() { let (tx_raw, _rx) = unbounded_channel::();