mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Preserve hook trust bypass in codex exec threads (#26434)
Addresses #26383 and #26452 ## Summary `codex exec --dangerously-bypass-hook-trust` printed the bypass warning, but valid untrusted hooks still did not run. Exec applied the flag to its initial config, then lost it when app-server reloaded config for the new or resumed thread. ## Fix Forward `bypass_hook_trust: true` through the existing thread request config override for both start and resume. The override is omitted when the flag is not enabled, preserving normal trust behavior. ## Testing Added: - A test confirming start and resume preserve the override. - An end-to-end exec test confirming a `SessionStart` hook runs and creates a marker file.
This commit is contained in:
committed by
GitHub
Unverified
parent
d5b4b98370
commit
d007b0852a
@@ -138,6 +138,7 @@ pub use exec_events::TurnStartedEvent;
|
||||
pub use exec_events::Usage;
|
||||
pub use exec_events::WebSearchItem;
|
||||
use serde_json::Value;
|
||||
use std::collections::HashMap;
|
||||
use std::future::Future;
|
||||
use std::io::IsTerminal;
|
||||
use std::io::Read;
|
||||
@@ -1054,7 +1055,7 @@ fn thread_start_params_from_config(config: &Config) -> ThreadStartParams {
|
||||
approvals_reviewer: approvals_reviewer_override_from_config(config),
|
||||
sandbox: sandbox.flatten(),
|
||||
permissions,
|
||||
config: None,
|
||||
config: thread_config_overrides_from_config(config),
|
||||
ephemeral: Some(config.ephemeral),
|
||||
thread_source: Some(ThreadSource::User),
|
||||
..ThreadStartParams::default()
|
||||
@@ -1079,11 +1080,17 @@ fn thread_resume_params_from_config(config: &Config, thread_id: String) -> Threa
|
||||
approvals_reviewer: approvals_reviewer_override_from_config(config),
|
||||
sandbox: sandbox.flatten(),
|
||||
permissions,
|
||||
config: None,
|
||||
config: thread_config_overrides_from_config(config),
|
||||
..ThreadResumeParams::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn thread_config_overrides_from_config(config: &Config) -> Option<HashMap<String, Value>> {
|
||||
config
|
||||
.bypass_hook_trust
|
||||
.then(|| HashMap::from([("bypass_hook_trust".to_string(), Value::Bool(true))]))
|
||||
}
|
||||
|
||||
fn permissions_selection_from_config(config: &Config) -> Option<String> {
|
||||
config
|
||||
.permissions
|
||||
|
||||
@@ -593,6 +593,32 @@ async fn thread_start_params_include_user_thread_source() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_lifecycle_params_preserve_hook_trust_bypass() {
|
||||
let codex_home = tempdir().expect("create temp codex home");
|
||||
let cwd = tempdir().expect("create temp cwd");
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.harness_overrides(ConfigOverrides {
|
||||
bypass_hook_trust: Some(true),
|
||||
..Default::default()
|
||||
})
|
||||
.fallback_cwd(Some(cwd.path().to_path_buf()))
|
||||
.build()
|
||||
.await
|
||||
.expect("build config with hook trust bypass");
|
||||
let expected_config = Some(HashMap::from([(
|
||||
"bypass_hook_trust".to_string(),
|
||||
serde_json::Value::Bool(true),
|
||||
)]));
|
||||
|
||||
let start_params = thread_start_params_from_config(&config);
|
||||
let resume_params = thread_resume_params_from_config(&config, "thread-id".to_string());
|
||||
|
||||
assert_eq!(start_params.config, expected_config);
|
||||
assert_eq!(resume_params.config, expected_config);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn active_profile_selection_uses_profile_id_only() {
|
||||
let selection = permission_profile_id_from_active_profile(ActivePermissionProfile::new(
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
#![cfg(not(target_os = "windows"))]
|
||||
#![allow(clippy::expect_used)]
|
||||
|
||||
use core_test_support::responses;
|
||||
use core_test_support::test_codex_exec::test_codex_exec;
|
||||
use serde_json::json;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_hook_trust_bypass_runs_session_start_hook() -> anyhow::Result<()> {
|
||||
let test = test_codex_exec();
|
||||
let marker_path = test.home_path().join("session-start-ran");
|
||||
let command = format!("touch {}", marker_path.display());
|
||||
std::fs::write(
|
||||
test.home_path().join("hooks.json"),
|
||||
serde_json::to_vec_pretty(&json!({
|
||||
"hooks": {
|
||||
"SessionStart": [{
|
||||
"hooks": [{
|
||||
"type": "command",
|
||||
"command": command,
|
||||
}],
|
||||
}],
|
||||
},
|
||||
}))?,
|
||||
)?;
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let body = responses::sse(vec![
|
||||
responses::ev_response_created("response_1"),
|
||||
responses::ev_assistant_message("response_1", "done"),
|
||||
responses::ev_completed("response_1"),
|
||||
]);
|
||||
responses::mount_sse_once(&server, body).await;
|
||||
|
||||
test.cmd_with_server(&server)
|
||||
.arg("--skip-git-repo-check")
|
||||
.arg("--dangerously-bypass-hook-trust")
|
||||
.arg("run the session start hook")
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
assert!(marker_path.exists(), "session start hook did not run");
|
||||
Ok(())
|
||||
}
|
||||
@@ -5,6 +5,7 @@ mod apply_patch;
|
||||
mod approval_policy;
|
||||
mod auth_env;
|
||||
mod ephemeral;
|
||||
mod hooks;
|
||||
mod mcp_required_exit;
|
||||
mod originator;
|
||||
mod output_schema;
|
||||
|
||||
Reference in New Issue
Block a user