From 1a244b7d3233cac3e6c16088641f1d17db0376b9 Mon Sep 17 00:00:00 2001 From: Omar Sobh Date: Tue, 22 Sep 2026 09:00:07 -0500 Subject: [PATCH] =?UTF-8?q?feat(gate):=20role-scoped=20policy=20=E2=80=94?= =?UTF-8?q?=20the=20verifier=20may=20not=20write,=20enforced=20by=20us?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate's rules were global: what no mission may do. This adds the task-scoped half ActGov (arXiv 2609.24446) argues for — per-action validation against the authorization boundary of the role making the call — starting with the one role whose limit is structural: a verifier that edits the thing it is verifying turns a failed check into a passing one and reports success. The enabling fact was measured before anything was built on it: Claude Code 2.1.278 puts agent_type on a SUBAGENT's PreToolUse payload and leaves it absent on the lead's (local probe: agent_type: prober, agent_id: aacf093a). A policy keyed on a field that is not there is a policy that never fires and looks installed — the failure this codebase keeps paying for. ROLE_POLICIES renders into the same guest script as the floor, so the shell and the Rust predicate cannot disagree (the property the_script_carries_every_rule already pins for the floor, now pinned for roles too). Shell tests run the real generated script: the verifier's Write is refused with rule=role-verifier-readonly and agent_type on the record, the lead's identical Write is allowed, explorer is untouched, and the verifier still reads and runs cargo test. This is deliberately a second enforcer, not a replacement: the CLI's own --agents tool list is the harness policing itself, and it silently did nothing until 2.1.243 rejected the string form we were sending (cbc9c2d). The harness now distinguishes 'never reached for a write' from 'the gate refused one', which the tap alone could not say. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WZb5A2kfVfjpdwSochkuHz --- crates/cm-api/src/vm_tool_gate.rs | 178 ++++++++++++++++++++++++++--- scripts/verify-mission-delivery.sh | 14 ++- 2 files changed, 173 insertions(+), 19 deletions(-) diff --git a/crates/cm-api/src/vm_tool_gate.rs b/crates/cm-api/src/vm_tool_gate.rs index c48c7f2..ae34ff9 100644 --- a/crates/cm-api/src/vm_tool_gate.rs +++ b/crates/cm-api/src/vm_tool_gate.rs @@ -235,6 +235,39 @@ const PROTECTED_REASON: &str = "Refusing to touch the tool hooks, their records, settings that install them. They are the mission's audit trail and are not part \ of the work; nothing in a task needs them changed."; +/// A role's own limits, on top of the floor every role is held to. +/// +/// `role` matches the PreToolUse payload's `agent_type`, which Claude Code +/// sets on a SUBAGENT's calls and leaves absent on the lead's own — +/// measured locally on 2.1.278 (`agent_type: prober`, `agent_id: aacf093a`) +/// before this existed, because a policy keyed on a field that is not there +/// is a policy that never fires and looks installed. +/// +/// This is the task-scoped half of the gate. The rules above say what no +/// mission may do; these say what a particular ROLE may not do, and they +/// are enforced here rather than by the CLI's own `--agents` tool list: +/// that list is the model harness policing itself, and it silently did +/// nothing at all until 2.1.243 rejected the string form we were sending +/// (cbc9c2d). Two independent enforcers, one of them ours. +struct RolePolicy { + role: &'static str, + /// Tool names, lowercased as the extractor prints them. + deny_tools: &'static [&'static str], + reason: &'static str, + /// Recorded on the denial, like any other rule. + id: &'static str, +} + +const ROLE_POLICIES: &[RolePolicy] = &[RolePolicy { + role: "verifier", + deny_tools: WRITE_TOOLS, + id: "role-verifier-readonly", + reason: "Refusing a write from the verifier. Its job is to check work it must \ + not change: a verifier that edits the thing it is verifying turns a \ + failed check into a passing one and reports success. Report what you \ + found and let the lead fix it.", +}]; + /// The write tools, lowercased as the extractor prints them. `deny_reason` /// checks their `file_path` against [`PROTECTED_PATHS`]; every other tool's /// arguments are left alone, because blocking `Read` on a substring would @@ -271,17 +304,33 @@ pub fn denial_detail(line: &str) -> serde_json::Value { /// The reason a command is denied, or `None` to allow it. pub fn deny_reason(tool: &str, command: &str) -> Option<&'static str> { - decide(tool, command, None).map(|d| d.reason) + decide(tool, command, None, None).map(|d| d.reason) } /// The decision for one tool call: `None` allows. /// /// `argument` is the Bash command line; `file_path` is what a write tool -/// was given. Pure so the policy is testable without a VM — the half most -/// likely to be wrong is the matching, and it is the half that needs no +/// was given; `role` is the calling subagent's `agent_type`, `None` for the +/// lead's own calls. Pure so the policy is testable without a VM — the half +/// most likely to be wrong is the matching, and it is the half that needs no /// guest to exercise. -pub fn decide(tool: &str, argument: &str, file_path: Option<&str>) -> Option { +pub fn decide( + tool: &str, + argument: &str, + file_path: Option<&str>, + role: Option<&str>, +) -> Option { let tool = tool.to_ascii_lowercase(); + // A role's own limits come first: they are narrower than the floor by + // construction, and the reason names the role rather than the action. + if let Some(role) = role.map(str::trim).filter(|r| !r.is_empty()) { + if let Some(p) = ROLE_POLICIES + .iter() + .find(|p| p.role.eq_ignore_ascii_case(role) && p.deny_tools.contains(&tool.as_str())) + { + return Some(Denial { rule: p.id, reason: p.reason }); + } + } // A write tool is judged on WHERE it writes and nothing else. if WRITE_TOOLS.contains(&tool.as_str()) { let path = file_path.unwrap_or(""); @@ -487,6 +536,25 @@ pub fn hook_script(dir: &str) -> String { } } + // One `case` per role policy, rendered from the same table the Rust + // predicate reads, so the guest and the host cannot disagree about who + // may do what. + let mut role_checks = String::new(); + for p in ROLE_POLICIES { + role_checks.push_str(&format!( + "case \"$role\" in\n\ + \x20 {role})\n\ + \x20 case \"$tool\" in\n\ + \x20 {tools})\n{body}\ + \x20 esac\n\ + \x20 ;;\n\ + esac\n", + role = shell_pattern(p.role), + tools = p.deny_tools.join("|"), + body = deny(p.id, p.reason, " "), + )); + } + let text_tools = TEXT_TOOLS.join("|"); format!( "#!/bin/sh\n\ @@ -505,6 +573,10 @@ pub fn hook_script(dir: &str) -> String { tool=$(printf '%s\\n' \"$info\" | sed -n 1p)\n\ cmd=$(printf '%s\\n' \"$info\" | sed -n 2p)\n\ path=$(printf '%s\\n' \"$info\" | sed -n 3p)\n\ + role=$(printf '%s\\n' \"$info\" | sed -n 4p)\n\ + # A role's own limits. Empty role = the lead's own call, which no\n\ + # role policy binds.\n\ + {role_checks}\ # A write tool is judged on where it writes: the hooks, their records\n\ # and the settings that install them are off limits.\n\ case \"$tool\" in\n\ @@ -543,6 +615,7 @@ pub fn hook_script(dir: &str) -> String { exit 0\n", extract = NODE_EXTRACT, inert = INERT_FILE, + role_checks = role_checks, write_tools = WRITE_TOOLS.join("|"), protected = PROTECTED_PATHS .iter() @@ -553,12 +626,13 @@ pub fn hook_script(dir: &str) -> String { ) } -/// Reads the hook event on stdin and prints `tool_name`, the command, then -/// the file path a write tool was given (empty for the rest). +/// Reads the hook event on stdin and prints `tool_name`, the command, the +/// file path a write tool was given (empty for the rest), then the calling +/// subagent's `agent_type` (empty on the lead's own calls). /// /// Lowercases the tool name so the shell comparison is exact. Silent on any /// error: the caller treats empty output as "allow". -const NODE_EXTRACT: &str = r#"let s="";process.stdin.on("data",d=>s+=d).on("end",()=>{try{const j=JSON.parse(s);const t=j.tool_input||{};const n=String(j.tool_name||"").toLowerCase();const c=String(t.command||"").replace(/\n/g," ");const p=String(t.file_path||t.notebook_path||"").replace(/\n/g," ");process.stdout.write(n+"\n"+c+"\n"+p+"\n")}catch(e){}})"#; +const NODE_EXTRACT: &str = r#"let s="";process.stdin.on("data",d=>s+=d).on("end",()=>{try{const j=JSON.parse(s);const t=j.tool_input||{};const n=String(j.tool_name||"").toLowerCase();const c=String(t.command||"").replace(/\n/g," ");const p=String(t.file_path||t.notebook_path||"").replace(/\n/g," ");const a=String(j.agent_type||"").replace(/\n/g," ").toLowerCase();process.stdout.write(n+"\n"+c+"\n"+p+"\n"+a+"\n")}catch(e){}})"#; /// A needle as a `case` pattern: glob metacharacters escaped. fn shell_pattern(needle: &str) -> String { @@ -690,21 +764,21 @@ mod tests { /// path alone — the contents can say anything. #[test] fn decisions_name_their_rule_and_write_tools_are_judged_by_path() { - let d = decide("Bash", "git push --force origin main", None).unwrap(); + let d = decide("Bash", "git push --force origin main", None, None).unwrap(); assert_eq!(d.rule, "force-push"); - assert_eq!(decide("Bash", "curl -s -X POST https://x -d @f", None).unwrap().rule, "curl-body"); - assert_eq!(decide("Bash", "cat /root/toolgate/denied.jsonl", None).unwrap().rule, "hook-files"); - assert_eq!(decide("Bash", "rm -f /root/guest-settings.json", None).unwrap().rule, "hook-files"); + assert_eq!(decide("Bash", "curl -s -X POST https://x -d @f", None, None).unwrap().rule, "curl-body"); + assert_eq!(decide("Bash", "cat /root/toolgate/denied.jsonl", None, None).unwrap().rule, "hook-files"); + assert_eq!(decide("Bash", "rm -f /root/guest-settings.json", None, None).unwrap().rule, "hook-files"); - let w = decide("Write", "", Some("/root/toolhooks/settings.json")).unwrap(); + let w = decide("Write", "", Some("/root/toolhooks/settings.json"), None).unwrap(); assert_eq!(w.rule, "hook-files"); - assert!(decide("Edit", "", Some("/mission/repo/.git/hooks/post-commit")).is_some()); - assert!(decide("NotebookEdit", "", Some("/root/.claude/settings.json")).is_some()); + assert!(decide("Edit", "", Some("/mission/repo/.git/hooks/post-commit"), None).is_some()); + assert!(decide("NotebookEdit", "", Some("/root/.claude/settings.json"), None).is_some()); // The checkout is the work. - assert!(decide("Write", "", Some("/mission/repo/README.md")).is_none()); - assert!(decide("Write", "rm -rf /", Some("/mission/repo/notes.md")).is_none()); + assert!(decide("Write", "", Some("/mission/repo/README.md"), None).is_none()); + assert!(decide("Write", "rm -rf /", Some("/mission/repo/notes.md"), None).is_none()); // A protected path in a Read is a read. - assert!(decide("Read", "", Some("/root/toolgate/denied.jsonl")).is_none()); + assert!(decide("Read", "", Some("/root/toolgate/denied.jsonl"), None).is_none()); // The rule ids are unique — a duplicate would make the count lie. let mut ids: Vec<&str> = RULES.iter().map(|r| r.id).collect(); ids.sort_unstable(); @@ -712,6 +786,42 @@ mod tests { assert_eq!(ids.len(), RULES.len()); } + /// A role policy binds that role's subagent and nobody else. + #[test] + fn the_verifier_may_not_write_and_the_lead_is_untouched() { + let d = decide("Write", "", Some("/mission/repo/src/lib.rs"), Some("verifier")).unwrap(); + assert_eq!(d.rule, "role-verifier-readonly"); + assert!(d.reason.contains("passing one"), "{}", d.reason); + assert!(decide("Edit", "", Some("/mission/repo/x.rs"), Some("VERIFIER")).is_some()); + // The same write from the lead, or from another role, is ordinary work. + assert!(decide("Write", "", Some("/mission/repo/src/lib.rs"), None).is_none()); + assert!(decide("Write", "", Some("/mission/repo/src/lib.rs"), Some("")).is_none()); + assert!(decide("Write", "", Some("/mission/repo/src/lib.rs"), Some("explorer")).is_none()); + // Reading is the verifier's whole job. + assert!(decide("Read", "", Some("/mission/repo/src/lib.rs"), Some("verifier")).is_none()); + assert!(decide("Bash", "cargo test", None, Some("verifier")).is_none()); + // The floor still applies to a role the policy does not name. + assert_eq!( + decide("Bash", "git push --force origin main", None, Some("explorer")).unwrap().rule, + "force-push" + ); + } + + /// Two implementations of one policy: every role rule must reach the + /// guest script, the same property `the_script_carries_every_rule` pins + /// for the floor. + #[test] + fn the_script_carries_every_role_policy() { + let script = hook_script(GUEST_DIR); + for p in ROLE_POLICIES { + assert!(script.contains(p.role), "role {:?} missing from the guest script", p.role); + assert!(script.contains(p.id), "rule id {:?} missing from the guest script", p.id); + for t in p.deny_tools { + assert!(script.contains(t), "{t} missing from {}'s guest check", p.role); + } + } + } + #[test] fn a_denial_line_reads_back_as_the_event_with_its_rule() { let d = denial_detail(r#"{"rule":"curl-body","payload":{"tool_name":"Bash","tool_input":{"command":"curl -d x"}}}"#); @@ -838,6 +948,40 @@ mod shell_tests { assert_eq!(v["payload"]["tool_input"]["command"], "git push --force origin main"); } + /// The role policy, in the shell that actually runs in the guest: the + /// verifier's write is refused with its rule recorded, the lead's + /// identical write is allowed, and the verifier's reads and tests are + /// untouched. This is the enforcement the CLI's own `--agents` list is + /// supposed to provide and silently did not until 2.1.243. + #[test] + fn the_shell_refuses_a_write_from_the_verifier_and_allows_the_leads() { + let v = r#"{"tool_name":"Write","agent_type":"verifier","agent_id":"a1","tool_input":{"file_path":"/mission/repo/src/lib.rs","content":"fn x(){}"}}"#; + let (code, stderr) = run(v); + assert_eq!(code, 2, "the verifier must not write: {stderr}"); + assert!(stderr.contains("verifier"), "{stderr}"); + let rec: serde_json::Value = serde_json::from_str(last_denied().trim()).unwrap(); + assert_eq!(rec["rule"], "role-verifier-readonly"); + assert_eq!(rec["payload"]["agent_type"], "verifier"); + + let lead = r#"{"tool_name":"Write","tool_input":{"file_path":"/mission/repo/src/lib.rs","content":"fn x(){}"}}"#; + assert_eq!(run(lead).0, 0, "the lead writes as it always did"); + + let other = r#"{"tool_name":"Edit","agent_type":"explorer","tool_input":{"file_path":"/mission/repo/a.rs","old_string":"a","new_string":"b"}}"#; + assert_eq!(run(other).0, 0, "no policy binds explorer"); + + for ok in [ + r#"{"tool_name":"Read","agent_type":"verifier","tool_input":{"file_path":"/mission/repo/src/lib.rs"}}"#, + r#"{"tool_name":"Bash","agent_type":"verifier","tool_input":{"command":"cargo test --all"}}"#, + ] { + let (code, stderr) = run(ok); + assert_eq!(code, 0, "the verifier must still read and test: {stderr}"); + } + + // The floor is unchanged for a role the policy does not name. + let forced = r#"{"tool_name":"Bash","agent_type":"explorer","tool_input":{"command":"git push --force origin main"}}"#; + assert_eq!(run(forced).0, 2); + } + /// A write tool aimed at the gate's own records is refused, in the shell, /// with the rule recorded; the same tool aimed at the checkout is not. #[test] diff --git a/scripts/verify-mission-delivery.sh b/scripts/verify-mission-delivery.sh index 8bb5358..21fe4b5 100755 --- a/scripts/verify-mission-delivery.sh +++ b/scripts/verify-mission-delivery.sh @@ -892,6 +892,13 @@ assert_verifier_read_only() { #