From ab930c70998812b99efe2516e1fa6a33e9a643bb Mon Sep 17 00:00:00 2001 From: Transaction Science Date: Sat, 17 Oct 2026 04:41:54 -0400 Subject: [PATCH] joulehook: an unrecognised hook payload fails closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `run()` had three paths out before the gate, and they disagreed on which way to fail. Unreadable stdin denied. Unparseable JSON denied, with the reason spelled out: "Fail closed: an unparseable hook payload denies (the safe default §8)" — and a test, `malformed_input_fails_ closed`, pinning it. But a payload that parsed and whose action could not be extracted *allowed*, with exit 0 and no receipt sealed: None => { let (b, c) = render(host, true, "joulehook: no tool action"); return (b, c, vec![]); } That branch sat before the `cfg.enforcing()` check, so it fired while enforcing too, and "no tool action" asserts something the code does not know. What it knows is that `extract_action` recognised none of its three shapes — `tool_name`, `/tool_info/mcp_tool_name`, and `command` / `/tool_info/command_line`. SPEC §3.1 requires an adapter to map its host's payload onto `ActionEvent` "losslessly"; it does not offer allowing as the alternative when it cannot. The gap is reachable through this crate's own documented wiring, not a hypothetical host. SPEC §3.4 lists Windsurf's gate mechanism as "Cascade hooks (`pre_run_command`, `pre_write_code`, …)" and INSTALL.md directs the operator to wire this binary into them. A `pre_write_code` payload carries a file path — not `tool_name`, not `mcp_tool_name`, not `command`/`command_line` — so with `JOULEHOOK_TOOLS` set the write was allowed, the process exited 0, and nothing was appended to `JOULEHOOK_RECEIPT_LOG`. The operator saw an enforcing gate and a clean decision log while every code write passed ungated. Any host that renames its tool field turns the whole gate into a blanket allow the same way, with no signal. The crate header claims the opposite: a tool the capability set does not hold "is **denied before it executes**, because the held capabilities decide, not the model." An unrecognised shape now denies, with a reason that states the actual condition rather than asserting nothing was pending. The observe-mode check moves above the mapping, because observe mode allows by contract and so must not depend on recognising the payload — that keeps observe mode's behaviour and its §7 non-enforcement disclosure unchanged. No receipt is sealed on this path, matching the malformed-JSON path above it: both reject before an `ActionEvent` exists to seal a decision about. Tests assert the fixture really is an unmappable shape, so they cannot pass vacuously; that it denies while enforcing on both a body-gated and an exit-code-gated host; that the reason no longer claims "no tool action"; and that observe mode still allows it and still discloses. Co-Authored-By: Claude Opus 5 --- .../crates/joulehook-hook/src/main.rs | 73 +++++++++++++++++-- 1 file changed, 66 insertions(+), 7 deletions(-) diff --git a/joulehook/joulehook-rs/crates/joulehook-hook/src/main.rs b/joulehook/joulehook-rs/crates/joulehook-hook/src/main.rs index e9dfc5255..82abb555b 100644 --- a/joulehook/joulehook-rs/crates/joulehook-hook/src/main.rs +++ b/joulehook/joulehook-rs/crates/joulehook-hook/src/main.rs @@ -236,15 +236,10 @@ fn run(host: Host, input: &str, cfg: &Config) -> (String, i32, Vec>) { return (b, c, vec![]); } }; - let event = match extract_action(&value) { - Some(e) => e, - None => { - let (b, c) = render(host, true, "joulehook: no tool action"); - return (b, c, vec![]); - } - }; if !cfg.enforcing() { // Observe mode — no allow-list. Disclose non-enforcement (SPEC §7). + // Checked before the mapping below: observe mode allows by contract, so + // it must not depend on recognising the payload. let (b, c) = render( host, true, @@ -252,6 +247,25 @@ fn run(host: Host, input: &str, cfg: &Config) -> (String, i32, Vec>) { ); return (b, c, vec![]); } + let event = match extract_action(&value) { + Some(e) => e, + // Fail closed, like the malformed payload above. A shape this adapter + // does not recognise means it could not map the payload onto an + // `ActionEvent` — SPEC §3.1 requires that mapping to be lossless — not + // that no action is pending, which is what allowing here asserted. + // + // The two are not interchangeable while enforcing. SPEC §3.4 lists + // Windsurf's `pre_write_code` among this binary's binding targets, and + // that payload carries a file path, not `tool_name`, `mcp_tool_name` or + // `command`/`command_line`. So every code write was allowed with exit 0 + // and no receipt, while the operator saw an enforcing gate and a clean + // decision log. Any host renaming its tool field turns the whole gate + // into a blanket allow the same way. + None => { + let (b, c) = render(host, false, "joulehook: unrecognised hook payload shape"); + return (b, c, vec![]); + } + }; let (allow, reason, receipts) = gate(&event, cfg); let (b, c) = render(host, allow, &reason); (b, c, receipts) @@ -359,6 +373,51 @@ mod tests { assert_eq!(field(&out, "/hookSpecificOutput/permissionDecision"), "deny"); } + /// A payload shape this adapter cannot map is not "no action pending". + /// + /// SPEC §3.4 lists Windsurf's `pre_write_code` as a binding target. Its + /// payload carries a file path and none of the three keys `extract_action` + /// recognises, so it used to be allowed with exit 0 and no receipt even + /// while enforcing — a blanket allow on every code write. + fn pre_write_code_json() -> String { + serde_json::json!({ + "hook_event_name": "pre_write_code", + "tool_info": { "file_path": "/repo/src/main.rs", "content": "fn main() {}" } + }) + .to_string() + } + + #[test] + fn an_unrecognised_payload_shape_fails_closed_when_enforcing() { + let input = pre_write_code_json(); + let v: Value = serde_json::from_str(&input).unwrap(); + assert!( + extract_action(&v).is_none(), + "the fixture has to be a shape the adapter cannot map, or this tests nothing" + ); + + let (out, _, _) = run(Host::ClaudeCode, &input, &cfg(&["Read", "Grep"])); + assert_eq!(field(&out, "/hookSpecificOutput/permissionDecision"), "deny"); + assert!( + !out.contains("no tool action"), + "the reason must not claim nothing was pending: {out}" + ); + + // And on the host that gates by exit code, it has to be the blocking one. + let (_, code, _) = run(Host::Windsurf, &input, &cfg(&["Read"])); + assert_eq!(code, 2, "Cascade blocks via exit code 2"); + } + + /// Observe mode still allows it, and still discloses that it is not + /// enforcing: its contract does not depend on recognising the payload. + #[test] + fn an_unrecognised_payload_shape_is_allowed_in_observe_mode() { + let (out, _, receipts) = run(Host::ClaudeCode, &pre_write_code_json(), &cfg(&[])); + assert_eq!(field(&out, "/hookSpecificOutput/permissionDecision"), "allow"); + assert!(out.contains("observe mode")); + assert!(receipts.is_empty()); + } + #[test] fn extracts_windsurf_and_shell_shapes() { // Windsurf pre_mcp_tool_use. -- 2.54.0 (Apple Git-157)