Skip to content
This repository was archived by the owner on Jul 10, 2026. It is now read-only.

Commit bdfdffd

Browse files
authored
fix(bootstrap): pin Claude hook binary path (#169)
Generate Claude Code hook commands with the installed ~/.harness-kit/bin/harness-kit-checks path instead of relying on PATH resolution, and add a check-claude-hook-commands verifier that fails when hook binaries are missing or non-executable. Evidence: cargo run --locked -p harness-kit-checks -- check --repo . Evidence: temp HOME bootstrap with minimal PATH produced 5 claude-hook commands, 0 bare harness-kit-checks hook lines, and check-claude-hook-commands failed as expected after corrupting the binary path. Fixes: #140 Agent: Codex Agent-Surface: Codex CLI Agent-Model: gpt-5 Agent-Task: GitHub issue #140
1 parent e475dcf commit bdfdffd

6 files changed

Lines changed: 387 additions & 8 deletions

File tree

‎crates/harness-kit-checks/src/bootstrap.rs‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,10 @@ fn link_harness(
367367
&[],
368368
lines,
369369
)?;
370-
copy_if_present(
370+
copy_claude_settings(
371371
&repo.join("harnesses/claude/settings.json"),
372372
&harness_dir.join("settings.json"),
373+
&crate::claude_settings::installed_cli_for_claude_dir(harness_dir),
373374
"settings.json (copied)",
374375
lines,
375376
)?;
@@ -576,14 +577,18 @@ fn link_if_present(src: &Path, dest: &Path, label: &str, lines: &mut Vec<String>
576577
Ok(())
577578
}
578579

579-
fn copy_if_present(src: &Path, dest: &Path, label: &str, lines: &mut Vec<String>) -> Result<()> {
580-
if src.exists() {
581-
if let Some(parent) = dest.parent() {
582-
fs::create_dir_all(parent)?;
583-
}
584-
fs::copy(src, dest)?;
585-
lines.push(green(format!(" {label}")));
580+
fn copy_claude_settings(
581+
src: &Path,
582+
dest: &Path,
583+
installed_cli: &Path,
584+
label: &str,
585+
lines: &mut Vec<String>,
586+
) -> Result<()> {
587+
if !src.exists() {
588+
return Ok(());
586589
}
590+
crate::claude_settings::install_rendered_settings(src, dest, installed_cli)?;
591+
lines.push(green(format!(" {label}")));
587592
Ok(())
588593
}
589594

@@ -747,4 +752,47 @@ mod tests {
747752
assert!(hooks_dir.path().join("live.py").exists());
748753
Ok(())
749754
}
755+
756+
#[test]
757+
fn bootstrap_writes_claude_hooks_with_installed_cli_path() -> Result<()> {
758+
let repo = tempfile::tempdir()?;
759+
fs::create_dir_all(repo.path().join("skills/demo"))?;
760+
fs::write(
761+
repo.path().join("skills/demo/SKILL.md"),
762+
"---\nname: demo\n---\n",
763+
)?;
764+
fs::create_dir_all(repo.path().join("harnesses/claude"))?;
765+
fs::write(
766+
repo.path().join("harnesses/claude/settings.json"),
767+
include_str!("../../../harnesses/claude/settings.json"),
768+
)?;
769+
fs::create_dir_all(repo.path().join("target/debug"))?;
770+
let build = repo.path().join("target/debug/harness-kit-checks");
771+
fs::write(&build, b"fake executable")?;
772+
#[cfg(unix)]
773+
{
774+
use std::os::unix::fs::PermissionsExt;
775+
fs::set_permissions(&build, fs::Permissions::from_mode(0o755))?;
776+
}
777+
778+
let home_root = tempfile::tempdir()?;
779+
let home = home_root.path().join("home with spaces");
780+
fs::create_dir_all(home.join(".claude"))?;
781+
782+
let output = run(&BootstrapOptions {
783+
repo: repo.path().to_path_buf(),
784+
home: home.clone(),
785+
bundle: None,
786+
dry_run: false,
787+
})?;
788+
assert!(output.contains("settings.json (copied)"));
789+
790+
let settings_path = home.join(".claude/settings.json");
791+
let settings = fs::read_to_string(&settings_path)?;
792+
let installed_cli = crate::claude_settings::installed_cli_path(&home);
793+
assert!(!settings.contains("\"harness-kit-checks claude-hook"));
794+
assert!(settings.contains(&installed_cli.to_string_lossy().to_string()));
795+
assert!(crate::claude_settings::validate_settings_file(&settings_path)?.is_empty());
796+
Ok(())
797+
}
750798
}
Lines changed: 278 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
1+
use std::collections::BTreeSet;
2+
use std::env;
3+
use std::fs;
4+
use std::path::{Path, PathBuf};
5+
6+
use anyhow::{Context, Result};
7+
use serde_json::Value;
8+
9+
const BARE_HOOK_PREFIX: &str = "harness-kit-checks claude-hook ";
10+
const EXPECTED_GENERATED_HOOKS: &[&str] = &[
11+
"permission-auto-approve",
12+
"destructive-command-guard",
13+
"github-cli-guard",
14+
"time-context",
15+
"skill-invocation-tracker",
16+
];
17+
18+
pub fn installed_cli_path(home: &Path) -> PathBuf {
19+
home.join(".harness-kit/bin/harness-kit-checks")
20+
}
21+
22+
pub fn render_with_cli_path(settings_text: &str, cli_path: &Path) -> Result<String> {
23+
let mut value: Value =
24+
serde_json::from_str(settings_text).context("failed to parse Claude settings JSON")?;
25+
rewrite_hook_commands(&mut value, &shell_quote(cli_path));
26+
Ok(format!("{}\n", serde_json::to_string_pretty(&value)?))
27+
}
28+
29+
pub fn validate_settings_file(settings_path: &Path) -> Result<Vec<String>> {
30+
let text = fs::read_to_string(settings_path)
31+
.with_context(|| format!("failed to read {}", settings_path.display()))?;
32+
validate_settings_text(&text)
33+
.with_context(|| format!("failed to validate {}", settings_path.display()))
34+
}
35+
36+
pub fn install_rendered_settings(src: &Path, dest: &Path, installed_cli: &Path) -> Result<()> {
37+
if let Some(parent) = dest.parent() {
38+
fs::create_dir_all(parent)?;
39+
}
40+
let source =
41+
fs::read_to_string(src).with_context(|| format!("failed to read {}", src.display()))?;
42+
let rendered = render_with_cli_path(&source, installed_cli)?;
43+
fs::write(dest, rendered).with_context(|| format!("failed to write {}", dest.display()))?;
44+
let errors = validate_settings_file(dest)?;
45+
if !errors.is_empty() {
46+
anyhow::bail!(
47+
"generated Claude settings contain unresolvable harness-kit hook commands:\n{}",
48+
errors.join("\n")
49+
);
50+
}
51+
Ok(())
52+
}
53+
54+
pub fn installed_cli_for_claude_dir(harness_dir: &Path) -> PathBuf {
55+
let home = harness_dir.parent().unwrap_or(harness_dir);
56+
installed_cli_path(home)
57+
}
58+
59+
pub fn validate_settings_text(settings_text: &str) -> Result<Vec<String>> {
60+
let value: Value =
61+
serde_json::from_str(settings_text).context("failed to parse Claude settings JSON")?;
62+
let commands = collect_claude_hook_commands(&value);
63+
let mut errors = Vec::new();
64+
65+
if commands.is_empty() {
66+
errors.push("Claude settings contain no harness-kit claude-hook commands".to_string());
67+
return Ok(errors);
68+
}
69+
70+
let mut observed_hooks = BTreeSet::new();
71+
for command in &commands {
72+
if let Some(hook_name) = claude_hook_name(command) {
73+
observed_hooks.insert(hook_name.to_string());
74+
}
75+
match first_shell_word(command) {
76+
Some(binary) if command_binary_resolves(&binary) => {}
77+
Some(binary) => errors.push(format!(
78+
"Claude hook command is not resolvable: {command:?} (binary {binary:?} is missing or not executable)"
79+
)),
80+
None => errors.push(format!(
81+
"Claude hook command has no parseable binary: {command:?}"
82+
)),
83+
}
84+
}
85+
86+
for expected in EXPECTED_GENERATED_HOOKS {
87+
if !observed_hooks.contains(*expected) {
88+
errors.push(format!("Claude hook command missing: {expected}"));
89+
}
90+
}
91+
92+
Ok(errors)
93+
}
94+
95+
fn rewrite_hook_commands(value: &mut Value, quoted_cli_path: &str) {
96+
match value {
97+
Value::Object(map) => {
98+
if let Some(Value::String(command)) = map.get_mut("command")
99+
&& let Some(rest) = command.strip_prefix(BARE_HOOK_PREFIX)
100+
{
101+
*command = format!("{quoted_cli_path} claude-hook {rest}");
102+
}
103+
for child in map.values_mut() {
104+
rewrite_hook_commands(child, quoted_cli_path);
105+
}
106+
}
107+
Value::Array(items) => {
108+
for child in items {
109+
rewrite_hook_commands(child, quoted_cli_path);
110+
}
111+
}
112+
_ => {}
113+
}
114+
}
115+
116+
fn collect_claude_hook_commands(value: &Value) -> Vec<String> {
117+
let mut commands = Vec::new();
118+
collect_claude_hook_commands_into(value, &mut commands);
119+
commands
120+
}
121+
122+
fn collect_claude_hook_commands_into(value: &Value, commands: &mut Vec<String>) {
123+
match value {
124+
Value::Object(map) => {
125+
if let Some(Value::String(command)) = map.get("command")
126+
&& claude_hook_name(command).is_some()
127+
{
128+
commands.push(command.clone());
129+
}
130+
for child in map.values() {
131+
collect_claude_hook_commands_into(child, commands);
132+
}
133+
}
134+
Value::Array(items) => {
135+
for child in items {
136+
collect_claude_hook_commands_into(child, commands);
137+
}
138+
}
139+
_ => {}
140+
}
141+
}
142+
143+
fn claude_hook_name(command: &str) -> Option<&str> {
144+
command
145+
.split_once("claude-hook ")
146+
.and_then(|(_, rest)| rest.split_whitespace().next())
147+
}
148+
149+
fn command_binary_resolves(binary: &str) -> bool {
150+
let path = Path::new(binary);
151+
if binary.contains('/') {
152+
return path.is_absolute() && is_executable_file(path);
153+
}
154+
env::var_os("PATH")
155+
.map(|path_var| {
156+
env::split_paths(&path_var).any(|dir| is_executable_file(&dir.join(binary)))
157+
})
158+
.unwrap_or(false)
159+
}
160+
161+
fn is_executable_file(path: &Path) -> bool {
162+
let Ok(metadata) = fs::metadata(path) else {
163+
return false;
164+
};
165+
if !metadata.is_file() {
166+
return false;
167+
}
168+
#[cfg(unix)]
169+
{
170+
use std::os::unix::fs::PermissionsExt;
171+
metadata.permissions().mode() & 0o111 != 0
172+
}
173+
#[cfg(not(unix))]
174+
{
175+
true
176+
}
177+
}
178+
179+
fn shell_quote(path: &Path) -> String {
180+
let value = path.to_string_lossy();
181+
format!("'{}'", value.replace('\'', r#"'\''"#))
182+
}
183+
184+
fn first_shell_word(command: &str) -> Option<String> {
185+
let mut chars = command.trim_start().chars().peekable();
186+
chars.peek()?;
187+
188+
let mut word = String::new();
189+
let mut in_single = false;
190+
let mut in_double = false;
191+
192+
while let Some(ch) = chars.next() {
193+
match ch {
194+
'\'' if !in_double => in_single = !in_single,
195+
'"' if !in_single => in_double = !in_double,
196+
'\\' if !in_single => {
197+
if let Some(next) = chars.next() {
198+
word.push(next);
199+
}
200+
}
201+
ch if ch.is_whitespace() && !in_single && !in_double => break,
202+
ch => word.push(ch),
203+
}
204+
}
205+
206+
if in_single || in_double || word.is_empty() {
207+
None
208+
} else {
209+
Some(word)
210+
}
211+
}
212+
213+
#[cfg(test)]
214+
mod tests {
215+
use super::*;
216+
217+
#[test]
218+
fn render_rewrites_bare_claude_hook_commands_to_installed_cli_path() -> Result<()> {
219+
let rendered = render_with_cli_path(
220+
r#"{
221+
"hooks": {
222+
"PreToolUse": [
223+
{"hooks": [
224+
{"command": "harness-kit-checks claude-hook permission-auto-approve"},
225+
{"command": "harness-kit-checks claude-hook destructive-command-guard"},
226+
{"command": "bash ~/.claude/statusline-command.sh"}
227+
]}
228+
],
229+
"SessionStart": [{"hooks": [
230+
{"command": "harness-kit-checks claude-hook time-context"}
231+
]}],
232+
"PostToolUse": [{"hooks": [
233+
{"command": "harness-kit-checks claude-hook skill-invocation-tracker"}
234+
]}]
235+
}
236+
}"#,
237+
Path::new("/tmp/home with spaces/.harness-kit/bin/harness-kit-checks"),
238+
)?;
239+
240+
assert!(!rendered.contains("\"harness-kit-checks claude-hook"));
241+
assert!(rendered.contains(
242+
"'/tmp/home with spaces/.harness-kit/bin/harness-kit-checks' claude-hook permission-auto-approve"
243+
));
244+
assert!(rendered.contains("bash ~/.claude/statusline-command.sh"));
245+
Ok(())
246+
}
247+
248+
#[test]
249+
fn validate_settings_fails_loud_for_missing_hook_binary() -> Result<()> {
250+
let errors = validate_settings_text(
251+
r#"{
252+
"hooks": {"SessionStart": [{"hooks": [
253+
{"command": "'/tmp/definitely-missing-harness-kit-checks' claude-hook time-context"}
254+
]}]}
255+
}"#,
256+
)?;
257+
258+
assert!(
259+
errors
260+
.iter()
261+
.any(|error| error.contains("is missing or not executable"))
262+
);
263+
assert!(
264+
errors
265+
.iter()
266+
.any(|error| error.contains("permission-auto-approve"))
267+
);
268+
Ok(())
269+
}
270+
271+
#[test]
272+
fn first_shell_word_handles_single_quote_escapes() {
273+
assert_eq!(
274+
first_shell_word(r#"'/tmp/O'\''Brien/harness-kit-checks' claude-hook time-context"#),
275+
Some("/tmp/O'Brien/harness-kit-checks".to_string())
276+
);
277+
}
278+
}

‎crates/harness-kit-checks/src/cli_usage.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ pub fn usage() -> ! {
1010
harness-kit-checks build-docs-site [--repo PATH] [--output PATH]
1111
harness-kit-checks check-docs-site [--repo PATH] [--site PATH] [--self-test]
1212
harness-kit-checks check-exclusions|check-conflict-markers|check-portable-paths|check-no-claims|check-vendored-copies|check-harness-install-paths [--repo PATH]
13+
harness-kit-checks check-claude-hook-commands [--settings PATH]
1314
harness-kit-checks check-godfiles [--write-baseline]|check-source-markers|check-supply-chain|check-supply-chain-advisories|check-template [--repo PATH]
1415
harness-kit-checks lint-external-skills [--strict]
1516
harness-kit-checks sync-external [--repo PATH] [--check] [--allow-floating] [--only owner/repo]

‎crates/harness-kit-checks/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ pub mod backlog;
22
pub mod bootstrap;
33
mod bundles;
44
pub mod ci_check;
5+
pub mod claude_settings;
56
mod cli_install;
67
pub mod cli_usage;
78
pub mod config_loader;

0 commit comments

Comments
 (0)