From 2151364dd58fc1283cfce55300500f975dd9bdf0 Mon Sep 17 00:00:00 2001 From: Max Haarhaus Date: Wed, 15 Jul 2026 15:39:55 -0400 Subject: [PATCH 1/5] test(guard): byte-pin deny verdicts and fresh hook files Full-string pins of the Claude/Codex deny-verdict bytes (CLI level) and the freshly installed hook-file contents (adapter level), captured against the current per-harness guard code. These are the compatibility lock for the upcoming generic-engine refactor (#137): armed hooks and their backups are an on-disk contract, so the refactor must reproduce these bytes exactly. Co-Authored-By: Claude Fable 5 --- src/sandbox/install.rs | 70 ++++++++++++++++++++++++++++++++++++++++++ tests/cli/guard.rs | 53 ++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/src/sandbox/install.rs b/src/sandbox/install.rs index e322758..ff9d772 100644 --- a/src/sandbox/install.rs +++ b/src/sandbox/install.rs @@ -240,6 +240,76 @@ mod tests { } } + /// Byte-pin of a fresh Claude hook file: armed envs and their backups are an + /// on-disk compatibility surface, so the merged settings must keep this exact + /// 2-space-pretty shape, key order, and trailing newline. + #[test] + fn claude_install_writes_this_exact_hook_file() { + let c = setup(); + let adapter = crate::adapters::adapter_for(Harness::resolve("claude-code").unwrap()); + let marker = adapter + .install_guard(&c.stage_root, Path::new("/g/eval-magic"), None) + .unwrap(); + + let settings = + fs::read_to_string(c.stage_root.join(".claude").join("settings.local.json")).unwrap(); + let expected = format!( + r#"{{ + "hooks": {{ + "PreToolUse": [ + {{ + "matcher": "Write|Edit|MultiEdit|NotebookEdit|Bash", + "hooks": [ + {{ + "type": "command", + "command": "\"/g/eval-magic\" guard \"{marker}\"" + }} + ] + }} + ] + }} +}} +"#, + marker = marker.display() + ); + assert_eq!(settings, expected); + } + + /// Byte-pin of a fresh Codex hook file — same compatibility contract as the + /// Claude pin, plus the Codex-only `timeout`/`statusMessage` keys. + #[test] + fn codex_install_writes_this_exact_hook_file() { + let c = setup(); + let adapter = crate::adapters::adapter_for(Harness::resolve("codex").unwrap()); + let marker = adapter + .install_guard(&c.stage_root, Path::new("/g/eval-magic"), None) + .unwrap(); + + let hooks = fs::read_to_string(c.stage_root.join(".codex").join("hooks.json")).unwrap(); + let expected = format!( + r#"{{ + "hooks": {{ + "PreToolUse": [ + {{ + "matcher": "^Bash$|^apply_patch$|^Edit$|^Write$", + "hooks": [ + {{ + "type": "command", + "command": "\"/g/eval-magic\" guard-codex \"{marker}\"", + "timeout": 30, + "statusMessage": "Checking eval write boundary" + }} + ] + }} + ] + }} +}} +"#, + marker = marker.display() + ); + assert_eq!(hooks, expected); + } + #[test] fn guard_is_armed_detects_claude_or_codex_marker() { let c = setup(); diff --git a/tests/cli/guard.rs b/tests/cli/guard.rs index 9e05d28..a921fb6 100644 --- a/tests/cli/guard.rs +++ b/tests/cli/guard.rs @@ -105,6 +105,59 @@ fn guard_codex_subcommand_blocks_with_codex_verdict_shape() { .stdout(contains("blocked Bash")); } +/// Byte-pin of the Claude deny verdict: armed hooks from previous releases keep +/// reading this exact shape, so the serialized bytes are a compatibility +/// contract — full-string equality, not substrings. +#[test] +fn guard_deny_verdict_bytes_are_stable() { + let tmp = TempDir::new().unwrap(); + let marker = tmp.path().join("marker.json"); + fs::write( + &marker, + r#"{"active":true,"allowedRoots":["/work/env"],"expiresAt":"2999-01-01T00:00:00.000Z"}"#, + ) + .unwrap(); + + skill_eval() + .arg("guard") + .arg(&marker) + .write_stdin(r#"{ "tool_name": "Write", "tool_input": { "file_path": "/etc/passwd" } }"#) + .assert() + .success() + .stdout( + "{\"hookSpecificOutput\":{\"hookEventName\":\"PreToolUse\",\ + \"permissionDecision\":\"deny\",\"permissionDecisionReason\":\ + \"eval guard: Write to /etc/passwd is outside the eval sandbox \ + (allowed: /work/env)\"}}", + ); +} + +/// Byte-pin of the Codex block verdict — same compatibility contract as the +/// Claude pin above. +#[test] +fn guard_codex_block_verdict_bytes_are_stable() { + let tmp = TempDir::new().unwrap(); + let marker = tmp.path().join("marker.json"); + fs::write( + &marker, + r#"{"active":true,"allowedRoots":["/work/env"],"expiresAt":"2999-01-01T00:00:00.000Z"}"#, + ) + .unwrap(); + + skill_eval() + .arg("guard-codex") + .arg(&marker) + .write_stdin( + r#"{ "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#, + ) + .assert() + .success() + .stdout( + "{\"decision\":\"block\",\"reason\":\"eval guard: blocked Bash \ + (package install/add) — runs outside the eval sandbox\"}", + ); +} + /// `guard` fails open when the marker is absent: empty stdout, exit 0. #[test] fn guard_fails_open_without_marker() { From accdaba7e326d090564fb96af2fb9070f4eff0c6 Mon Sep 17 00:00:00 2001 From: Max Haarhaus Date: Wed, 15 Jul 2026 15:52:01 -0400 Subject: [PATCH 2/5] feat(guard): generic guard engine over embedded descriptor data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapse the per-harness write-guard modules (claude_code/guard.rs, codex/guard.rs) into one engine (src/adapters/guard.rs) driven by the descriptor's [guard] data block. The four per-harness axes — hooks_file, matcher, hook_entry template, verdict_template — plus command_template move into harnesses/{claude-code,codex}.toml; the GuardEngine capability enum is gone. Templates are parsed to JSON and placeholder-substituted in string values, so key order (and therefore verdict bytes and hook-file shape) is authored data pinned by full-string tests. The teardown cleanup dir is now derived from hooks_file's parent instead of being per-harness code. Descriptor validation grows guard-template invariants (matcher⊆vocabulary now reads guard.matcher; hook_entry/verdict_template must parse as JSON with {command}/{matcher}/{reason} placeholders in string values; command_template must carry {exe}/{marker}; hooks_file must stay relative). The guard↔banner lockstep and guard-requires-skills_dir invariants carry over unchanged. The hidden guard / guard-codex subcommands stay as frozen aliases over a new generic run_guard_hook handler resolving the verdict through the adapter's new guard_verdict method (default fails open). Verdict bytes verified identical against a pre-change binary. Part of #137. Co-Authored-By: Claude Fable 5 --- harnesses/claude-code.toml | 10 +- harnesses/codex.toml | 11 +- schema/harness-descriptor.schema.json | 39 +- src/adapters/capabilities.rs | 64 +-- src/adapters/claude_code/guard.rs | 348 -------------- src/adapters/claude_code/mod.rs | 12 +- src/adapters/codex/guard.rs | 260 ----------- src/adapters/codex/mod.rs | 10 +- src/adapters/descriptor.rs | 65 ++- src/adapters/descriptor/layers.rs | 8 +- src/adapters/descriptor/validation.rs | 181 +++++++- src/adapters/descriptor_adapter.rs | 20 +- src/adapters/guard.rs | 638 ++++++++++++++++++++++++++ src/adapters/harness.rs | 10 + src/adapters/mod.rs | 4 +- src/adapters/registry.rs | 22 +- src/cli/commands/guard.rs | 77 ++-- src/sandbox/install.rs | 6 +- src/sandbox/mod.rs | 12 +- tests/cli/harness.rs | 12 +- 20 files changed, 1038 insertions(+), 771 deletions(-) delete mode 100644 src/adapters/claude_code/guard.rs delete mode 100644 src/adapters/codex/guard.rs create mode 100644 src/adapters/guard.rs diff --git a/harnesses/claude-code.toml b/harnesses/claude-code.toml index 12d0612..3418f35 100644 --- a/harnesses/claude-code.toml +++ b/harnesses/claude-code.toml @@ -39,8 +39,16 @@ surfaces_skill_invocation = true [model] flag = "--model" +# Guard data rendered by the generic engine (src/adapters/guard.rs). The JSON +# templates' key order is written/serialized verbatim and is an on-disk +# contract with armed hooks from earlier releases — do not reorder keys. The +# `guard` subcommand in command_template is a frozen alias for the same reason. [guard] -engine = "claude-hooks" +hooks_file = ".claude/settings.local.json" +matcher = "Write|Edit|MultiEdit|NotebookEdit|Bash" +command_template = '"{exe}" guard "{marker}"' +hook_entry = '{"matcher":"{matcher}","hooks":[{"type":"command","command":"{command}"}]}' +verdict_template = '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"{reason}"}}' armed_message = ''' 🛡 Write guard armed: a PreToolUse hook is staged in .claude/settings.local.json diff --git a/harnesses/codex.toml b/harnesses/codex.toml index 87179c9..901a53b 100644 --- a/harnesses/codex.toml +++ b/harnesses/codex.toml @@ -44,8 +44,17 @@ surfaces_skill_invocation = false [model] flag = "-m" +# Guard data rendered by the generic engine (src/adapters/guard.rs). The JSON +# templates' key order is written/serialized verbatim and is an on-disk +# contract with armed hooks from earlier releases — do not reorder keys. The +# `guard-codex` subcommand in command_template is a frozen alias for the same +# reason. [guard] -engine = "codex-hooks" +hooks_file = ".codex/hooks.json" +matcher = "^Bash$|^apply_patch$|^Edit$|^Write$" +command_template = '"{exe}" guard-codex "{marker}"' +hook_entry = '{"matcher":"{matcher}","hooks":[{"type":"command","command":"{command}","timeout":30,"statusMessage":"Checking eval write boundary"}]}' +verdict_template = '{"decision":"block","reason":"{reason}"}' armed_message = ''' 🛡 Write guard armed: a PreToolUse hook is staged in .codex/hooks.json diff --git a/schema/harness-descriptor.schema.json b/schema/harness-descriptor.schema.json index b0e1854..4467e85 100644 --- a/schema/harness-descriptor.schema.json +++ b/schema/harness-descriptor.schema.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "$id": "https://slow-powers.dev/schemas/harness-descriptor.schema.json", "title": "Harness Descriptor", - "description": "The data half of a harness adapter: every declarative value a harness exposes, plus references to named code capabilities (transcript parsers, guard engines, slug generation, shadow preflight). Authored as TOML (harnesses/*.toml), validated after transcoding to JSON. Cross-field invariants (guard/banner lockstep, slug/naming-rule agreement, vocabulary coverage) are enforced by the Rust loader on top of this schema.", + "description": "The data half of a harness adapter: every declarative value a harness exposes, plus references to named code capabilities (transcript parsers, slug generation, shadow preflight). The write guard is pure data (the [guard] block) rendered by one generic engine. Authored as TOML (harnesses/*.toml), validated after transcoding to JSON. Cross-field invariants (guard/banner lockstep, slug/naming-rule agreement, vocabulary coverage, guard template contracts) are enforced by the Rust loader on top of this schema.", "type": "object", "required": ["label"], "additionalProperties": false, @@ -106,11 +106,42 @@ }, "guard": { "type": "object", - "required": ["engine", "armed_message"], + "required": [ + "hooks_file", + "matcher", + "command_template", + "hook_entry", + "verdict_template", + "armed_message" + ], "additionalProperties": false, - "description": "Write guard: the named hook engine and the post-arm banner.", + "description": "Write guard: the data the generic guard engine renders into a PreToolUse hook and a deny verdict. Restricted to embedded built-in descriptors (the guard fails open, so a mistyped user descriptor would silently disarm it).", "properties": { - "engine": { "enum": ["claude-hooks", "codex-hooks"] }, + "hooks_file": { + "type": "string", + "minLength": 1, + "description": "Hook-config file the entry is merged into, `/`-separated relative to the staged env root (e.g. \".claude/settings.local.json\")." + }, + "matcher": { + "type": "string", + "minLength": 1, + "description": "Tool-name matcher the hook registers for; every hooked tool must be declared in [tools] write/patch/shell." + }, + "command_template": { + "type": "string", + "minLength": 1, + "description": "Shell command the armed hook runs, with {exe} and {marker} placeholders (e.g. '\"{exe}\" guard \"{marker}\"')." + }, + "hook_entry": { + "type": "string", + "minLength": 1, + "description": "JSON template of the hook entry appended to the hook config's hooks.PreToolUse array, with {matcher} and {command} placeholders in its string values; authored key order is written verbatim." + }, + "verdict_template": { + "type": "string", + "minLength": 1, + "description": "JSON template of the deny verdict printed on stdout, with a {reason} placeholder; authored key order is the harness's on-disk contract." + }, "armed_message": { "type": "string", "minLength": 1 } } }, diff --git a/src/adapters/capabilities.rs b/src/adapters/capabilities.rs index 83a3f5c..d5f2333 100644 --- a/src/adapters/capabilities.rs +++ b/src/adapters/capabilities.rs @@ -1,19 +1,19 @@ //! Named code capabilities a harness descriptor references. //! //! Everything a descriptor cannot express as data — transcript stitching, -//! guard hook installation, slug sanitization, plugin-shadow scanning — lives -//! behind one of these closed enums. A descriptor opts in by naming the -//! capability (`parser = "codex-items"`, `engine = "claude-hooks"`); a harness -//! whose stream or hooks are compatible with an existing capability gets the -//! full feature from configuration alone. +//! slug sanitization, plugin-shadow scanning — lives behind one of these +//! closed enums. A descriptor opts in by naming the capability +//! (`parser = "codex-items"`); a harness whose stream is compatible with an +//! existing capability gets the full feature from configuration alone. (The +//! write guard needs no named capability: its install and verdict render from +//! the descriptor's `[guard]` data via [`super::guard`].) //! //! The enums deserialize from the kebab-case capability names the //! `harness-descriptor` schema also enumerates, so an unknown name fails the //! schema gate with a listed-allowed-values message before ever reaching Rust. use std::io; -use std::path::{Path, PathBuf}; -use std::time::Duration; +use std::path::Path; use serde::{Deserialize, Serialize}; @@ -55,56 +55,6 @@ impl TranscriptParser { } } -/// Write-guard engines: install a PreToolUse-style hook under the staged env. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] -#[serde(rename_all = "kebab-case")] -pub enum GuardEngine { - /// Hook merged into `.claude/settings.local.json`. - ClaudeHooks, - /// Hook merged into `.codex/hooks.json`. - CodexHooks, -} - -impl GuardEngine { - /// The tool-name matcher the engine's hook registers for. Exposed so - /// descriptor validation can prove every hooked tool is declared in the - /// descriptor's `[tools]` vocabulary. - pub(crate) fn hook_matcher(self) -> &'static str { - match self { - GuardEngine::ClaudeHooks => super::claude_code::guard::HOOK_MATCHER, - GuardEngine::CodexHooks => super::codex::guard::HOOK_MATCHER, - } - } - - /// Arm the write guard under `stage_root`, returning the staged marker - /// path. - pub(crate) fn install_guard( - self, - stage_root: &Path, - guard_exe: &Path, - ttl: Option, - ) -> io::Result { - match self { - GuardEngine::ClaudeHooks => { - super::claude_code::guard::install_guard(stage_root, guard_exe, ttl) - } - GuardEngine::CodexHooks => { - super::codex::guard::install_guard(stage_root, guard_exe, ttl) - } - } - } - - /// A hook-config dir the install created outside the skills dir, which - /// teardown prunes when restoring the original config leaves it empty. - pub(crate) fn hook_cleanup_dir(self, stage_root: &Path) -> Option { - match self { - // The Claude hook lives in .claude/, which staging already owns. - GuardEngine::ClaudeHooks => None, - GuardEngine::CodexHooks => Some(super::codex::guard::hook_cleanup_dir(stage_root)), - } - } -} - /// Staged-slug generators, for harnesses whose naming rules need /// sanitization/truncation beyond a format string. #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] diff --git a/src/adapters/claude_code/guard.rs b/src/adapters/claude_code/guard.rs deleted file mode 100644 index 4dd87d4..0000000 --- a/src/adapters/claude_code/guard.rs +++ /dev/null @@ -1,348 +0,0 @@ -//! Claude Code write-guard hook: install + verdict shape. -//! -//! Arms the guard by merging a `PreToolUse` hook into the env's -//! `.claude/settings.local.json`; each `claude -p` dispatch runs from the env -//! dir, so it loads and enforces the hook. The hook invokes the hidden `guard` -//! subcommand (a stable on-disk contract), whose deny verdict uses Claude -//! Code's native `hookSpecificOutput` shape. - -use std::fs; -use std::io; -use std::path::{Path, PathBuf}; -use std::time::Duration; - -use serde_json::{Value, json}; - -use crate::sandbox::decide::{GuardMarker, decide}; -use crate::sandbox::install::{ - GUARD_MANIFEST, GUARD_MARKER, write_json, write_manifest, write_marker, -}; -use crate::sandbox::{now_ms, parse_tool_call}; - -/// Tool names the Claude Code PreToolUse hook fires on. -pub(crate) const HOOK_MATCHER: &str = "Write|Edit|MultiEdit|NotebookEdit|Bash"; - -/// Arm the write guard using Claude Code's project-local hook surface. Returns -/// the staged marker path. -pub(crate) fn install_guard( - stage_root: &Path, - guard_exe: &Path, - ttl: Option, -) -> io::Result { - let skills_dir = stage_root.join(".claude").join("skills"); - fs::create_dir_all(&skills_dir)?; - - let marker_path = skills_dir.join(GUARD_MARKER); - write_marker(&marker_path, stage_root, ttl)?; - - let settings_path = stage_root.join(".claude").join("settings.local.json"); - let settings_existed = settings_path.exists(); - let backup = if settings_existed { - Some(fs::read_to_string(&settings_path)?) - } else { - None - }; - - // Start from the existing settings (or an empty object), preserving key - // order, then append the PreToolUse hook entry. - let mut settings: Value = backup - .as_deref() - .and_then(|s| serde_json::from_str(s).ok()) - .unwrap_or_else(|| json!({})); - let hooks = settings - .as_object_mut() - .expect("settings is a JSON object") - .entry("hooks") - .or_insert_with(|| json!({})); - let pre = hooks - .as_object_mut() - .expect("hooks is a JSON object") - .entry("PreToolUse") - .or_insert_with(|| json!([])); - let command = format!( - "\"{}\" guard \"{}\"", - guard_exe.display(), - marker_path.display() - ); - pre.as_array_mut() - .expect("PreToolUse is an array") - .push(json!({ - "matcher": HOOK_MATCHER, - "hooks": [ { "type": "command", "command": command } ], - })); - write_json(&settings_path, &settings)?; - - write_manifest( - &skills_dir.join(GUARD_MANIFEST), - &settings_path, - settings_existed, - backup, - &marker_path, - )?; - - Ok(marker_path) -} - -/// Evaluate a PreToolUse hook `payload` (the JSON Claude Code sends on stdin) -/// against `marker`. Returns the serialized deny verdict to print on stdout when -/// the call is blocked — Claude Code's native `hookSpecificOutput` shape — or -/// `None` to allow (print nothing). An empty or malformed payload is treated as -/// allow. -pub(crate) fn guard_decision(payload: &str, marker: Option) -> Option { - let (tool_name, tool_input) = parse_tool_call(payload)?; - - let decision = decide(&tool_name, &tool_input, marker.as_ref(), now_ms()); - if decision.allow { - return None; - } - Some( - serde_json::to_string(&json!({ - "hookSpecificOutput": { - "hookEventName": "PreToolUse", - "permissionDecision": "deny", - "permissionDecisionReason": decision.reason, - } - })) - .expect("deny verdict serializes"), - ) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::sandbox::install::{iso_millis, teardown_guard}; - use crate::sandbox::{guard_is_armed, now_ms}; - use chrono::DateTime; - use tempfile::TempDir; - - struct Case { - _tmp: TempDir, - stage_root: PathBuf, - } - - fn setup() -> Case { - let tmp = TempDir::new().unwrap(); - let stage_root = tmp.path().join("stage"); - fs::create_dir_all(&stage_root).unwrap(); - Case { - _tmp: tmp, - stage_root, - } - } - - fn skills_dir(stage_root: &Path) -> PathBuf { - stage_root.join(".claude").join("skills") - } - - fn settings_path(stage_root: &Path) -> PathBuf { - stage_root.join(".claude").join("settings.local.json") - } - - fn read_json(path: &Path) -> Value { - serde_json::from_str(&fs::read_to_string(path).unwrap()).unwrap() - } - - fn absolutize(p: &Path) -> PathBuf { - std::path::absolute(p).unwrap_or_else(|_| p.to_path_buf()) - } - - /// A live marker (active, no expiry → unexpired) scoped to one root. - fn marker() -> GuardMarker { - GuardMarker { - active: Some(true), - allowed_roots: Some(vec!["/work/.eval-magic".to_string()]), - expires_at: None, - } - } - - #[test] - fn install_writes_an_active_marker_hook_and_manifest() { - let c = setup(); - let exe = Path::new("/g/eval-magic"); - install_guard(&c.stage_root, exe, None).unwrap(); - - let marker = read_json(&skills_dir(&c.stage_root).join(GUARD_MARKER)); - assert_eq!(marker["active"], json!(true)); - let expires = marker["expiresAt"].as_str().unwrap(); - let exp_ms = DateTime::parse_from_rfc3339(expires) - .unwrap() - .timestamp_millis(); - assert!(exp_ms > now_ms()); - let env = absolutize(&c.stage_root).display().to_string(); - assert!( - marker["allowedRoots"] - .as_array() - .unwrap() - .iter() - .any(|r| r.as_str().unwrap() == env) - ); - - let settings = read_json(&settings_path(&c.stage_root)); - let hook = &settings["hooks"]["PreToolUse"][0]; - assert!(hook["matcher"].as_str().unwrap().contains("Write")); - assert!( - hook["hooks"][0]["command"] - .as_str() - .unwrap() - .contains("guard") - ); - - assert!(skills_dir(&c.stage_root).join(GUARD_MANIFEST).exists()); - } - - #[test] - fn marker_scopes_allowed_roots_to_the_env_and_temp_only() { - let c = setup(); - let exe = Path::new("/g/eval-magic"); - install_guard(&c.stage_root, exe, None).unwrap(); - - let marker = read_json(&skills_dir(&c.stage_root).join(GUARD_MARKER)); - let roots: Vec = marker["allowedRoots"] - .as_array() - .unwrap() - .iter() - .map(|r| r.as_str().unwrap().to_string()) - .collect(); - - // The guard boundary is the isolated env (stage_root) plus temp — nothing - // above it. The parent workspace tree must NOT be an allowed root, or the - // agent could write into sibling iterations / the meta dir above `env/`. - let env = absolutize(&c.stage_root).display().to_string(); - let temp = absolutize(&std::env::temp_dir()).display().to_string(); - assert_eq!(roots, vec![env, temp]); - assert!( - !roots.iter().any(|r| r.ends_with(".eval-magic")), - "workspace_root must not be an allowed root: {roots:?}" - ); - } - - #[test] - fn hook_command_invokes_the_binary_guard_subcommand() { - let c = setup(); - let exe = Path::new("/g/eval-magic"); - let marker = install_guard(&c.stage_root, exe, None).unwrap(); - let settings = read_json(&settings_path(&c.stage_root)); - let command = settings["hooks"]["PreToolUse"][0]["hooks"][0]["command"] - .as_str() - .unwrap() - .to_string(); - assert_eq!( - command, - format!("\"/g/eval-magic\" guard \"{}\"", marker.display()) - ); - } - - #[test] - fn teardown_deletes_settings_it_created() { - let c = setup(); - let exe = Path::new("/g/eval-magic"); - install_guard(&c.stage_root, exe, None).unwrap(); - assert!(settings_path(&c.stage_root).exists()); - - assert!(teardown_guard(&c.stage_root)); - assert!(!settings_path(&c.stage_root).exists()); - assert!(!skills_dir(&c.stage_root).join(GUARD_MARKER).exists()); - assert!(!skills_dir(&c.stage_root).join(GUARD_MANIFEST).exists()); - } - - #[test] - fn teardown_restores_a_pre_existing_settings_verbatim() { - let c = setup(); - fs::create_dir_all(c.stage_root.join(".claude")).unwrap(); - let original = format!( - "{}\n", - serde_json::to_string_pretty(&json!({ - "permissions": { "allow": ["Bash(ls)"] } - })) - .unwrap() - ); - fs::write(settings_path(&c.stage_root), &original).unwrap(); - - let exe = Path::new("/g/eval-magic"); - install_guard(&c.stage_root, exe, None).unwrap(); - // hook present while armed - assert!( - fs::read_to_string(settings_path(&c.stage_root)) - .unwrap() - .contains("PreToolUse") - ); - - teardown_guard(&c.stage_root); - assert_eq!( - fs::read_to_string(settings_path(&c.stage_root)).unwrap(), - original - ); - } - - #[test] - fn guard_is_armed_ignores_missing_inactive_expired_and_malformed_markers() { - let c = setup(); - let marker_path = skills_dir(&c.stage_root).join(GUARD_MARKER); - fs::create_dir_all(skills_dir(&c.stage_root)).unwrap(); - - assert!(!guard_is_armed(&c.stage_root)); - - fs::write( - &marker_path, - serde_json::to_string(&json!({ "active": false })).unwrap(), - ) - .unwrap(); - assert!(!guard_is_armed(&c.stage_root)); - - fs::write( - &marker_path, - serde_json::to_string(&json!({ - "active": true, - "expiresAt": iso_millis(now_ms() - 60_000), - })) - .unwrap(), - ) - .unwrap(); - assert!(!guard_is_armed(&c.stage_root)); - - fs::write(&marker_path, "not json").unwrap(); - assert!(!guard_is_armed(&c.stage_root)); - } - - #[test] - fn teardown_sweeps_a_stray_marker_even_without_a_manifest() { - let c = setup(); - fs::create_dir_all(skills_dir(&c.stage_root)).unwrap(); - fs::write(skills_dir(&c.stage_root).join(GUARD_MARKER), "{}").unwrap(); - assert!(teardown_guard(&c.stage_root)); - assert!(!skills_dir(&c.stage_root).join(GUARD_MARKER).exists()); - } - - #[test] - fn allows_returns_none() { - let payload = r#"{ "tool_name": "Read", "tool_input": { "file_path": "/etc/passwd" } }"#; - assert_eq!(guard_decision(payload, Some(marker())), None); - } - - #[test] - fn deny_returns_pretooluse_deny_json() { - let payload = r#"{ "tool_name": "Write", "tool_input": { "file_path": "/etc/passwd" } }"#; - let out = guard_decision(payload, Some(marker())).expect("should deny"); - let v: Value = serde_json::from_str(&out).unwrap(); - assert_eq!(v["hookSpecificOutput"]["hookEventName"], "PreToolUse"); - assert_eq!(v["hookSpecificOutput"]["permissionDecision"], "deny"); - assert!( - v["hookSpecificOutput"]["permissionDecisionReason"] - .as_str() - .unwrap() - .contains("outside") - ); - } - - #[test] - fn no_marker_allows_everything() { - let payload = r#"{ "tool_name": "Write", "tool_input": { "file_path": "/etc/passwd" } }"#; - assert_eq!(guard_decision(payload, None), None); - } - - #[test] - fn empty_or_malformed_payload_fails_open() { - assert_eq!(guard_decision("", Some(marker())), None); - assert_eq!(guard_decision("not json", Some(marker())), None); - } -} diff --git a/src/adapters/claude_code/mod.rs b/src/adapters/claude_code/mod.rs index 584b9c6..1b95861 100644 --- a/src/adapters/claude_code/mod.rs +++ b/src/adapters/claude_code/mod.rs @@ -1,12 +1,12 @@ //! Claude Code harness support — the default harness. //! -//! The declarative half of this harness lives in `harnesses/claude-code.toml`; -//! this module tree keeps only the code-backed capabilities the descriptor -//! references: `claude -p` stream-json transcript parsing ([`stream_json`] + -//! [`transcript`]), plugin-shadow detection ([`plugin_shadow`]), and the -//! write-guard hook ([`guard`]). +//! The declarative half of this harness lives in `harnesses/claude-code.toml` +//! (including the write-guard data rendered by the generic engine in +//! [`crate::adapters::guard`]); this module tree keeps only the code-backed +//! capabilities the descriptor references: `claude -p` stream-json transcript +//! parsing ([`stream_json`] + [`transcript`]) and plugin-shadow detection +//! ([`plugin_shadow`]). -pub(crate) mod guard; pub mod plugin_shadow; pub mod stream_json; pub mod transcript; diff --git a/src/adapters/codex/guard.rs b/src/adapters/codex/guard.rs deleted file mode 100644 index f4b6e82..0000000 --- a/src/adapters/codex/guard.rs +++ /dev/null @@ -1,260 +0,0 @@ -//! Codex write-guard hook: install + verdict shape. -//! -//! Arms the guard by merging a `PreToolUse` hook into the env's -//! `.codex/hooks.json`; dispatches must pass `--dangerously-bypass-hook-trust` -//! so the vetted project-local hook actually runs. The hook invokes the hidden -//! `guard-codex` subcommand (a stable on-disk contract), whose block verdict -//! uses Codex's native `{ "decision": "block", "reason": "..." }` shape. - -use std::fs; -use std::io; -use std::path::{Path, PathBuf}; -use std::time::Duration; - -use serde_json::{Value, json}; - -use crate::sandbox::decide::{GuardMarker, decide}; -use crate::sandbox::install::{ - GUARD_MANIFEST, GUARD_MARKER, write_json, write_manifest, write_marker, -}; -use crate::sandbox::{now_ms, parse_tool_call}; - -/// Tool names the Codex PreToolUse hook fires on. -pub(crate) const HOOK_MATCHER: &str = "^Bash$|^apply_patch$|^Edit$|^Write$"; - -/// Arm the write guard using Codex's project-local hook surface. Returns the -/// staged marker path. -pub(crate) fn install_guard( - stage_root: &Path, - guard_exe: &Path, - ttl: Option, -) -> io::Result { - let skills_dir = stage_root.join(".agents").join("skills"); - fs::create_dir_all(&skills_dir)?; - - let marker_path = skills_dir.join(GUARD_MARKER); - write_marker(&marker_path, stage_root, ttl)?; - - let hooks_path = stage_root.join(".codex").join("hooks.json"); - if let Some(parent) = hooks_path.parent() { - fs::create_dir_all(parent)?; - } - let hooks_existed = hooks_path.exists(); - let backup = if hooks_existed { - Some(fs::read_to_string(&hooks_path)?) - } else { - None - }; - - let mut hooks: Value = backup - .as_deref() - .and_then(|s| serde_json::from_str(s).ok()) - .unwrap_or_else(|| json!({})); - let hooks_obj = hooks - .as_object_mut() - .expect("hooks.json root is a JSON object") - .entry("hooks") - .or_insert_with(|| json!({})); - let pre = hooks_obj - .as_object_mut() - .expect("hooks is a JSON object") - .entry("PreToolUse") - .or_insert_with(|| json!([])); - let command = format!( - "\"{}\" guard-codex \"{}\"", - guard_exe.display(), - marker_path.display() - ); - pre.as_array_mut() - .expect("PreToolUse is an array") - .push(json!({ - "matcher": HOOK_MATCHER, - "hooks": [ - { - "type": "command", - "command": command, - "timeout": 30, - "statusMessage": "Checking eval write boundary", - } - ], - })); - write_json(&hooks_path, &hooks)?; - - write_manifest( - &skills_dir.join(GUARD_MANIFEST), - &hooks_path, - hooks_existed, - backup, - &marker_path, - )?; - - Ok(marker_path) -} - -/// The hook-config dir the Codex guard writes under `stage_root`; teardown -/// prunes it when the restored config leaves it empty. -pub(crate) fn hook_cleanup_dir(stage_root: &Path) -> PathBuf { - stage_root.join(".codex") -} - -/// Evaluate a PreToolUse hook `payload` (the JSON Codex sends on stdin) against -/// `marker`. Codex's hook contract blocks by returning `{ "decision": "block", -/// "reason": "..." }` on stdout — kept separate from Claude Code's -/// `hookSpecificOutput` shape so both harnesses use their native conventions. -pub(crate) fn guard_decision(payload: &str, marker: Option) -> Option { - let (tool_name, tool_input) = parse_tool_call(payload)?; - let decision = decide(&tool_name, &tool_input, marker.as_ref(), now_ms()); - if decision.allow { - return None; - } - Some( - serde_json::to_string(&json!({ - "decision": "block", - "reason": decision.reason, - })) - .expect("Codex block verdict serializes"), - ) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::sandbox::install::teardown_guard; - use tempfile::TempDir; - - struct Case { - _tmp: TempDir, - stage_root: PathBuf, - } - - fn setup() -> Case { - let tmp = TempDir::new().unwrap(); - let stage_root = tmp.path().join("stage"); - fs::create_dir_all(&stage_root).unwrap(); - Case { - _tmp: tmp, - stage_root, - } - } - - fn codex_hooks_path(stage_root: &Path) -> PathBuf { - stage_root.join(".codex").join("hooks.json") - } - - fn read_json(path: &Path) -> Value { - serde_json::from_str(&fs::read_to_string(path).unwrap()).unwrap() - } - - fn absolutize(p: &Path) -> PathBuf { - std::path::absolute(p).unwrap_or_else(|_| p.to_path_buf()) - } - - /// A live marker (active, no expiry → unexpired) scoped to one root. - fn marker() -> GuardMarker { - GuardMarker { - active: Some(true), - allowed_roots: Some(vec!["/work/.eval-magic".to_string()]), - expires_at: None, - } - } - - #[test] - fn codex_install_writes_project_hook_marker_and_manifest() { - let c = setup(); - let exe = Path::new("/g/eval-magic"); - install_guard(&c.stage_root, exe, None).unwrap(); - - let marker = read_json( - &c.stage_root - .join(".agents") - .join("skills") - .join(GUARD_MARKER), - ); - assert_eq!(marker["active"], json!(true)); - // The Codex guard shares the env-scoped roots: the staged `.agents/skills` - // dir lives inside `stage_root`, so the single env root already covers it. - let env = absolutize(&c.stage_root).display().to_string(); - assert!( - marker["allowedRoots"] - .as_array() - .unwrap() - .iter() - .any(|r| r.as_str().unwrap() == env) - ); - - let hooks = read_json(&codex_hooks_path(&c.stage_root)); - let hook = &hooks["hooks"]["PreToolUse"][0]; - assert!(hook["matcher"].as_str().unwrap().contains("apply_patch")); - assert!( - hook["hooks"][0]["command"] - .as_str() - .unwrap() - .contains("guard-codex") - ); - assert!( - c.stage_root - .join(".agents") - .join("skills") - .join(GUARD_MANIFEST) - .exists() - ); - } - - #[test] - fn codex_teardown_restores_pre_existing_hooks_json_verbatim() { - let c = setup(); - fs::create_dir_all(c.stage_root.join(".codex")).unwrap(); - let original = format!( - "{}\n", - serde_json::to_string_pretty(&json!({ - "hooks": { - "PostToolUse": [ - { - "matcher": "Bash", - "hooks": [{ "type": "command", "command": "echo ok" }] - } - ] - } - })) - .unwrap() - ); - fs::write(codex_hooks_path(&c.stage_root), &original).unwrap(); - - install_guard(&c.stage_root, Path::new("/g/eval-magic"), None).unwrap(); - assert!( - fs::read_to_string(codex_hooks_path(&c.stage_root)) - .unwrap() - .contains("guard-codex") - ); - - teardown_guard(&c.stage_root); - assert_eq!( - fs::read_to_string(codex_hooks_path(&c.stage_root)).unwrap(), - original - ); - } - - #[test] - fn codex_deny_returns_decision_block_json() { - let payload = r#"{ "hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#; - let out = guard_decision(payload, Some(marker())).expect("should block"); - let v: Value = serde_json::from_str(&out).unwrap(); - assert_eq!(v["decision"], "block"); - assert!(v["reason"].as_str().unwrap().contains("blocked Bash")); - } - - #[test] - fn codex_apply_patch_outside_allowed_roots_blocks() { - let payload = r#"{ "hook_event_name": "PreToolUse", "tool_name": "apply_patch", "tool_input": { "files": ["/etc/passwd"] } }"#; - let out = guard_decision(payload, Some(marker())).expect("should block"); - let v: Value = serde_json::from_str(&out).unwrap(); - assert_eq!(v["decision"], "block"); - assert!(v["reason"].as_str().unwrap().contains("apply_patch")); - } - - #[test] - fn codex_apply_patch_inside_allowed_roots_allows() { - let payload = r#"{ "hook_event_name": "PreToolUse", "tool_name": "apply_patch", "tool_input": { "files": ["/work/.eval-magic/out.md"] } }"#; - assert_eq!(guard_decision(payload, Some(marker())), None); - } -} diff --git a/src/adapters/codex/mod.rs b/src/adapters/codex/mod.rs index 712faf6..205c711 100644 --- a/src/adapters/codex/mod.rs +++ b/src/adapters/codex/mod.rs @@ -1,11 +1,11 @@ //! Codex harness support. //! -//! The declarative half of this harness lives in `harnesses/codex.toml`; this -//! module tree keeps only the code-backed capabilities the descriptor -//! references: `item.completed` event-stream parsing ([`transcript`]) and the -//! write-guard hook ([`guard`]). +//! The declarative half of this harness lives in `harnesses/codex.toml` +//! (including the write-guard data rendered by the generic engine in +//! [`crate::adapters::guard`]); this module tree keeps only the code-backed +//! capability the descriptor references: `item.completed` event-stream +//! parsing ([`transcript`]). -pub(crate) mod guard; pub mod transcript; #[cfg(test)] diff --git a/src/adapters/descriptor.rs b/src/adapters/descriptor.rs index 531ac30..11ff914 100644 --- a/src/adapters/descriptor.rs +++ b/src/adapters/descriptor.rs @@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize}; use crate::validation::{SchemaName, ValidationError, validate_against_schema}; -use super::capabilities::{GuardEngine, ShadowPreflight, SlugCapability, TranscriptParser}; +use super::capabilities::{ShadowPreflight, SlugCapability, TranscriptParser}; pub mod layers; mod validation; @@ -186,9 +186,29 @@ pub struct ModelSection { pub flag: String, } +/// The write-guard data block: everything the generic guard engine +/// ([`super::guard`]) needs to arm a hook and render a deny verdict. Embedded +/// built-in descriptors only — the guard fails open, so +/// [`layers::check_user_layer_restrictions`] bars user layers from declaring +/// it. #[derive(Debug, Clone, Deserialize, Serialize)] pub struct GuardSection { - pub engine: GuardEngine, + /// Hook-config file the entry is merged into, `/`-separated relative to + /// the staged env root (e.g. `.claude/settings.local.json`). + pub hooks_file: String, + /// Tool-name matcher the hook registers for; also the source of the + /// matcher⊆vocabulary invariant. + pub matcher: String, + /// Shell command the hook runs, with `{exe}` and `{marker}` placeholders. + pub command_template: String, + /// JSON template of the hook entry appended to the hook config's + /// `hooks.PreToolUse` array, with `{matcher}` and `{command}` placeholders + /// in its string values. Authored key order is serialized verbatim. + pub hook_entry: String, + /// JSON template of the deny verdict printed on stdout, with a `{reason}` + /// placeholder. Authored key order is the harness's on-disk contract. + pub verdict_template: String, + /// Banner printed after `--guard` arms the hook. pub armed_message: String, } @@ -430,7 +450,11 @@ write = ["Edit", "MultiEdit", "NotebookEdit", "Write"] shell = ["Bash"] [guard] -engine = "claude-hooks" +hooks_file = ".demo/hooks.json" +matcher = "Write|Edit|MultiEdit|NotebookEdit|Bash" +command_template = '"{exe}" guard-hook --harness demo "{marker}"' +hook_entry = '{"matcher":"{matcher}","hooks":[{"type":"command","command":"{command}"}]}' +verdict_template = '{"decision":"block","reason":"{reason}"}' armed_message = "guard armed" "#; @@ -453,7 +477,14 @@ armed_message = "guard armed" fn guarded_descriptor_loads() { let d = load(GUARDED).unwrap(); assert!(d.run.supports_guard); - assert!(d.guard.is_some()); + let guard = d.guard.expect("guard section loads"); + assert_eq!(guard.hooks_file, ".demo/hooks.json"); + assert_eq!(guard.matcher, "Write|Edit|MultiEdit|NotebookEdit|Bash"); + assert_eq!( + guard.command_template, + r#""{exe}" guard-hook --harness demo "{marker}""# + ); + assert_eq!(guard.armed_message, "guard armed"); } #[test] @@ -474,21 +505,7 @@ armed_message = "guard armed" #[test] fn rejects_guard_without_skills_dir() { - let toml = r#" -label = "demo" -config_dirs = [".demo"] - -[run] -supports_guard = true - -[tools] -write = ["Edit", "MultiEdit", "NotebookEdit", "Write"] -shell = ["Bash"] - -[guard] -engine = "claude-hooks" -armed_message = "guard armed" -"#; + let toml = &GUARDED.replace("skills_dir = \".demo/skills\"\n", ""); let err = err_of(toml); assert!(err.contains("guard"), "{err}"); assert!(err.contains("skills_dir"), "{err}"); @@ -522,9 +539,15 @@ armed_message = "guard armed" assert!(err.contains("harness-descriptor schema"), "{err}"); } + /// Migration canary: the pre-#137 named-engine shape must fail the schema + /// gate, so a stale `engine = "..."` line is caught with a schema message + /// instead of silently ignored. #[test] - fn rejects_unknown_guard_engine_name() { - let err = err_of(&GUARDED.replace("claude-hooks", "mystery-hooks")); + fn rejects_the_retired_guard_engine_field() { + let err = err_of(&GUARDED.replace( + "hooks_file = \".demo/hooks.json\"", + "engine = \"claude-hooks\"", + )); assert!(err.contains("harness-descriptor schema"), "{err}"); } diff --git a/src/adapters/descriptor/layers.rs b/src/adapters/descriptor/layers.rs index 47e3db6..d80fed7 100644 --- a/src/adapters/descriptor/layers.rs +++ b/src/adapters/descriptor/layers.rs @@ -217,10 +217,10 @@ mod tests { #[test] fn user_layer_may_not_declare_a_guard_table() { - let value: serde_json::Value = toml::from_str( - "label = \"demo\"\n\n[guard]\nengine = \"claude-hooks\"\narmed_message = \"x\"\n", - ) - .unwrap(); + // The restriction fires on the [guard] table's presence alone — field + // shape is irrelevant (the schema gate owns that). + let value: serde_json::Value = + toml::from_str("label = \"demo\"\n\n[guard]\narmed_message = \"x\"\n").unwrap(); let err = check_user_layer_restrictions(&value, "user.toml") .unwrap_err() .to_string(); diff --git a/src/adapters/descriptor/validation.rs b/src/adapters/descriptor/validation.rs index 09fd4cf..9c117e8 100644 --- a/src/adapters/descriptor/validation.rs +++ b/src/adapters/descriptor/validation.rs @@ -4,6 +4,8 @@ use regex::Regex; +use crate::adapters::guard; + use super::{DescriptorError, HarnessDescriptor, render_staged_slug, stage_name_error}; /// The placeholders a slug template must carry to keep cleanup prefix-scans @@ -114,9 +116,12 @@ pub(super) fn validate_descriptor( } } - // Every tool the guard engine hooks must be declared in the vocabulary, - // or the write-guard arbiter would silently wave it through. + // The guard block is rendered by the generic engine at arm/verdict time, + // and the guard fails open — so every data contract is proven here, before + // any run arms the hook. if let Some(guard) = &d.guard { + // Every tool the guard hooks must be declared in the vocabulary, or + // the write-guard arbiter would silently wave it through. let vocabulary: Vec<&str> = d .tools .write @@ -125,15 +130,82 @@ pub(super) fn validate_descriptor( .chain(&d.tools.shell) .map(String::as_str) .collect(); - for token in guard.engine.hook_matcher().split('|') { + for token in guard.matcher.split('|') { let token = token.trim_matches(['^', '$']); if !vocabulary.contains(&token) { return fail(format!( - "the guard engine hooks tool \"{token}\" but [tools] does not declare it \ + "the guard matcher hooks tool \"{token}\" but [tools] does not declare it \ in write/patch/shell — the write-guard arbiter would not recognize it" )); } } + + if guard.hooks_file.starts_with('/') + || guard + .hooks_file + .split('/') + .any(|seg| seg.is_empty() || seg == "." || seg == "..") + { + return fail(format!( + "guard.hooks_file must be a relative `/`-separated path without \".\" or \ + \"..\" segments (got \"{}\") — it resolves under the staged env root", + guard.hooks_file + )); + } + + for placeholder in ["{exe}", "{marker}"] { + if !guard.command_template.contains(placeholder) { + return fail(format!( + "guard.command_template must reference {placeholder} — the armed hook \ + invokes this binary with the marker path" + )); + } + } + + match serde_json::from_str::(&guard.hook_entry) { + Err(e) => { + return fail(format!( + "guard.hook_entry does not parse as JSON ({e}); it is the hook object \ + appended to the harness's hook config" + )); + } + Ok(entry) => { + if !entry.is_object() { + return fail( + "guard.hook_entry must be a JSON object — it is appended to the hook \ + config's hooks.PreToolUse array" + .into(), + ); + } + for placeholder in ["{matcher}", "{command}"] { + if !guard::any_string_value_contains(&entry, placeholder) { + return fail(format!( + "guard.hook_entry must reference the {placeholder} placeholder in \ + a string value — placeholders substitute into string values only, \ + so anywhere else would render an inert hook" + )); + } + } + } + } + + match serde_json::from_str::(&guard.verdict_template) { + Err(e) => { + return fail(format!( + "guard.verdict_template does not parse as JSON ({e}); it is printed \ + verbatim as the deny verdict" + )); + } + Ok(verdict) => { + if !guard::any_string_value_contains(&verdict, "{reason}") { + return fail( + "guard.verdict_template must reference the {reason} placeholder in a \ + string value — a deny verdict that hides the reason is undebuggable" + .into(), + ); + } + } + } } // A transcript parser without a write/shell vocabulary makes the @@ -303,8 +375,8 @@ skills_dir = ".demo/skills" config_dirs = [".demo"] "#; - /// A guard-wired descriptor whose tool vocabulary covers the claude-hooks - /// matcher; the base for guard/matcher mutation tests. + /// A guard-wired descriptor whose tool vocabulary covers its matcher; the + /// base for guard/matcher mutation tests. const GUARDED: &str = r#" label = "demo" skills_dir = ".demo/skills" @@ -318,7 +390,11 @@ write = ["Edit", "MultiEdit", "NotebookEdit", "Write"] shell = ["Bash"] [guard] -engine = "claude-hooks" +hooks_file = ".demo/hooks.json" +matcher = "Write|Edit|MultiEdit|NotebookEdit|Bash" +command_template = '"{exe}" guard-hook --harness demo "{marker}"' +hook_entry = '{"matcher":"{matcher}","hooks":[{"type":"command","command":"{command}"}]}' +verdict_template = '{"decision":"block","reason":"{reason}"}' armed_message = "guard armed" "#; @@ -372,13 +448,100 @@ armed_message = "guard armed" #[test] fn rejects_guard_matcher_tool_missing_from_vocabulary() { - // claude-hooks matches Write|Edit|MultiEdit|NotebookEdit|Bash; drop - // Bash from the shell vocabulary and the arbiter would wave it through. + // The matcher hooks Write|Edit|MultiEdit|NotebookEdit|Bash; drop Bash + // from the shell vocabulary and the arbiter would wave it through. let err = err_of(&GUARDED.replace("shell = [\"Bash\"]", "shell = [\"Shell\"]")); assert!(err.contains("Bash"), "{err}"); assert!(err.contains("[tools]"), "{err}"); } + #[test] + fn rejects_hook_entry_that_is_not_json() { + let err = err_of(&GUARDED.replace( + r#"hook_entry = '{"matcher":"{matcher}","hooks":[{"type":"command","command":"{command}"}]}'"#, + "hook_entry = 'not json'", + )); + assert!(err.contains("guard.hook_entry"), "{err}"); + assert!(err.contains("JSON"), "{err}"); + } + + #[test] + fn rejects_hook_entry_missing_a_placeholder() { + // {command} in a JSON *key* must not count: only string values are + // substituted, so a key-side placeholder would render an inert hook. + for (mutated, needle) in [ + ( + r#"hook_entry = '{"matcher":"{matcher}","hooks":[{"type":"command","{command}":"x"}]}'"#, + "{command}", + ), + ( + r#"hook_entry = '{"matcher":"Write","hooks":[{"type":"command","command":"{command}"}]}'"#, + "{matcher}", + ), + ] { + let err = err_of(&GUARDED.replace( + r#"hook_entry = '{"matcher":"{matcher}","hooks":[{"type":"command","command":"{command}"}]}'"#, + mutated, + )); + assert!(err.contains("guard.hook_entry"), "{err}"); + assert!(err.contains(needle), "expected {needle} in: {err}"); + } + } + + #[test] + fn rejects_verdict_template_that_is_not_json() { + let err = err_of(&GUARDED.replace( + r#"verdict_template = '{"decision":"block","reason":"{reason}"}'"#, + "verdict_template = 'block it'", + )); + assert!(err.contains("guard.verdict_template"), "{err}"); + assert!(err.contains("JSON"), "{err}"); + } + + #[test] + fn rejects_verdict_template_without_reason_placeholder() { + let err = err_of(&GUARDED.replace( + r#"verdict_template = '{"decision":"block","reason":"{reason}"}'"#, + r#"verdict_template = '{"decision":"block"}'"#, + )); + assert!(err.contains("guard.verdict_template"), "{err}"); + assert!(err.contains("{reason}"), "{err}"); + } + + #[test] + fn rejects_command_template_missing_exe_or_marker() { + for (mutated, needle) in [ + ( + r#"command_template = 'eval-magic guard-hook "{marker}"'"#, + "{exe}", + ), + ( + r#"command_template = '"{exe}" guard-hook --harness demo'"#, + "{marker}", + ), + ] { + let err = err_of(&GUARDED.replace( + r#"command_template = '"{exe}" guard-hook --harness demo "{marker}"'"#, + mutated, + )); + assert!(err.contains("guard.command_template"), "{err}"); + assert!(err.contains(needle), "expected {needle} in: {err}"); + } + } + + #[test] + fn rejects_hooks_file_that_escapes_the_env() { + for mutated in [ + "hooks_file = \"/etc/hooks.json\"", + "hooks_file = \"../hooks.json\"", + "hooks_file = \"./hooks.json\"", + ] { + let err = err_of(&GUARDED.replace("hooks_file = \".demo/hooks.json\"", mutated)); + assert!(err.contains("guard.hooks_file"), "{err}"); + assert!(err.contains("relative"), "{err}"); + } + } + #[test] fn rejects_transcript_without_write_and_shell_tools() { let err = err_of(&format!( diff --git a/src/adapters/descriptor_adapter.rs b/src/adapters/descriptor_adapter.rs index 4daf49a..60c480b 100644 --- a/src/adapters/descriptor_adapter.rs +++ b/src/adapters/descriptor_adapter.rs @@ -10,6 +10,7 @@ use std::time::Duration; use regex::Regex; use crate::core::{AvailableSkill, HarnessRunCapabilities, ToolInvocation}; +use crate::sandbox::GuardMarker; use super::TranscriptSummary; use super::cli_command::{ @@ -231,7 +232,12 @@ impl HarnessAdapter for DescriptorAdapter { ttl: Option, ) -> io::Result { match &self.descriptor.guard { - Some(guard) => guard.engine.install_guard(stage_root, guard_exe, ttl), + Some(guard) => { + let skills_dir = self + .skills_dir(stage_root) + .expect("descriptor validation pairs [guard] with skills_dir"); + super::guard::install_guard(guard, &skills_dir, stage_root, guard_exe, ttl) + } None => Err(io::Error::new( io::ErrorKind::Unsupported, format!("--guard is not supported for the {} harness", self.label()), @@ -246,11 +252,15 @@ impl HarnessAdapter for DescriptorAdapter { .map(|g| g.armed_message.clone()) } + fn guard_verdict(&self, payload: &str, marker: Option) -> Option { + let guard = self.descriptor.guard.as_ref()?; + super::guard::guard_verdict(guard, payload, marker) + } + fn guard_hook_cleanup_dir(&self, stage_root: &Path) -> Option { - self.descriptor - .guard - .as_ref() - .and_then(|g| g.engine.hook_cleanup_dir(stage_root)) + self.descriptor.guard.as_ref().and_then(|g| { + super::guard::hook_cleanup_dir(g, self.descriptor.skills_dir.as_deref(), stage_root) + }) } fn detect_shadowed_skills( diff --git a/src/adapters/guard.rs b/src/adapters/guard.rs new file mode 100644 index 0000000..708b6e9 --- /dev/null +++ b/src/adapters/guard.rs @@ -0,0 +1,638 @@ +//! The generic write-guard engine: one install + verdict implementation +//! rendered from a descriptor's `[guard]` data block. +//! +//! Arming merges one PreToolUse hook entry (the descriptor's `hook_entry` JSON +//! template) into the harness's hook-config file (`hooks_file`) and stages the +//! marker/manifest via [`crate::sandbox::install`]. The verdict side feeds a +//! hook payload through the shared arbiter ([`crate::sandbox::decide`]) and +//! serializes the descriptor's `verdict_template` on deny. Template key order +//! is authored in the descriptor and serialized verbatim (`serde_json` keeps +//! insertion order), so verdict bytes and hook-file shape are pinned by data, +//! not code. +//! +//! Guard blocks exist only in embedded built-in descriptors — the guard fails +//! open, so [`super::descriptor::layers::check_user_layer_restrictions`] bars +//! user layers from declaring one. + +use std::fs; +use std::io; +use std::path::{Path, PathBuf}; +use std::time::Duration; + +use serde_json::{Value, json}; + +use crate::sandbox::decide::{GuardMarker, decide}; +use crate::sandbox::install::{ + GUARD_MANIFEST, GUARD_MARKER, write_json, write_manifest, write_marker, +}; +use crate::sandbox::{now_ms, parse_tool_call}; + +use super::descriptor::{GuardSection, subst}; + +/// Arm the write guard: marker + manifest under `skills_dir` (absolute, +/// resolved by the caller), one hook entry merged into the descriptor's +/// `hooks_file`. Returns the staged marker path. +/// +/// Template parse failures panic (`expect`): descriptor validation proved the +/// templates at load time, and arming runs in the orchestrator where a loud +/// failure beats a silently unarmed guard. +pub(crate) fn install_guard( + guard: &GuardSection, + skills_dir: &Path, + stage_root: &Path, + guard_exe: &Path, + ttl: Option, +) -> io::Result { + fs::create_dir_all(skills_dir)?; + + let marker_path = skills_dir.join(GUARD_MARKER); + write_marker(&marker_path, stage_root, ttl)?; + + let hooks_path = resolve_rel(stage_root, &guard.hooks_file); + if let Some(parent) = hooks_path.parent() { + fs::create_dir_all(parent)?; + } + let hooks_existed = hooks_path.exists(); + let backup = if hooks_existed { + Some(fs::read_to_string(&hooks_path)?) + } else { + None + }; + + // Start from the existing hook config (or an empty object), preserving key + // order, then append the rendered hook entry. + let mut config: Value = backup + .as_deref() + .and_then(|s| serde_json::from_str(s).ok()) + .unwrap_or_else(|| json!({})); + let exe = guard_exe.display().to_string(); + let marker = marker_path.display().to_string(); + let command = subst( + &guard.command_template, + &[("exe", &exe), ("marker", &marker)], + ); + let mut entry: Value = serde_json::from_str(&guard.hook_entry) + .expect("guard.hook_entry parses as JSON (proven at descriptor load)"); + substitute_strings( + &mut entry, + &[("matcher", &guard.matcher), ("command", &command)], + ); + + let hooks = config + .as_object_mut() + .expect("hook config root is a JSON object") + .entry("hooks") + .or_insert_with(|| json!({})); + let pre = hooks + .as_object_mut() + .expect("hooks is a JSON object") + .entry("PreToolUse") + .or_insert_with(|| json!([])); + pre.as_array_mut() + .expect("PreToolUse is an array") + .push(entry); + write_json(&hooks_path, &config)?; + + write_manifest( + &skills_dir.join(GUARD_MANIFEST), + &hooks_path, + hooks_existed, + backup, + &marker_path, + )?; + + Ok(marker_path) +} + +/// Evaluate a PreToolUse hook `payload` against `marker`. Returns the deny +/// verdict to print on stdout (the descriptor's `verdict_template` with +/// `{reason}` filled), or `None` to allow (print nothing). Every error path — +/// empty/malformed payload, unrenderable template — fails open: the hook must +/// never brick a session. +pub(crate) fn guard_verdict( + guard: &GuardSection, + payload: &str, + marker: Option, +) -> Option { + let (tool_name, tool_input) = parse_tool_call(payload)?; + let decision = decide(&tool_name, &tool_input, marker.as_ref(), now_ms()); + if decision.allow { + return None; + } + let mut verdict: Value = serde_json::from_str(&guard.verdict_template).ok()?; + substitute_strings( + &mut verdict, + &[("reason", decision.reason.as_deref().unwrap_or(""))], + ); + serde_json::to_string(&verdict).ok() +} + +/// The hook-config dir the install created outside the skills dir, which +/// teardown prunes when restoring the original config leaves it empty. +/// Derived from the data: `hooks_file`'s parent dir, unless the hook file is +/// at the env root (nothing to prune) or the parent is the skills dir or an +/// ancestor of it (staging already owns that tree). +pub(crate) fn hook_cleanup_dir( + guard: &GuardSection, + skills_dir_rel: Option<&str>, + stage_root: &Path, +) -> Option { + let (parent, _) = guard.hooks_file.rsplit_once('/')?; + if let Some(skills) = skills_dir_rel { + // Component-wise ancestry: `.a` owns `.a/b/skills` but not `.ab/skills`. + let is_ancestor = skills == parent + || skills + .strip_prefix(parent) + .is_some_and(|rest| rest.starts_with('/')); + if is_ancestor { + return None; + } + } + Some(resolve_rel(stage_root, parent)) +} + +/// True when any *string value* in `value` contains `token`. Descriptor +/// validation uses this to prove template placeholders sit where +/// [`substitute_strings`] will actually reach them (keys are never +/// substituted). +pub(crate) fn any_string_value_contains(value: &Value, token: &str) -> bool { + match value { + Value::String(s) => s.contains(token), + Value::Array(items) => items.iter().any(|v| any_string_value_contains(v, token)), + Value::Object(map) => map.values().any(|v| any_string_value_contains(v, token)), + _ => false, + } +} + +/// Substitute `{token}` placeholders in every string value of `value`, +/// in place. Substituting after parsing (rather than into the template text) +/// lets values carry quotes without JSON-escaping concerns — the serializer +/// escapes them, exactly as `json!` used to. +fn substitute_strings(value: &mut Value, vars: &[(&str, &str)]) { + match value { + Value::String(s) if s.contains('{') => { + *s = subst(s, vars); + } + Value::Array(items) => { + for item in items { + substitute_strings(item, vars); + } + } + Value::Object(map) => { + for item in map.values_mut() { + substitute_strings(item, vars); + } + } + _ => {} + } +} + +/// Resolve a `/`-separated descriptor-relative path under `root` — the same +/// idiom as [`super::descriptor_adapter::DescriptorAdapter::skills_dir`]. +fn resolve_rel(root: &Path, rel: &str) -> PathBuf { + rel.split('/') + .fold(root.to_path_buf(), |path, segment| path.join(segment)) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::adapters::descriptor::{EMBEDDED_DESCRIPTORS, HarnessDescriptor, load_descriptor}; + use crate::sandbox::guard_is_armed; + use crate::sandbox::install::{iso_millis, teardown_guard}; + use chrono::DateTime; + use tempfile::TempDir; + + struct Case { + _tmp: TempDir, + stage_root: PathBuf, + } + + fn setup() -> Case { + let tmp = TempDir::new().unwrap(); + let stage_root = tmp.path().join("stage"); + fs::create_dir_all(&stage_root).unwrap(); + Case { + _tmp: tmp, + stage_root, + } + } + + /// Load one embedded descriptor by label — engine tests run against the + /// real shipped guard data, not fixtures. + fn descriptor(label: &str) -> HarnessDescriptor { + let (source, toml_src) = EMBEDDED_DESCRIPTORS + .iter() + .find(|(path, _)| path.ends_with(&format!("{label}.toml"))) + .unwrap_or_else(|| panic!("no embedded descriptor for {label}")); + load_descriptor(toml_src, source).unwrap() + } + + /// Arm the guard for `label` under `stage_root` via the engine, returning + /// the marker path. + fn install(label: &str, stage_root: &Path) -> PathBuf { + let d = descriptor(label); + let skills = resolve_rel(stage_root, d.skills_dir.as_deref().unwrap()); + install_guard( + d.guard.as_ref().unwrap(), + &skills, + stage_root, + Path::new("/g/eval-magic"), + None, + ) + .unwrap() + } + + fn verdict(label: &str, payload: &str, marker: Option) -> Option { + let d = descriptor(label); + guard_verdict(d.guard.as_ref().unwrap(), payload, marker) + } + + fn claude_skills_dir(stage_root: &Path) -> PathBuf { + stage_root.join(".claude").join("skills") + } + + fn settings_path(stage_root: &Path) -> PathBuf { + stage_root.join(".claude").join("settings.local.json") + } + + fn codex_hooks_path(stage_root: &Path) -> PathBuf { + stage_root.join(".codex").join("hooks.json") + } + + fn read_json(path: &Path) -> Value { + serde_json::from_str(&fs::read_to_string(path).unwrap()).unwrap() + } + + fn absolutize(p: &Path) -> PathBuf { + std::path::absolute(p).unwrap_or_else(|_| p.to_path_buf()) + } + + /// A live marker (active, no expiry → unexpired) scoped to one root. + fn marker() -> GuardMarker { + GuardMarker { + active: Some(true), + allowed_roots: Some(vec!["/work/.eval-magic".to_string()]), + expires_at: None, + } + } + + #[test] + fn install_writes_an_active_marker_hook_and_manifest() { + let c = setup(); + install("claude-code", &c.stage_root); + + let marker = read_json(&claude_skills_dir(&c.stage_root).join(GUARD_MARKER)); + assert_eq!(marker["active"], json!(true)); + let expires = marker["expiresAt"].as_str().unwrap(); + let exp_ms = DateTime::parse_from_rfc3339(expires) + .unwrap() + .timestamp_millis(); + assert!(exp_ms > now_ms()); + let env = absolutize(&c.stage_root).display().to_string(); + assert!( + marker["allowedRoots"] + .as_array() + .unwrap() + .iter() + .any(|r| r.as_str().unwrap() == env) + ); + + let settings = read_json(&settings_path(&c.stage_root)); + let hook = &settings["hooks"]["PreToolUse"][0]; + assert!(hook["matcher"].as_str().unwrap().contains("Write")); + assert!( + hook["hooks"][0]["command"] + .as_str() + .unwrap() + .contains("guard") + ); + + assert!( + claude_skills_dir(&c.stage_root) + .join(GUARD_MANIFEST) + .exists() + ); + } + + #[test] + fn marker_scopes_allowed_roots_to_the_env_and_temp_only() { + let c = setup(); + install("claude-code", &c.stage_root); + + let marker = read_json(&claude_skills_dir(&c.stage_root).join(GUARD_MARKER)); + let roots: Vec = marker["allowedRoots"] + .as_array() + .unwrap() + .iter() + .map(|r| r.as_str().unwrap().to_string()) + .collect(); + + // The guard boundary is the isolated env (stage_root) plus temp — nothing + // above it. The parent workspace tree must NOT be an allowed root, or the + // agent could write into sibling iterations / the meta dir above `env/`. + let env = absolutize(&c.stage_root).display().to_string(); + let temp = absolutize(&std::env::temp_dir()).display().to_string(); + assert_eq!(roots, vec![env, temp]); + assert!( + !roots.iter().any(|r| r.ends_with(".eval-magic")), + "workspace_root must not be an allowed root: {roots:?}" + ); + } + + #[test] + fn hook_command_invokes_the_binary_guard_subcommand() { + let c = setup(); + let marker = install("claude-code", &c.stage_root); + let settings = read_json(&settings_path(&c.stage_root)); + let command = settings["hooks"]["PreToolUse"][0]["hooks"][0]["command"] + .as_str() + .unwrap() + .to_string(); + assert_eq!( + command, + format!("\"/g/eval-magic\" guard \"{}\"", marker.display()) + ); + } + + #[test] + fn teardown_deletes_settings_it_created() { + let c = setup(); + install("claude-code", &c.stage_root); + assert!(settings_path(&c.stage_root).exists()); + + assert!(teardown_guard(&c.stage_root)); + assert!(!settings_path(&c.stage_root).exists()); + assert!(!claude_skills_dir(&c.stage_root).join(GUARD_MARKER).exists()); + assert!( + !claude_skills_dir(&c.stage_root) + .join(GUARD_MANIFEST) + .exists() + ); + } + + #[test] + fn teardown_restores_a_pre_existing_settings_verbatim() { + let c = setup(); + fs::create_dir_all(c.stage_root.join(".claude")).unwrap(); + let original = format!( + "{}\n", + serde_json::to_string_pretty(&json!({ + "permissions": { "allow": ["Bash(ls)"] } + })) + .unwrap() + ); + fs::write(settings_path(&c.stage_root), &original).unwrap(); + + install("claude-code", &c.stage_root); + // hook present while armed + assert!( + fs::read_to_string(settings_path(&c.stage_root)) + .unwrap() + .contains("PreToolUse") + ); + + teardown_guard(&c.stage_root); + assert_eq!( + fs::read_to_string(settings_path(&c.stage_root)).unwrap(), + original + ); + } + + #[test] + fn guard_is_armed_ignores_missing_inactive_expired_and_malformed_markers() { + let c = setup(); + let marker_path = claude_skills_dir(&c.stage_root).join(GUARD_MARKER); + fs::create_dir_all(claude_skills_dir(&c.stage_root)).unwrap(); + + assert!(!guard_is_armed(&c.stage_root)); + + fs::write( + &marker_path, + serde_json::to_string(&json!({ "active": false })).unwrap(), + ) + .unwrap(); + assert!(!guard_is_armed(&c.stage_root)); + + fs::write( + &marker_path, + serde_json::to_string(&json!({ + "active": true, + "expiresAt": iso_millis(now_ms() - 60_000), + })) + .unwrap(), + ) + .unwrap(); + assert!(!guard_is_armed(&c.stage_root)); + + fs::write(&marker_path, "not json").unwrap(); + assert!(!guard_is_armed(&c.stage_root)); + } + + #[test] + fn teardown_sweeps_a_stray_marker_even_without_a_manifest() { + let c = setup(); + fs::create_dir_all(claude_skills_dir(&c.stage_root)).unwrap(); + fs::write(claude_skills_dir(&c.stage_root).join(GUARD_MARKER), "{}").unwrap(); + assert!(teardown_guard(&c.stage_root)); + assert!(!claude_skills_dir(&c.stage_root).join(GUARD_MARKER).exists()); + } + + #[test] + fn allows_returns_none() { + let payload = r#"{ "tool_name": "Read", "tool_input": { "file_path": "/etc/passwd" } }"#; + assert_eq!(verdict("claude-code", payload, Some(marker())), None); + } + + #[test] + fn deny_returns_pretooluse_deny_json() { + let payload = r#"{ "tool_name": "Write", "tool_input": { "file_path": "/etc/passwd" } }"#; + let out = verdict("claude-code", payload, Some(marker())).expect("should deny"); + let v: Value = serde_json::from_str(&out).unwrap(); + assert_eq!(v["hookSpecificOutput"]["hookEventName"], "PreToolUse"); + assert_eq!(v["hookSpecificOutput"]["permissionDecision"], "deny"); + assert!( + v["hookSpecificOutput"]["permissionDecisionReason"] + .as_str() + .unwrap() + .contains("outside") + ); + } + + /// Byte-pin of both deny verdicts: full-string equality against the exact + /// serialization armed hooks have always read. Key order comes from the + /// descriptor templates and must never drift. + #[test] + fn deny_verdict_bytes_match_the_on_disk_contract() { + let payload = r#"{ "tool_name": "Write", "tool_input": { "file_path": "/etc/passwd" } }"#; + assert_eq!( + verdict("claude-code", payload, Some(marker())).expect("should deny"), + "{\"hookSpecificOutput\":{\"hookEventName\":\"PreToolUse\",\ + \"permissionDecision\":\"deny\",\"permissionDecisionReason\":\ + \"eval guard: Write to /etc/passwd is outside the eval sandbox \ + (allowed: /work/.eval-magic)\"}}" + ); + + let payload = + r#"{ "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#; + assert_eq!( + verdict("codex", payload, Some(marker())).expect("should block"), + "{\"decision\":\"block\",\"reason\":\"eval guard: blocked Bash \ + (package install/add) — runs outside the eval sandbox\"}" + ); + } + + #[test] + fn no_marker_allows_everything() { + let payload = r#"{ "tool_name": "Write", "tool_input": { "file_path": "/etc/passwd" } }"#; + assert_eq!(verdict("claude-code", payload, None), None); + } + + #[test] + fn empty_or_malformed_payload_fails_open() { + assert_eq!(verdict("claude-code", "", Some(marker())), None); + assert_eq!(verdict("claude-code", "not json", Some(marker())), None); + } + + #[test] + fn codex_install_writes_project_hook_marker_and_manifest() { + let c = setup(); + install("codex", &c.stage_root); + + let marker = read_json( + &c.stage_root + .join(".agents") + .join("skills") + .join(GUARD_MARKER), + ); + assert_eq!(marker["active"], json!(true)); + // The Codex guard shares the env-scoped roots: the staged `.agents/skills` + // dir lives inside `stage_root`, so the single env root already covers it. + let env = absolutize(&c.stage_root).display().to_string(); + assert!( + marker["allowedRoots"] + .as_array() + .unwrap() + .iter() + .any(|r| r.as_str().unwrap() == env) + ); + + let hooks = read_json(&codex_hooks_path(&c.stage_root)); + let hook = &hooks["hooks"]["PreToolUse"][0]; + assert!(hook["matcher"].as_str().unwrap().contains("apply_patch")); + assert!( + hook["hooks"][0]["command"] + .as_str() + .unwrap() + .contains("guard-codex") + ); + assert!( + c.stage_root + .join(".agents") + .join("skills") + .join(GUARD_MANIFEST) + .exists() + ); + } + + #[test] + fn codex_teardown_restores_pre_existing_hooks_json_verbatim() { + let c = setup(); + fs::create_dir_all(c.stage_root.join(".codex")).unwrap(); + let original = format!( + "{}\n", + serde_json::to_string_pretty(&json!({ + "hooks": { + "PostToolUse": [ + { + "matcher": "Bash", + "hooks": [{ "type": "command", "command": "echo ok" }] + } + ] + } + })) + .unwrap() + ); + fs::write(codex_hooks_path(&c.stage_root), &original).unwrap(); + + install("codex", &c.stage_root); + assert!( + fs::read_to_string(codex_hooks_path(&c.stage_root)) + .unwrap() + .contains("guard-codex") + ); + + teardown_guard(&c.stage_root); + assert_eq!( + fs::read_to_string(codex_hooks_path(&c.stage_root)).unwrap(), + original + ); + } + + #[test] + fn codex_deny_returns_decision_block_json() { + let payload = r#"{ "hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#; + let out = verdict("codex", payload, Some(marker())).expect("should block"); + let v: Value = serde_json::from_str(&out).unwrap(); + assert_eq!(v["decision"], "block"); + assert!(v["reason"].as_str().unwrap().contains("blocked Bash")); + } + + #[test] + fn codex_apply_patch_outside_allowed_roots_blocks() { + let payload = r#"{ "hook_event_name": "PreToolUse", "tool_name": "apply_patch", "tool_input": { "files": ["/etc/passwd"] } }"#; + let out = verdict("codex", payload, Some(marker())).expect("should block"); + let v: Value = serde_json::from_str(&out).unwrap(); + assert_eq!(v["decision"], "block"); + assert!(v["reason"].as_str().unwrap().contains("apply_patch")); + } + + #[test] + fn codex_apply_patch_inside_allowed_roots_allows() { + let payload = r#"{ "hook_event_name": "PreToolUse", "tool_name": "apply_patch", "tool_input": { "files": ["/work/.eval-magic/out.md"] } }"#; + assert_eq!(verdict("codex", payload, Some(marker())), None); + } + + /// The cleanup dir is derived from the guard data: the hooks file's parent, + /// unless the hook file sits at the env root or inside the skills dir's + /// ancestry (staging already owns that tree). + #[test] + fn hook_cleanup_dir_derivation_table() { + let root = Path::new("/env"); + let with = |hooks_file: &str| { + let mut guard = descriptor("codex").guard.unwrap(); + guard.hooks_file = hooks_file.to_string(); + guard + }; + + // Claude: `.claude` is the skills dir's parent — staging owns it. + assert_eq!( + hook_cleanup_dir( + &with(".claude/settings.local.json"), + Some(".claude/skills"), + root + ), + None + ); + // Codex: `.codex` is created for the hook alone — prune it. + assert_eq!( + hook_cleanup_dir(&with(".codex/hooks.json"), Some(".agents/skills"), root), + Some(PathBuf::from("/env/.codex")) + ); + // Hook file at the env root: never prune the env itself. + assert_eq!( + hook_cleanup_dir(&with("hooks.json"), Some(".agents/skills"), root), + None + ); + // Parent is an ancestor of the skills dir (not merely a string prefix: + // `.a` vs `.ab/skills` stays prunable). + assert_eq!( + hook_cleanup_dir(&with(".a/hooks.json"), Some(".a/b/skills"), root), + None + ); + assert_eq!( + hook_cleanup_dir(&with(".a/hooks.json"), Some(".ab/skills"), root), + Some(PathBuf::from("/env/.a")) + ); + } +} diff --git a/src/adapters/harness.rs b/src/adapters/harness.rs index 6d755c7..4f7e7c0 100644 --- a/src/adapters/harness.rs +++ b/src/adapters/harness.rs @@ -27,6 +27,7 @@ use std::path::{Path, PathBuf}; use std::time::Duration; use crate::core::{AvailableSkill, HarnessRunCapabilities, ToolInvocation}; +use crate::sandbox::GuardMarker; use super::TranscriptSummary; use super::skill_shadow::PluginShadowReport; @@ -263,6 +264,15 @@ pub trait HarnessAdapter { None } + /// **Enhancement: write guard.** Evaluate a PreToolUse hook `payload` + /// against `marker`, returning the serialized deny verdict to print on + /// stdout, or `None` to allow. The default fails open — a harness with no + /// guard never denies — matching the hook entry points' contract that a + /// guard invocation can never brick a session. + fn guard_verdict(&self, _payload: &str, _marker: Option) -> Option { + None + } + /// **Enhancement: write guard.** A hook-config dir the guard install /// created outside [`skills_dir`](Self::skills_dir) (e.g. Codex's /// `.codex/`), which teardown prunes when restoring the original config diff --git a/src/adapters/mod.rs b/src/adapters/mod.rs index e94fbdc..aeb45c0 100644 --- a/src/adapters/mod.rs +++ b/src/adapters/mod.rs @@ -7,7 +7,8 @@ //! generic [`descriptor_adapter`]); the code-backed features a descriptor //! references by name live in [`capabilities`], backed by the per-harness //! module trees ([`claude_code`], [`codex`], [`opencode`]): transcript -//! parsers, write-guard hooks, plugin-shadow detection, slug sanitization. +//! parsers, plugin-shadow detection, slug sanitization. The write guard is +//! pure descriptor data rendered by the generic engine in [`guard`]. //! The [`registry`] loads the descriptors into label-keyed entries and owns //! harness-identifier resolution; generic code resolves an adapter with //! [`adapter_for`] and calls the trait. @@ -18,6 +19,7 @@ mod cli_command; pub mod codex; pub mod descriptor; pub mod descriptor_adapter; +pub(crate) mod guard; pub mod harness; pub mod opencode; pub mod registry; diff --git a/src/adapters/registry.rs b/src/adapters/registry.rs index 871f3f2..4441f8a 100644 --- a/src/adapters/registry.rs +++ b/src/adapters/registry.rs @@ -423,6 +423,20 @@ mod tests { } } + /// A schema-valid user descriptor declaring a [guard] block — must be + /// rejected by the user-layer restriction, not the schema gate. + const USER_GUARD_TOML: &str = r#" +label = "armed" + +[guard] +hooks_file = ".armed/hooks.json" +matcher = "Write" +command_template = '"{exe}" guard-hook --harness armed "{marker}"' +hook_entry = '{"matcher":"{matcher}","hooks":[{"type":"command","command":"{command}"}]}' +verdict_template = '{"decision":"block","reason":"{reason}"}' +armed_message = "x" +"#; + #[test] fn duplicate_embedded_label_errors() { let mut sources = embedded_sources(); @@ -509,7 +523,7 @@ mod tests { sources.push(src( Layer::ProjectLocal, ".eval-magic/harnesses/armed.toml", - "label = \"armed\"\n\n[guard]\nengine = \"claude-hooks\"\narmed_message = \"x\"\n", + USER_GUARD_TOML, )); let built = build_registry(sources).unwrap(); assert_eq!( @@ -603,11 +617,7 @@ mod tests { #[test] fn harness_file_guard_rejection_is_fatal() { let mut sources = embedded_sources(); - sources.push(src( - Layer::HarnessFile, - "one-off.toml", - "label = \"armed\"\n\n[guard]\nengine = \"claude-hooks\"\narmed_message = \"x\"\n", - )); + sources.push(src(Layer::HarnessFile, "one-off.toml", USER_GUARD_TOML)); let err = build_registry(sources).unwrap_err().to_string(); assert!(err.contains("may not declare [guard]"), "{err}"); } diff --git a/src/cli/commands/guard.rs b/src/cli/commands/guard.rs index 693a541..9ef73bf 100644 --- a/src/cli/commands/guard.rs +++ b/src/cli/commands/guard.rs @@ -1,45 +1,53 @@ -//! Write-guard command handlers: the hidden per-harness PreToolUse hook entry -//! points and the user-facing `teardown-guard`. +//! Write-guard command handlers: the hidden PreToolUse hook entry points and +//! the user-facing `teardown-guard`. //! //! The `guard` / `guard-codex` subcommand names are a **stable on-disk //! contract**: armed hooks staged into harness config reference them by name, -//! so renaming either would break every already-armed guard. +//! so renaming either would break every already-armed guard. Both are aliases +//! of the generic `guard-hook` entry point, which future guard-capable +//! harnesses use directly (`eval-magic guard-hook --harness `). use std::io; use std::path::PathBuf; -use crate::adapters::{adapter_for, claude_code, codex}; +use crate::adapters::{HarnessAdapter, adapter_for}; use crate::core::Harness; use crate::sandbox; -/// The hidden Claude Code PreToolUse hook entry point. Reads the hook payload -/// from stdin and the marker path from argv, then prints a deny verdict for -/// out-of-bounds calls. It **fails open** — every error path allows the call -/// and exits 0, so the guard can never brick a session. +/// The hidden Claude Code PreToolUse hook entry point — a frozen alias for +/// `guard-hook --harness claude-code`. pub(crate) fn run_guard(marker: Option) -> anyhow::Result<()> { - let marker_path = marker - .map(PathBuf::from) - .unwrap_or_else(|| default_marker_path("claude-code")); - let payload = io::read_to_string(io::stdin()).unwrap_or_default(); - if let Some(verdict) = - claude_code::guard::guard_decision(&payload, sandbox::read_marker(&marker_path)) - { - print!("{verdict}"); - } - Ok(()) + run_guard_hook("claude-code", marker) } -/// The hidden Codex PreToolUse hook entry point. Same policy as `guard`, but -/// Codex blocks by reading `{ "decision": "block", "reason": "..." }` from the -/// hook's stdout. +/// The hidden Codex PreToolUse hook entry point — a frozen alias for +/// `guard-hook --harness codex`. pub(crate) fn run_guard_codex(marker: Option) -> anyhow::Result<()> { - let marker_path = marker - .map(PathBuf::from) - .unwrap_or_else(|| default_marker_path("codex")); + run_guard_hook("codex", marker) +} + +/// The generic PreToolUse hook entry point. Reads the hook payload from stdin +/// and the marker path from argv, resolves the harness's guard from the +/// embedded descriptors (hook invocations skip layered discovery — see +/// `cli::run`'s `is_guard_hook` gate), and prints a deny verdict for +/// out-of-bounds calls. It **fails open** — an unknown harness, a guard-less +/// descriptor, or any error path allows the call and exits 0, so the guard can +/// never brick a session. +pub(crate) fn run_guard_hook(harness_name: &str, marker: Option) -> anyhow::Result<()> { + // Drain stdin before any early return so the harness writing the payload + // never sees a broken pipe, whatever the verdict. let payload = io::read_to_string(io::stdin()).unwrap_or_default(); - if let Some(verdict) = - codex::guard::guard_decision(&payload, sandbox::read_marker(&marker_path)) - { + let Ok(harness) = Harness::resolve(harness_name) else { + return Ok(()); + }; + let adapter = adapter_for(harness); + let Some(marker_path) = marker + .map(PathBuf::from) + .or_else(|| default_marker_path(adapter)) + else { + return Ok(()); + }; + if let Some(verdict) = adapter.guard_verdict(&payload, sandbox::read_marker(&marker_path)) { print!("{verdict}"); } Ok(()) @@ -63,12 +71,11 @@ pub(crate) fn run_teardown_guard() -> anyhow::Result<()> { /// The marker path a guard hook reads when argv carries none: the harness's /// skills dir under the cwd, e.g. `/.claude/skills/.slow-powers-eval-guard.json`. -/// The names are bundled-harness literals because each hook entry point IS its -/// harness's on-disk contract (see the module docs). -fn default_marker_path(harness_name: &str) -> PathBuf { - let harness = Harness::resolve(harness_name).expect("bundled harness"); - adapter_for(harness) - .skills_dir(&std::env::current_dir().unwrap_or_default()) - .expect("bundled guard-capable harnesses declare skills_dir") - .join(sandbox::GUARD_MARKER) +/// `None` (fail open) for a harness that declares no skills dir. +fn default_marker_path(adapter: &dyn HarnessAdapter) -> Option { + Some( + adapter + .skills_dir(&std::env::current_dir().unwrap_or_default())? + .join(sandbox::GUARD_MARKER), + ) } diff --git a/src/sandbox/install.rs b/src/sandbox/install.rs index ff9d772..530842f 100644 --- a/src/sandbox/install.rs +++ b/src/sandbox/install.rs @@ -314,12 +314,14 @@ mod tests { fn guard_is_armed_detects_claude_or_codex_marker() { let c = setup(); let exe = Path::new("/g/eval-magic"); - crate::adapters::claude_code::guard::install_guard(&c.stage_root, exe, None).unwrap(); + let claude = crate::adapters::adapter_for(Harness::resolve("claude-code").unwrap()); + claude.install_guard(&c.stage_root, exe, None).unwrap(); assert!(guard_is_armed(&c.stage_root)); teardown_guard(&c.stage_root); assert!(!guard_is_armed(&c.stage_root)); - crate::adapters::codex::guard::install_guard(&c.stage_root, exe, None).unwrap(); + let codex = crate::adapters::adapter_for(Harness::resolve("codex").unwrap()); + codex.install_guard(&c.stage_root, exe, None).unwrap(); assert!(guard_is_armed(&c.stage_root)); } } diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index f5266e6..000e053 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -1,11 +1,13 @@ //! Execution sandbox: shared write-guard machinery and write-boundary policy. //! //! The hook entry points are hidden subcommands on this binary (see `cli`), so -//! the installed PreToolUse hook invokes `eval-magic guard ` or -//! `eval-magic guard-codex ` — no separate hook script to ship or -//! locate. Each harness's installer and verdict shape live in its adapter -//! module (`crate::adapters::::guard`); this module holds the shared -//! marker/manifest/teardown machinery and the boundary policy. +//! the installed PreToolUse hook invokes `eval-magic guard `, +//! `eval-magic guard-codex `, or the generic +//! `eval-magic guard-hook --harness ` — no separate hook script +//! to ship or locate. Each harness's hook path, matcher, and verdict shape are +//! descriptor data rendered by the generic engine (`crate::adapters::guard`); +//! this module holds the shared marker/manifest/teardown machinery and the +//! boundary policy. pub mod decide; pub mod guard; diff --git a/tests/cli/harness.rs b/tests/cli/harness.rs index 1626371..cc9d5d2 100644 --- a/tests/cli/harness.rs +++ b/tests/cli/harness.rs @@ -231,7 +231,17 @@ fn harness_lint_rejects_a_user_guard_block() { let file = tmp.path().join("armed.toml"); fs::write( &file, - "label = \"armed\"\n\n[guard]\nengine = \"claude-hooks\"\narmed_message = \"x\"\n", + r#" +label = "armed" + +[guard] +hooks_file = ".armed/hooks.json" +matcher = "Write" +command_template = '"{exe}" guard-hook --harness armed "{marker}"' +hook_entry = '{"matcher":"{matcher}","hooks":[{"type":"command","command":"{command}"}]}' +verdict_template = '{"decision":"block","reason":"{reason}"}' +armed_message = "x" +"#, ) .unwrap(); From ac35f272a140fa0171cd1abd9d0d355e29545bf9 Mon Sep 17 00:00:00 2001 From: Max Haarhaus Date: Wed, 15 Jul 2026 15:53:50 -0400 Subject: [PATCH 3/5] feat(cli): generic guard-hook subcommand for descriptor-driven guards Hidden `guard-hook --harness ` entry point resolving the verdict shape from the named harness's embedded descriptor; `guard` / `guard-codex` remain frozen aliases. The new subcommand joins the is_guard_hook gate so hook invocations stay off layered descriptor discovery, and it fails open on an unknown harness name. Part of #137. Co-Authored-By: Claude Fable 5 --- src/cli/args.rs | 14 +++++++++ src/cli/commands/mod.rs | 2 +- src/cli/mod.rs | 3 +- tests/cli/guard.rs | 69 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/cli/args.rs b/src/cli/args.rs index b3c4843..9a98732 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -525,4 +525,18 @@ pub(crate) enum Commands { /// `/.agents/skills/.slow-powers-eval-guard.json`. marker: Option, }, + /// Internal generic PreToolUse hook entry point. Invoked by the installed + /// write-guard hook as `eval-magic guard-hook --harness `, + /// not by users; hidden from help. `guard` / `guard-codex` are frozen + /// aliases of this for the claude-code and codex harnesses. + #[command(hide = true, name = "guard-hook")] + GuardHook { + /// Harness whose embedded descriptor supplies the verdict shape; an + /// unknown name fails open (allows the call). + #[arg(long)] + harness: String, + /// Path to the guard marker file. Defaults to the harness's + /// `/.slow-powers-eval-guard.json` under the cwd. + marker: Option, + }, } diff --git a/src/cli/commands/mod.rs b/src/cli/commands/mod.rs index c9b41af..2000432 100644 --- a/src/cli/commands/mod.rs +++ b/src/cli/commands/mod.rs @@ -12,7 +12,7 @@ mod run; mod validate; mod workspace; -pub(crate) use guard::{run_guard, run_guard_codex, run_teardown_guard}; +pub(crate) use guard::{run_guard, run_guard_codex, run_guard_hook, run_teardown_guard}; pub(crate) use harness::run_harness; pub(crate) use init::run_init; pub(crate) use pipeline::{ diff --git a/src/cli/mod.rs b/src/cli/mod.rs index a220a64..9e334f1 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -32,7 +32,7 @@ pub fn run() -> anyhow::Result<()> { // lazy registry fallback serves them embedded-only). let is_guard_hook = matches!( cli.command, - Some(Commands::Guard { .. } | Commands::GuardCodex { .. }) + Some(Commands::Guard { .. } | Commands::GuardCodex { .. } | Commands::GuardHook { .. }) ); if !is_guard_hook { crate::adapters::registry::init_registry(cli.harness_file.as_deref().map(Path::new))?; @@ -76,6 +76,7 @@ fn dispatch(command: Option) -> anyhow::Result<()> { Commands::TeardownGuard(_) => run_teardown_guard(), Commands::Guard { marker } => run_guard(marker), Commands::GuardCodex { marker } => run_guard_codex(marker), + Commands::GuardHook { harness, marker } => run_guard_hook(&harness, marker), Commands::RecordRuns(args) => run_record_runs(args), Commands::FillTranscripts(args) => run_fill_transcripts(args), Commands::DetectStrayWrites(args) => run_detect_stray_writes(args), diff --git a/tests/cli/guard.rs b/tests/cli/guard.rs index a921fb6..3946d7e 100644 --- a/tests/cli/guard.rs +++ b/tests/cli/guard.rs @@ -17,6 +17,10 @@ fn guard_subcommand_is_hidden_but_callable() { .stdout(contains("PreToolUse hook entry point").not()); skill_eval().arg("guard").arg("--help").assert().success(); + skill_eval() + .args(["guard-hook", "--help"]) + .assert() + .success(); } /// Write an armed guard marker scoping writes to ``, and return its path. @@ -171,6 +175,71 @@ fn guard_fails_open_without_marker() { .stdout(""); } +/// The generic `guard-hook --harness ` entry point resolves the verdict +/// shape from the named harness's embedded descriptor — Claude's deny shape +/// for claude-code, Codex's block shape for codex. Future guard-capable +/// built-ins use this instead of minting another alias. +#[test] +fn guard_hook_resolves_the_harness_verdict_shape() { + let tmp = TempDir::new().unwrap(); + let marker = write_armed_marker(tmp.path(), &tmp.path().join(".eval-magic")); + + skill_eval() + .args(["guard-hook", "--harness", "claude-code"]) + .arg(&marker) + .write_stdin(r#"{ "tool_name": "Write", "tool_input": { "file_path": "/etc/passwd" } }"#) + .assert() + .success() + .stdout(contains(r#""permissionDecision":"deny""#)); + + skill_eval() + .args(["guard-hook", "--harness", "codex"]) + .arg(&marker) + .write_stdin( + r#"{ "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#, + ) + .assert() + .success() + .stdout(contains(r#""decision":"block""#)); +} + +/// `guard-hook` fails open on an unknown harness: empty stdout, exit 0 — the +/// hook must never brick a session, whatever config invoked it. +#[test] +fn guard_hook_fails_open_for_an_unknown_harness() { + let tmp = TempDir::new().unwrap(); + let marker = write_armed_marker(tmp.path(), &tmp.path().join(".eval-magic")); + + skill_eval() + .args(["guard-hook", "--harness", "mystery"]) + .arg(&marker) + .write_stdin(r#"{ "tool_name": "Write", "tool_input": { "file_path": "/etc/passwd" } }"#) + .assert() + .success() + .stdout(""); +} + +/// Like `guard`, the generic entry point must stay off layered descriptor +/// discovery: it fires per tool call and reads embedded descriptors only. +#[test] +fn guard_hook_generic_skips_descriptor_discovery() { + let tmp = TempDir::new().unwrap(); + let marker = write_armed_marker(tmp.path(), &tmp.path().join(".eval-magic")); + let harnesses = tmp.path().join(".eval-magic").join("harnesses"); + fs::create_dir_all(&harnesses).unwrap(); + fs::write(harnesses.join("broken.toml"), "label = ").unwrap(); + + skill_eval() + .args(["guard-hook", "--harness", "claude-code"]) + .arg(&marker) + .current_dir(tmp.path()) + .write_stdin(r#"{ "tool_name": "Write", "tool_input": { "file_path": "/etc/passwd" } }"#) + .assert() + .success() + .stdout(contains(r#""permissionDecision":"deny""#)) + .stderr(contains("skipping harness descriptor").not()); +} + /// `teardown-guard` reports when no guard is installed (cwd has no marker). #[test] fn teardown_guard_reports_nothing_to_remove() { From 2222e5f516f9e7c72fab70b6a1de370594929985 Mon Sep 17 00:00:00 2001 From: Max Haarhaus Date: Wed, 15 Jul 2026 15:56:44 -0400 Subject: [PATCH 4/5] feat(run): hard preflight rejection of --guard on user-only harnesses --guard with a harness defined by user-supplied descriptors alone now stops the run in preflight instead of downgrading to a warning: the write guard is restricted to embedded built-ins (fail-open safety), and a run the user asked to guard must not continue silently unguarded. The error names the detect-stray-writes fallback. Built-in guardless harnesses (opencode) keep the warn-and-continue downgrade. Registry entries now expose embedded-layer provenance via has_embedded_layer(). Part of #137. Co-Authored-By: Claude Fable 5 --- src/adapters/registry.rs | 42 ++++++++++++++++++++++++++++++++++++++++ src/cli/run/util.rs | 20 ++++++++++++++++++- tests/run/byoh.rs | 37 +++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/src/adapters/registry.rs b/src/adapters/registry.rs index 4441f8a..a25d128 100644 --- a/src/adapters/registry.rs +++ b/src/adapters/registry.rs @@ -35,6 +35,16 @@ struct RegistryEntry { adapter: DescriptorAdapter, } +impl RegistryEntry { + /// True when any contributing source is an embedded built-in — the + /// provenance check behind guard gating (guards are embedded-only). + fn has_embedded_layer(&self) -> bool { + self.sources + .iter() + .any(|(layer, _)| *layer == Layer::Embedded) + } +} + /// Set once by `init_registry` (layered sources), or lazily to the embedded /// built-ins on first pre-init access. The lazy fallback keeps unit tests /// hermetic: they never call `init_registry`, so user-supplied descriptor @@ -317,6 +327,18 @@ pub fn adapter_for(harness: Harness) -> &'static dyn HarnessAdapter { .adapter } +/// True when `harness` has an embedded built-in descriptor among its sources +/// — i.e. it is not defined by user-supplied descriptor files alone. Preflight +/// uses this to hard-reject `--guard` on user-only harnesses (the write guard +/// stays restricted to built-ins because it fails open). +pub fn has_embedded_layer(harness: Harness) -> bool { + registry() + .iter() + .find(|e| e.label == harness.name()) + .expect("Harness handles originate from the registry") + .has_embedded_layer() +} + /// One registered harness as `harness list`/`show`/`lint` see it: identity, /// contributing sources in layer order, the merged descriptor value, and the /// resolved descriptor. @@ -646,6 +668,26 @@ armed_message = "x" assert_eq!(entry.sources.len(), 3); } + #[test] + fn embedded_layer_provenance_distinguishes_built_ins_from_user_only_harnesses() { + let mut sources = embedded_sources(); + sources.push(src( + Layer::ProjectLocal, + ".eval-magic/harnesses/cool.toml", + "label = \"cool\"\n", + )); + let built = build_registry(sources).unwrap(); + let entry = |label: &str| built.entries.iter().find(|e| e.label == label).unwrap(); + assert!( + entry("claude-code").has_embedded_layer(), + "built-ins carry their embedded source" + ); + assert!( + !entry("cool").has_embedded_layer(), + "a user-only harness has no embedded layer" + ); + } + #[test] fn resolve_unknown_name_lists_known_harnesses() { let err = Harness::resolve("nonexistent").unwrap_err().to_string(); diff --git a/src/cli/run/util.rs b/src/cli/run/util.rs index 7b2d60f..7143213 100644 --- a/src/cli/run/util.rs +++ b/src/cli/run/util.rs @@ -8,6 +8,7 @@ use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; use crate::adapters::adapter_for; +use crate::adapters::registry::has_embedded_layer; use crate::core::{Harness, Mode, RunContext}; use super::RunError; @@ -73,7 +74,9 @@ pub(crate) struct HarnessPreflight<'a> { /// Check the run options against the selected harness's declared enhancements /// — the #126 model: a missing enhancement *warns* naming its fallback and the /// run continues degraded; only genuinely contradictory flag combinations -/// (options the harness declares incompatible with `--no-stage`) stay errors. +/// (options the harness declares incompatible with `--no-stage`) and `--guard` +/// on a harness defined by user-supplied descriptors alone (guards are +/// embedded-only) stay errors. /// /// Adjustments: `--guard` on a guard-less harness is forced off (the /// detect-stray-writes audit is the fallback), and a harness without a @@ -108,6 +111,21 @@ pub(crate) fn harness_run_preflight<'a>( ))); } + // `--guard` on a harness defined only by user-supplied descriptors is a + // hard error, not a downgrade: the write guard stays restricted to + // built-in descriptors (it fails open, so a mistyped user descriptor + // would silently disarm it), and a run the user asked to guard must not + // continue silently unguarded. + if opts.guard && !capabilities.supports_guard && !has_embedded_layer(ctx.harness) { + return Err(RunError::msg(format!( + "--guard: --harness {label} comes from user-supplied descriptors only, and the \ + write guard stays restricted to built-in harnesses (it fails open, so a mistyped \ + descriptor would silently disarm it). Rerun without --guard — out-of-bounds \ + writes are detected after the fact by the detect-stray-writes audit (folded \ + into `ingest`)." + ))); + } + let mut opts = opts.clone(); let mut warnings = Vec::new(); if opts.guard && !capabilities.supports_guard { diff --git a/tests/run/byoh.rs b/tests/run/byoh.rs index 967f7b5..e782ac7 100644 --- a/tests/run/byoh.rs +++ b/tests/run/byoh.rs @@ -132,6 +132,43 @@ fn descriptor_alone_carries_a_complete_run() { } } +/// `--guard` with a harness that exists only in user-supplied descriptors is +/// a hard preflight error: guards are restricted to embedded built-ins (fail- +/// open safety), and a run the user asked to guard must not continue silently +/// unguarded. The message names the detect-stray-writes fallback. +#[test] +fn guard_with_a_user_only_harness_is_rejected_in_preflight() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), DEFAULT_EVALS); + write_project_descriptor(&cwd, COOL_DESCRIPTOR); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args([ + "--skill", + "mr-review", + "--mode", + "new-skill", + "--harness", + "cool-custom-harness", + "--guard", + ]) + .assert() + .failure() + .stderr( + contains("built-in") + .and(contains("detect-stray-writes")) + .and(contains("--guard")), + ); + + assert!( + !iteration_dir(&cwd).join("dispatch.json").exists(), + "the run must stop in preflight, before building anything" + ); +} + /// A `--harness-file` descriptor becomes the invocation's default harness /// when `--harness` is omitted. #[test] From 2973efe254a9f239579544943ec9a27484f7125f Mon Sep 17 00:00:00 2001 From: Max Haarhaus Date: Wed, 15 Jul 2026 15:58:53 -0400 Subject: [PATCH 5/5] docs(guard): descriptor-data guard story across docs and messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user-layer restriction message and its doc comment drop the stale 'until the guard engine is opened up' framing — the engine is generic now; guard data stays embedded-only because the guard fails open. progressive-enhancements.md documents the six [guard] fields, the one engine at src/adapters/guard.rs, the frozen guard/guard-codex aliases, and the guard-hook entry point for future built-ins. byoh.md notes the hard --guard preflight rejection; the per-harness notes point at the shared engine; the --guard --help text names the user-descriptor rejection. Part of #137. Co-Authored-By: Claude Fable 5 --- docs/byoh.md | 17 +++++++------- docs/claude-notes.md | 5 ++++- docs/codex-notes.md | 6 ++++- docs/progressive-enhancements.md | 37 ++++++++++++++++++------------- src/adapters/descriptor.rs | 6 ++--- src/adapters/descriptor/layers.rs | 17 +++++++------- src/cli/args.rs | 3 +++ 7 files changed, 55 insertions(+), 36 deletions(-) diff --git a/docs/byoh.md b/docs/byoh.md index e762709..d867e9b 100644 --- a/docs/byoh.md +++ b/docs/byoh.md @@ -122,9 +122,9 @@ and fallbacks per enhancement are in ### Named capabilities: real code for free -Everything that is genuinely code — transcript stitching, guard hooks, slug sanitization, shadow -scanning — is a **named capability** a descriptor references. If your harness emits a compatible -stream, you get the full feature from configuration alone: +Everything that is genuinely code — transcript stitching, slug sanitization, shadow scanning — +is a **named capability** a descriptor references. If your harness emits a compatible stream, +you get the full feature from configuration alone: - `transcript.parser = "claude-stream-json"` — Claude Code `-p --output-format stream-json` events. - `transcript.parser = "codex-items"` — Codex `item.started`/`item.completed` JSONL. @@ -149,11 +149,12 @@ An unknown capability name fails the schema gate listing the allowed values. ### The guard restriction User-supplied descriptors may **not** declare `[guard]` or set `run.supports_guard = true`: the -write guard installs native hook config into dispatch environments and stays restricted to -built-in descriptors until the guard engine is opened up (fail-open safety). Unguarded runs fall -back to the `detect-stray-writes` audit. A project-local overlay of a guarded built-in is fine — -the restriction applies to the user file's own content, and the embedded guard merges through -underneath it. +write guard **fails open** — a mistyped guard block would silently disarm it — so guard data +stays restricted to the embedded built-in descriptors. Unguarded runs fall back to the +`detect-stray-writes` audit, and `run --guard` with a harness defined only by user descriptors is +rejected in preflight (the run stops rather than continuing silently unguarded). A project-local +overlay of a guarded built-in is fine — the restriction applies to the user file's own content, +and the embedded guard merges through underneath it. ## The workflow diff --git a/docs/claude-notes.md b/docs/claude-notes.md index e32a126..c8fac0d 100644 --- a/docs/claude-notes.md +++ b/docs/claude-notes.md @@ -16,7 +16,10 @@ descriptor references: | `stream_json.rs` | `-p --output-format stream-json` transcript parsing (`claude-stream-json`) | | `transcript.rs` | JSONL record shapes + shared tool-call extractors | | `plugin_shadow.rs` | plugin-shadow detection + isolation banner (`claude-plugins`) | -| `guard.rs` | write-guard hook install + `hookSpecificOutput` deny verdict (`claude-hooks`) | + +The write guard has no per-harness code: the descriptor's `[guard]` block (hook file, matcher, +hook-entry and `hookSpecificOutput` verdict templates) is rendered by the generic engine in +`src/adapters/guard.rs`; the hidden `guard` subcommand is its frozen hook entry-point alias. ## Dispatch quirks (all forced by the `claude` CLI) diff --git a/docs/codex-notes.md b/docs/codex-notes.md index 809f901..538561d 100644 --- a/docs/codex-notes.md +++ b/docs/codex-notes.md @@ -14,7 +14,11 @@ references: |------|--------------| | `harnesses/codex.toml` | the descriptor — every declarative value + capability references | | `transcript.rs` | `item.completed` event-stream parsing (`codex-items`) | -| `guard.rs` | write-guard hook install + `{"decision": "block"}` verdict (`codex-hooks`) | + +The write guard has no per-harness code: the descriptor's `[guard]` block (hook file, matcher, +hook-entry and `{"decision": "block"}` verdict templates) is rendered by the generic engine in +`src/adapters/guard.rs`; the hidden `guard-codex` subcommand is its frozen hook entry-point +alias. ## Dispatch (`codex exec`) diff --git a/docs/progressive-enhancements.md b/docs/progressive-enhancements.md index 25acf8f..9bf76b6 100644 --- a/docs/progressive-enhancements.md +++ b/docs/progressive-enhancements.md @@ -64,10 +64,11 @@ only). Only genuinely contradictory flag combinations stay errors. `src/adapters/descriptor_adapter.rs` — the one generic `DescriptorAdapter` implementing the trait from a descriptor. - `src/adapters/capabilities.rs` — the **named capabilities**: closed enums a descriptor references - by kebab-case name for everything that is real code (transcript parsers, guard engines, slug - generation, shadow preflight). -- `src/adapters//` — only the code behind those capabilities: transcript parsers, guard - hooks, the plugin-shadow scan, the OpenCode slug sanitizer. + by kebab-case name for everything that is real code (transcript parsers, slug generation, + shadow preflight). The write guard needs no named capability: it is pure `[guard]` data + rendered by the one generic engine in `src/adapters/guard.rs`. +- `src/adapters//` — only the code behind those capabilities: transcript parsers, the + plugin-shadow scan, the OpenCode slug sanitizer. - `run_capabilities()` (descriptor table `[run]`) + `harness_run_preflight()` (`src/cli/run/util.rs`) — the `run` preflight: undeclared enhancements warn naming their fallback and adjust the options (`--guard` forced off, missing `skills_dir` forces @@ -147,18 +148,24 @@ afterwards. arm whose subagent read the live skill source instead of its staged copy, which contaminates the arm; fatal in revision mode, where the `old_skill` arm then sees new-skill content.) -*Descriptor fields:* the `[guard]` table — `engine`, `armed_message` — plus `[tools]` (the -write/patch/shell/read vocabulary) and `run.supports_guard` (validated to stay in lockstep with -the `[guard]` table). -*Capability:* `guard.engine` names the hook installer (`claude-hooks`, `codex-hooks`); each engine -also exposes its hook matcher so validation can prove every hooked tool is declared in `[tools]`. -The guard arbiter and `detect-stray-writes` classify tool names against the cross-harness -vocabulary union (`all_tool_vocabulary`), so wiring a guard or transcript parser without declaring -the harness's tool names is rejected at descriptor load. The hidden `guard` / `guard-codex` -subcommands are the hook entry points — their names are a stable on-disk contract. Shared +*Descriptor fields:* the `[guard]` table — `hooks_file`, `matcher`, `command_template`, +`hook_entry`, `verdict_template`, `armed_message` — plus `[tools]` (the write/patch/shell/read +vocabulary) and `run.supports_guard` (validated to stay in lockstep with the `[guard]` table). +There is no guard code capability: one generic engine (`src/adapters/guard.rs`) renders the +install (hook entry merged into `hooks_file`) and the deny verdict from these templates, whose +authored JSON key order is serialized verbatim — the verdict bytes are the harness's on-disk +contract. Validation proves every hooked `matcher` tool is declared in `[tools]`, that the +templates parse as JSON, and that their `{command}`/`{matcher}`/`{reason}` placeholders sit in +string values. The guard arbiter and `detect-stray-writes` classify tool names against the +cross-harness vocabulary union (`all_tool_vocabulary`), so wiring a guard or transcript parser +without declaring the harness's tool names is rejected at descriptor load. The hidden `guard` / +`guard-codex` subcommands are frozen hook entry-point aliases (a stable on-disk contract); a +future guard-capable built-in uses the generic `guard-hook --harness