From d50dfd4827609f133992425f45ed47fbd76464e2 Mon Sep 17 00:00:00 2001 From: Hunter B Date: Fri, 12 Jun 2026 02:38:56 -0700 Subject: [PATCH] fix(hooks): require workspace trust for project hooks Gate .codewhale/hooks.toml behind user-owned workspace trust, mirroring the project-local MCP trust boundary while preserving shell-command hook semantics. Harvested from PR #3140. Co-authored-by: Hmbown <101357273+Hmbown@users.noreply.github.com> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> --- CHANGELOG.md | 4 ++ crates/tui/CHANGELOG.md | 4 ++ crates/tui/src/hooks.rs | 97 +++++++++++++++++++++++++++++++++++++--- crates/tui/src/tui/ui.rs | 9 ++++ docs/CONFIGURATION.md | 14 ++++-- 5 files changed, 118 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f86f169..87fff320 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 persisted turn items once and groups them by turn instead of reading the items directory once per turn, preserving item order while keeping large thread detail loads responsive. +- **Project-local hook trust boundary (#3140).** `.codewhale/hooks.toml` is now + loaded only after the workspace is trusted in user-owned config, matching the + project-local MCP trust model while preserving the documented shell-command + hook contract. - **SiliconFlow China provider config (#2893/#2895).** `siliconflow-CN` now reads its own `[providers.siliconflow_cn]` / `[providers.siliconflow-CN]` table and falls back to `[providers.siliconflow]` only for unset diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index 990c1c8d..dd22eac1 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -57,6 +57,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 persisted turn items once and groups them by turn instead of reading the items directory once per turn, preserving item order while keeping large thread detail loads responsive. +- **Project-local hook trust boundary (#3140).** `.codewhale/hooks.toml` is now + loaded only after the workspace is trusted in user-owned config, matching the + project-local MCP trust model while preserving the documented shell-command + hook contract. - **SiliconFlow China provider config (#2893/#2895).** `siliconflow-CN` now reads its own `[providers.siliconflow_cn]` / `[providers.siliconflow-CN]` table and falls back to `[providers.siliconflow]` only for unset diff --git a/crates/tui/src/hooks.rs b/crates/tui/src/hooks.rs index 7843ed54..6ceb2489 100644 --- a/crates/tui/src/hooks.rs +++ b/crates/tui/src/hooks.rs @@ -18,7 +18,7 @@ use serde::{Deserialize, Serialize}; use serde_json::json; use std::collections::HashMap; use std::io::{Read, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant}; @@ -219,10 +219,15 @@ fn default_enabled() -> bool { impl HooksConfig { /// Load global hooks merged with project-local `.codewhale/hooks.toml` (#3026). /// - /// Project hooks are appended after global hooks. A malformed project file - /// logs a warning and falls back to global-only. - pub fn load_with_project(global: HooksConfig, workspace: &std::path::Path) -> HooksConfig { + /// Project hooks are executable repository configuration, so they are only + /// honored after the workspace has been trusted in user-owned config. + /// Trusted project hooks are appended after global hooks. A malformed + /// trusted project file logs a warning and falls back to global-only. + pub fn load_with_project(global: HooksConfig, workspace: &Path) -> HooksConfig { let project_path = workspace.join(".codewhale").join("hooks.toml"); + if !project_path.exists() || !workspace_allows_project_hooks(workspace) { + return global; + } let Ok(contents) = std::fs::read_to_string(&project_path) else { return global; }; @@ -256,6 +261,10 @@ impl HooksConfig { } } +fn workspace_allows_project_hooks(workspace: &Path) -> bool { + crate::config::is_workspace_trusted(workspace) +} + /// Context passed to hooks via environment variables #[derive(Debug, Clone, Default)] pub struct HookContext { @@ -1424,8 +1433,15 @@ fn parse_env_lines(stdout: &str) -> HashMap { #[cfg(test)] mod tests { use super::*; + use crate::test_support::{EnvVarGuard, lock_test_env}; use std::collections::HashMap; - use std::path::PathBuf; + use std::path::{Path, PathBuf}; + + fn trust_workspace_for_project_hooks(workspace: &Path, config_path: &Path) -> EnvVarGuard { + let guard = EnvVarGuard::set("CODEWHALE_CONFIG_PATH", config_path); + crate::config::save_workspace_trust(workspace).expect("save workspace trust"); + guard + } /// #456 — `parse_env_lines` covers the formats users actually emit from /// shell hooks: bare `KEY=VAL`, `export KEY=VAL`, quoted values, comments, @@ -2438,7 +2454,11 @@ exit 7 #[test] fn load_with_project_appends_project_hooks_after_global() { + let _lock = lock_test_env(); let dir = tempfile::tempdir().expect("tempdir"); + let config_path = dir.path().join("user-config.toml"); + let _config = trust_workspace_for_project_hooks(dir.path(), &config_path); + let _legacy_config = EnvVarGuard::remove("DEEPSEEK_CONFIG_PATH"); let project_dir = dir.path().join(".codewhale"); std::fs::create_dir_all(&project_dir).expect("mkdir .codewhale"); std::fs::write( @@ -2470,8 +2490,73 @@ command = "echo project" } #[test] - fn load_with_project_malformed_file_falls_back_to_global() { + fn load_with_project_ignores_project_hooks_until_workspace_trusted() { + let _lock = lock_test_env(); let dir = tempfile::tempdir().expect("tempdir"); + let _config = EnvVarGuard::set("CODEWHALE_CONFIG_PATH", dir.path().join("config.toml")); + let _legacy_config = EnvVarGuard::remove("DEEPSEEK_CONFIG_PATH"); + let project_dir = dir.path().join(".codewhale"); + std::fs::create_dir_all(&project_dir).expect("mkdir .codewhale"); + std::fs::write( + project_dir.join("hooks.toml"), + r#" +[[hooks]] +event = "tool_call_before" +command = "echo project" +"#, + ) + .expect("write hooks.toml"); + + let global = HooksConfig { + enabled: true, + hooks: vec![Hook::new(HookEvent::ToolCallBefore, "echo global")], + ..HooksConfig::default() + }; + + let merged = HooksConfig::load_with_project(global, dir.path()); + assert_eq!(merged.hooks.len(), 1); + assert_eq!(merged.hooks[0].command, "echo global"); + } + + #[test] + fn load_with_project_ignores_project_local_legacy_trust_marker() { + let _lock = lock_test_env(); + let dir = tempfile::tempdir().expect("tempdir"); + let _config = EnvVarGuard::set("CODEWHALE_CONFIG_PATH", dir.path().join("config.toml")); + let _legacy_config = EnvVarGuard::remove("DEEPSEEK_CONFIG_PATH"); + let project_dir = dir.path().join(".codewhale"); + let legacy_trust_dir = dir.path().join(".deepseek"); + std::fs::create_dir_all(&project_dir).expect("mkdir .codewhale"); + std::fs::create_dir_all(&legacy_trust_dir).expect("mkdir .deepseek"); + std::fs::write(legacy_trust_dir.join("trusted"), "").expect("write legacy trust marker"); + std::fs::write( + project_dir.join("hooks.toml"), + r#" +[[hooks]] +event = "tool_call_before" +command = "echo project" +"#, + ) + .expect("write hooks.toml"); + + let global = HooksConfig { + enabled: true, + hooks: vec![Hook::new(HookEvent::ToolCallBefore, "echo global")], + ..HooksConfig::default() + }; + + let merged = HooksConfig::load_with_project(global, dir.path()); + assert_eq!(merged.hooks.len(), 1); + assert_eq!(merged.hooks[0].command, "echo global"); + } + + #[test] + fn load_with_project_malformed_file_falls_back_to_global() { + let _lock = lock_test_env(); + let dir = tempfile::tempdir().expect("tempdir"); + let config_path = dir.path().join("user-config.toml"); + let _config = trust_workspace_for_project_hooks(dir.path(), &config_path); + let _legacy_config = EnvVarGuard::remove("DEEPSEEK_CONFIG_PATH"); let project_dir = dir.path().join(".codewhale"); std::fs::create_dir_all(&project_dir).expect("mkdir .codewhale"); std::fs::write(project_dir.join("hooks.toml"), "this is [ not toml") diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index b9fab27a..3b18eafd 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -3202,6 +3202,15 @@ async fn run_event_loop( match onboarding::mark_trusted(&app.workspace) { Ok(_) => { app.trust_mode = true; + app.hooks = HookExecutor::new( + crate::hooks::HooksConfig::load_with_project( + config.hooks_config(), + &app.workspace, + ), + app.workspace.clone(), + ); + app.runtime_services.hook_executor = + Some(std::sync::Arc::new(app.hooks.clone())); app.status_message = None; if app.onboarding_workspace_trust_gate { app.onboarding_workspace_trust_gate = false; diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c85cc768..a22ff415 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -691,10 +691,16 @@ metacharacters in the pattern are matched literally. Repositories can ship policy in `/.codewhale/hooks.toml`, using the same shape as the `[hooks]` table (top-level fields plus -`[[hooks]]` entries). Project hooks are appended after global hooks -from `config.toml`, so they run last and, for `updatedInput`, win -ties. A malformed project file logs a warning and startup falls back -to global hooks only. +`[[hooks]]` entries). Project hooks are executable shell +configuration, so CodeWhale only loads them after the workspace has +been trusted in user-owned config through the trust prompt or a +`[projects.""] trust_level = "trusted"` entry. Session +`/trust on` mode does not enable repo-supplied hooks by itself, and +repo-local legacy markers such as `.deepseek/trusted` do not enable +project hooks. Once trusted, project hooks are appended after global +hooks from `config.toml`, so they run last and, for `updatedInput`, +win ties. A malformed trusted project file logs a warning and startup +falls back to global hooks only. ```toml # .codewhale/hooks.toml