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.