From bfcf2e8d5b46a888e63dec7c7f97319d37e602b1 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 1 Sep 2026 16:09:55 +0800 Subject: [PATCH 1/9] feat(execpolicy): skip cmd.exe single-letter slash flags cmd.exe spells its flags with a slash plus exactly one letter (del /f /s /q, xcopy /e /y), in any order and position. The denied_prefix_matches DFS now skips such tokens the same way it skips '-' flags: alone, and together with a following token when the flag could take a separate value, while a rule token that names the flag itself is still consumed as a match first. The single-letter shape is load-bearing: multi-character '/'-tokens are real POSIX paths (/tmp, /etc, /usr, /dev) and must keep matching positionally, or an exfil command such as 'cp /tmp/new_key ~/.ssh/authorized_keys' would slip past a rule guarding ~/.ssh/authorized_keys. Case needs no extra handling because normalize_command already lowercases tokens. Signed-off-by: asto --- crates/execpolicy/src/lib.rs | 97 ++++++++++++++++++++++++++++++++++-- 1 file changed, 94 insertions(+), 3 deletions(-) diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs index 4ca877cdb8..81f7fbe585 100644 --- a/crates/execpolicy/src/lib.rs +++ b/crates/execpolicy/src/lib.rs @@ -726,6 +726,11 @@ fn command_is_chained(command: &str) -> bool { /// direction. Matching stays anchored at the first positional token, so a /// non-flag token that isn't in the rule ends it — `git push` does not block /// `git checkout push`, and `rm` does not block `rmdir`. +/// +/// One command-side spelling widens what a rule can name: cmd.exe-style +/// single-letter `/` flags (`del /f /s /q`) are skippable like `-` flags, in +/// any position, so a rule holds against every interleaving without the app +/// enumerating canonical flag sequences. fn denied_prefix_matches(rule: &str, command: &str) -> bool { let rule_tokens: Vec = normalize_command(rule) .split_whitespace() @@ -776,11 +781,16 @@ fn denied_prefix_matches(rule: &str, command: &str) -> bool { if matches_rule_token { stack.push((i + 1, j + 1)); } - if token.starts_with('-') { + if token.starts_with('-') || is_single_letter_slash_flag(token) { // An unrelated flag is skippable — alone, and (when it could take // a separate value) together with the token after it. Consuming it - // as a rule token above takes priority, so a rule that names a flag - // (`cargo test --danger`) still matches it. + // as a rule token above takes priority, so a rule that names a + // flag (`cargo test --danger`) still matches it. cmd.exe spells + // its flags the same way shells spell paths, so only the + // single-letter shape (`/f`, `/s`, `/q`, `/y`) may skip; anything + // longer is a POSIX path (`/tmp`, `/etc`, `/usr`, `/dev`) and must + // stay positional, or `cp /tmp/new_key ~/.ssh/authorized_keys` + // would slip past a rule guarding `~/.ssh/authorized_keys`. stack.push((i + 1, j)); if !token.contains('=') { stack.push((i + 2, j)); @@ -792,6 +802,18 @@ fn denied_prefix_matches(rule: &str, command: &str) -> bool { false } +/// True for a cmd.exe-style single-letter flag such as `/f`, `/s`, `/q`, `/y`. +/// +/// cmd.exe flags are a slash plus exactly one letter (`del /f /s /q`, `xcopy +/// /e /y`), so only that shape may skip like a `-` flag. The narrowness is +/// load-bearing: multi-character `/`-tokens are real POSIX paths (`/tmp`, +/// `/etc`, `/usr`, `/dev`) and must keep matching positionally. Case needs no +/// handling here — `normalize_command` has already lowercased the token. +fn is_single_letter_slash_flag(token: &str) -> bool { + let bytes = token.as_bytes(); + bytes.len() == 2 && bytes[0] == b'/' && bytes[1].is_ascii_alphabetic() +} + /// Whether a command word matches a deny rule's command word. /// /// Exact first, then the command's basename — `/bin/rm`, `./rm`, and @@ -1394,6 +1416,75 @@ mod tests { assert!(allowed.allow, "rmdir must not be denied: {allowed:?}"); } + #[test] + fn denied_prefix_skips_cmd_exe_single_letter_slash_flags() { + // cmd.exe spells its flags `/f`, `/s`, `/q` — a slash plus exactly one + // letter, in any order and position. A deny rule must hold against + // every interleaving (`del /f /s /q`, `del /q /s /f`, ...); the app + // would otherwise have to enumerate canonical flag sequences, so the + // engine skips the shape itself, like `-` flags. + let engine = ExecPolicyEngine::new( + vec![], + vec![ + r"del c:\users\x\file".to_string(), + r"xcopy c:\src d:\dst".to_string(), + ], + ); + for command in [ + r"del c:\users\x\file", + r"del /f c:\users\x\file", + r"del /f /s /q c:\users\x\file", + r"del /q /s /f c:\users\x\file", + r"del /f c:\users\x\file /s /q", + r"xcopy /e /y c:\src d:\dst", + ] { + let decision = engine.check(ctx(command, AskForApproval::Never)).unwrap(); + assert!( + !decision.allow, + "cmd.exe flag spelling evaded deny: {command:?} -> {decision:?}" + ); + } + // A rule that NAMES a `/x` flag still consumes it as a rule token — + // the rule-token branch is tried before the skip branches. + let named = ExecPolicyEngine::new(vec![], vec![r"del /q c:\x".to_string()]); + let decision = named + .check(ctx(r"del /q c:\x", AskForApproval::Never)) + .unwrap(); + assert!( + !decision.allow, + "rule naming a slash flag missed: {decision:?}" + ); + } + + #[test] + fn denied_prefix_slash_skipping_keeps_multi_char_slash_tokens_positional() { + // The single-letter constraint is load-bearing: `/tmp` is a POSIX + // directory, not a flag. If multi-character `/`-tokens skipped, an + // exfil command could hide its real operand behind a skipped path and + // slip past a rule guarding the sensitive target. + let engine = ExecPolicyEngine::new(vec![], vec!["cp ~/.ssh/authorized_keys".to_string()]); + for command in [ + "cp /tmp/new_key ~/.ssh/authorized_keys", + "cp /etc/passwd ~/.ssh/authorized_keys", + ] { + let decision = engine + .check(ctx(command, AskForApproval::UnlessTrusted)) + .unwrap(); + assert!( + decision.allow, + "POSIX path argument wrongly treated as a flag: {command:?} -> {decision:?}" + ); + } + // The guarded target itself still denies, skip branches or not. + let denied = engine + .check(ctx( + "cp ~/.ssh/authorized_keys ~/.ssh/authorized_keys.bak", + AskForApproval::Never, + )) + .unwrap(); + assert!(!denied.allow, "guarded target must stay denied: {denied:?}"); + } + #[test] fn path_rules_respect_filesystem_case_sensitivity() { // #4725: on a case-sensitive filesystem `config/allowed.toml` and From e5a626159eb18599014acda56e9283ca0e34d66c Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 1 Sep 2026 16:10:11 +0800 Subject: [PATCH 2/9] feat(execpolicy): add middle wildcard token to deny rules A deny rule token of exactly '*' now matches zero or more consecutive command tokens regardless of their shape, via two DFS branches: (i, j+1) matches nothing, (i+1, j) skips one more command token. 'seen' keeps the state space finite, and the branch runs before the end-of-command bail so a trailing '*' still matches zero tokens and degrades to plain prefix semantics. A wildcard is never itself treated as a command word; a leading '*' gets the generic branches, with the command-word anchor kept at the rule's literal first token (documented by test). This lets a rule anchor on its sensitive tail (grep * ~/.ssh/id_rsa, dd * of=/dev/sda) without the app enumerating every flag spelling, covering interleavings like 'grep -i PATTERN ~/.ssh/id_rsa'. Middle wildcards widen the deny face of a rule: the engine is deliberately permissive and the rulesets feeding it own the false-positive discipline of justifying each wildcard. Signed-off-by: asto --- crates/execpolicy/src/lib.rs | 140 ++++++++++++++++++++++++++++++++++- 1 file changed, 136 insertions(+), 4 deletions(-) diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs index 81f7fbe585..84f68e3825 100644 --- a/crates/execpolicy/src/lib.rs +++ b/crates/execpolicy/src/lib.rs @@ -727,10 +727,15 @@ fn command_is_chained(command: &str) -> bool { /// non-flag token that isn't in the rule ends it — `git push` does not block /// `git checkout push`, and `rm` does not block `rmdir`. /// -/// One command-side spelling widens what a rule can name: cmd.exe-style -/// single-letter `/` flags (`del /f /s /q`) are skippable like `-` flags, in -/// any position, so a rule holds against every interleaving without the app -/// enumerating canonical flag sequences. +/// Two rule-side spellings widen what a rule can name. cmd.exe-style +/// single-letter `/` flags (`del /f /s /q`) in the *command* are skippable like +/// `-` flags, in any position. And a rule token of exactly `*` is a middle +/// wildcard matching zero or more consecutive command tokens regardless of +/// shape, so a rule can anchor on a tail (`grep * ~/.ssh/id_rsa`, +/// `dd * of=/dev/sda`) without enumerating every flag spelling. A wildcard +/// widens the deny face of a rule — each one must be justified by the rule +/// author. This engine is deliberately permissive; the rulesets that feed it +/// own the false-positive discipline of keeping wildcards narrow. fn denied_prefix_matches(rule: &str, command: &str) -> bool { let rule_tokens: Vec = normalize_command(rule) .split_whitespace() @@ -763,6 +768,24 @@ fn denied_prefix_matches(rule: &str, command: &str) -> bool { if j == rule_tokens.len() { return true; } + // A rule token of exactly `*` is a middle wildcard: it matches zero or + // more consecutive command tokens regardless of shape — that is its + // point, since `grep -i PATTERN ~/.ssh/id_rsa` interleaves flags and + // positionals no flag rule could enumerate. `(i, j+1)` lets it match + // nothing; `(i+1, j)` skips one more command token. `seen` keeps the + // run of states finite. This branch runs BEFORE the end-of-command + // bail below so a trailing `*` can still match zero tokens once the + // command is exhausted, degrading to plain prefix semantics, and a + // wildcard is never itself treated as a command word. + if rule_tokens[j] == "*" { + if seen.insert((i, j)) { + stack.push((i, j + 1)); + if i < command_tokens.len() { + stack.push((i + 1, j)); + } + } + continue; + } if i >= command_tokens.len() || !seen.insert((i, j)) { continue; } @@ -1485,6 +1508,115 @@ mod tests { assert!(!denied.allow, "guarded target must stay denied: {denied:?}"); } + #[test] + fn denied_prefix_middle_wildcard_matches_zero_or_more_tokens() { + // A rule token of exactly `*` matches zero or more consecutive command + // tokens REGARDLESS of shape — flags, flag values, extra positionals — + // so a rule can anchor on its sensitive tail without the app + // enumerating every flag spelling. + let engine = ExecPolicyEngine::new( + vec![], + vec![ + "grep * ~/.ssh/id_rsa".to_string(), + "dd * of=/dev/sda".to_string(), + ], + ); + for command in [ + "grep root ~/.ssh/id_rsa", + "grep -i root ~/.ssh/id_rsa", + "grep -r root ~/.ssh/id_rsa", + // The wildcard matches nothing at all. + "grep ~/.ssh/id_rsa", + // `dd` has no dash flags at all: its operands are `key=value`. + "dd if=/dev/zero of=/dev/sda", + "dd if=boot.img bs=1M of=/dev/sda", + // A trailing `*` is allowed and degrades to plain prefix + // semantics: once reached, the rule matches. + "grep -i root ~/.ssh/id_rsa > /tmp/out", + ] { + let decision = engine.check(ctx(command, AskForApproval::Never)).unwrap(); + assert!( + !decision.allow, + "wildcard rule missed {command:?}: {decision:?}" + ); + } + + // A rule whose LAST token is `*` still matches a shorter command — + // prefix semantics, not suffix equality. + let trailing = ExecPolicyEngine::new(vec![], vec!["grep * ~/.ssh/id_rsa *".to_string()]); + for command in [ + "grep root ~/.ssh/id_rsa", + "grep -i root ~/.ssh/id_rsa backup", + ] { + let decision = trailing.check(ctx(command, AskForApproval::Never)).unwrap(); + assert!( + !decision.allow, + "trailing-wildcard rule missed {command:?}: {decision:?}" + ); + } + } + + #[test] + fn denied_prefix_wildcard_stays_anchored_on_the_tail_token() { + // The wildcard bridges the MIDDLE of a rule; it does not relax the + // tail. A rule is still a prefix match: when the tail token never + // appears in the segment, there is no deny — here or inside a chain. + let engine = ExecPolicyEngine::new(vec![], vec!["grep * /home/z".to_string()]); + for command in ["grep x /etc/y", "ls && grep x /etc/y"] { + let decision = engine + .check(ctx(command, AskForApproval::UnlessTrusted)) + .unwrap(); + assert!( + decision.allow, + "wildcard rule over-matched {command:?}: {decision:?}" + ); + } + // Chained segments are still scanned individually: a wildcard rule + // denies when its anchor appears in ANY segment, and does not leak + // across the chain boundary in either direction. + let chain = ExecPolicyEngine::new(vec![], vec!["grep * ~/.ssh/id_rsa".to_string()]); + let denied = chain + .check(ctx( + "echo hi && grep root ~/.ssh/id_rsa", + AskForApproval::Never, + )) + .unwrap(); + assert!(!denied.allow, "chained segment must still deny: {denied:?}"); + let shielded = chain + .check(ctx("grep x /etc/y && echo done", AskForApproval::Never)) + .unwrap(); + assert!( + shielded.allow, + "wildcard must not reach into unrelated segments: {shielded:?}" + ); + } + + #[test] + fn denied_prefix_leading_wildcard_follows_generic_wildcard_semantics() { + // Rules in practice anchor their first token, but a leading `*` is not + // an error: the generic DFS gives it the same two branches and it is + // never treated as a command word. Documented consequence of keeping + // the anchor at the rule's literal first token: the command word after + // a leading wildcard is matched exactly, so `/bin/rm` is NOT folded to + // `rm` for it. Rule authors should not start rules with `*`; this test + // only pins the behavior the generic DFS produces. + let engine = ExecPolicyEngine::new(vec![], vec!["* rm -rf /".to_string()]); + let bare = engine + .check(ctx("rm -rf /", AskForApproval::Never)) + .unwrap(); + assert!( + !bare.allow, + "leading-wildcard rule must match its bare spelling: {bare:?}" + ); + let path = engine + .check(ctx("/bin/rm -rf /", AskForApproval::Never)) + .unwrap(); + assert!( + path.allow, + "leading wildcard must not gain command-word folding: {path:?}" + ); + } + #[test] fn path_rules_respect_filesystem_case_sensitivity() { // #4725: on a case-sensitive filesystem `config/allowed.toml` and From 468c9c0a5448329dc99e03d7e1528757b639aca5 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 1 Sep 2026 16:10:23 +0800 Subject: [PATCH 3/9] feat(execpolicy): fold .exe suffix on deny command word command_word_matches now also folds a trailing '.exe' from the command token's basename at the j==0 anchor: rule 'cat' matches 'cat.exe' and 'C:\Windows\System32\cat.exe', which are the same binary as the 'cat' the rule names. At most one '.exe' suffix strips, so 'catalog'/'catalog.exe' never fold into rule 'cat'. The fold stays one-directional like the basename fold: when the RULE itself ends in '.exe' (the existing control.exe rule), the fold is suppressed and the bare 'control' does not match, keeping the rule's requirement of that exact spelling. Signed-off-by: asto --- crates/execpolicy/src/lib.rs | 62 +++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs index 84f68e3825..fc7df8492e 100644 --- a/crates/execpolicy/src/lib.rs +++ b/crates/execpolicy/src/lib.rs @@ -844,6 +844,13 @@ fn is_single_letter_slash_flag(token: &str) -> bool { /// direction only: a rule that spells a path (`/usr/bin/rm`) still requires /// that path, because the rule author asked for it specifically. Both /// separators are honored so a Windows spelling cannot slip past. +/// +/// A trailing `.exe` on the command's basename also folds: Windows spells the +/// same binary `cat.exe` or `C:\Windows\System32\cat.exe`, and a `cat +/// ~/.ssh/id_rsa` rule must hold against that spelling too. The fold is one +/// direction only — when the RULE itself ends in `.exe` (`control.exe`) it +/// keeps requiring that spelling, and `catalog` never matches `cat` because +/// only a whole `.exe` suffix strips, never a prefix. fn command_word_matches(rule_token: &str, command_token: &str) -> bool { if command_token == rule_token { return true; @@ -852,10 +859,15 @@ fn command_word_matches(rule_token: &str, command_token: &str) -> bool { if rule_token.contains('/') || rule_token.contains('\\') { return false; } - let basename = command_token + let mut basename = command_token .rsplit(['/', '\\']) .next() .unwrap_or(command_token); + if !rule_token.ends_with(".exe") + && let Some(stem) = basename.strip_suffix(".exe") + { + basename = stem; + } !basename.is_empty() && basename == rule_token } @@ -1617,6 +1629,54 @@ mod tests { ); } + #[test] + fn denied_prefix_folds_windows_exe_suffix_on_the_command_word() { + // Windows spells the same binary `cat.exe` or + // `C:\Windows\System32\cat.exe`; a `cat ~/.ssh/id_rsa` rule must hold + // against those spellings. The fold is one-directional: a rule that + // names `.exe` itself keeps requiring it, and only a WHOLE `.exe` + // suffix strips — `catalog` never becomes `cat`. + let engine = ExecPolicyEngine::new(vec![], vec!["cat ~/.ssh/id_rsa".to_string()]); + for command in [ + "cat ~/.ssh/id_rsa", + "cat.exe ~/.ssh/id_rsa", + "cat.EXE ~/.ssh/id_rsa", + r"C:\Windows\System32\cat.exe ~/.ssh/id_rsa", + ] { + let decision = engine.check(ctx(command, AskForApproval::Never)).unwrap(); + assert!( + !decision.allow, + "`.exe` spelling evaded deny: {command:?} -> {decision:?}" + ); + } + + // A rule ending in `.exe` must still require that spelling: the bare + // `control` is a different binary and must not match `control.exe`. + let control = ExecPolicyEngine::new(vec![], vec!["control.exe".to_string()]); + let spelled = control + .check(ctx("control.exe", AskForApproval::Never)) + .unwrap(); + assert!(!spelled.allow, "control.exe must be denied: {spelled:?}"); + let bare = control + .check(ctx("control", AskForApproval::UnlessTrusted)) + .unwrap(); + assert!( + bare.allow, + "bare `control` must not match rule `control.exe`: {bare:?}" + ); + + // Only a whole `.exe` suffix folds, never a word prefix. + for command in ["catalog ~/.ssh/id_rsa", "catalog.exe ~/.ssh/id_rsa"] { + let decision = engine + .check(ctx(command, AskForApproval::UnlessTrusted)) + .unwrap(); + assert!( + decision.allow, + "prefix word must not fold into the rule word: {command:?} -> {decision:?}" + ); + } + } + #[test] fn path_rules_respect_filesystem_case_sensitivity() { // #4725: on a case-sensitive filesystem `config/allowed.toml` and From cd79e58f9be6844082271f208a36a3106f80d101 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 1 Sep 2026 16:04:54 +0800 Subject: [PATCH 4/9] feat(execpolicy): share live rulesets across clones The layered rulesets now live behind an Arc> so that set_ruleset applied through any clone of an ExecPolicyEngine is observed by every other clone. Hosts clone the engine into long-lived side executors (nested sub-agent tool registries read it through the SubAgentRuntime that the parent turn loop hands down). With a plain Vec those executors kept a snapshot taken at spawn time, so a permission ruleset updated mid-session (Op::SetPermissionRuleset) never bound commands that the parent had already delegated to a running child - a hard deny on the main line was not a hard deny inside the child. Clone-visible sharing keeps the existing value-semantics call sites (Config -> EngineConfig -> runtime) unchanged while making the live mutation surface (set_ruleset / add_ruleset) globally visible. Per-engine session approvals stay per-clone, which is the historical behavior and irrelevant to hard-deny enforcement. Signed-off-by: asto --- crates/execpolicy/src/lib.rs | 57 ++++++++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs index fc7df8492e..5b696fa0a6 100644 --- a/crates/execpolicy/src/lib.rs +++ b/crates/execpolicy/src/lib.rs @@ -2,6 +2,7 @@ pub mod bash_arity; pub mod shell_expand; use std::collections::HashSet; +use std::sync::{Arc, RwLock}; use anyhow::Result; use bash_arity::BashArityDict; @@ -312,7 +313,14 @@ pub struct ExecPolicyContext<'a> { pub struct ExecPolicyEngine { /// Layered rulesets (builtin → agent → user). When non-empty, takes precedence /// over the legacy flat lists below. - rulesets: Vec, + /// + /// Shared behind an `Arc>` so that [`Self::set_ruleset`] applied + /// through one clone is observed by every clone. Hosts clone the engine + /// into long-lived side executors (nested sub-agent tool registries); a + /// plain `Vec` would leave those executors on a stale ruleset after a live + /// permission update, reopening an enforcement gap the parent no longer + /// has. + rulesets: Arc>>, /// Legacy flat lists kept for backward compatibility with `new()`. trusted_prefixes: Vec, denied_prefixes: Vec, @@ -325,7 +333,7 @@ impl ExecPolicyEngine { /// Legacy constructor: wraps the two vecs into a User-layer ruleset. pub fn new(trusted_prefixes: Vec, denied_prefixes: Vec) -> Self { Self { - rulesets: vec![], + rulesets: Arc::new(RwLock::new(vec![])), trusted_prefixes, denied_prefixes, approved_for_session: HashSet::new(), @@ -338,7 +346,7 @@ impl ExecPolicyEngine { pub fn with_rulesets(mut rulesets: Vec) -> Self { rulesets.sort_by_key(|r| r.layer); Self { - rulesets, + rulesets: Arc::new(RwLock::new(rulesets)), trusted_prefixes: vec![], denied_prefixes: vec![], approved_for_session: HashSet::new(), @@ -348,17 +356,37 @@ impl ExecPolicyEngine { /// Add a ruleset layer (re-sorts internally). pub fn add_ruleset(&mut self, ruleset: Ruleset) { - self.rulesets.push(ruleset); - self.rulesets.sort_by_key(|r| r.layer); + let mut guard = Self::lock_rulesets(&self.rulesets); + guard.push(ruleset); + guard.sort_by_key(|r| r.layer); } /// Replace the ruleset at one priority layer without clearing approvals /// remembered for the current session. pub fn set_ruleset(&mut self, ruleset: Ruleset) { + let mut guard = Self::lock_rulesets(&self.rulesets); + guard.retain(|existing| existing.layer != ruleset.layer); + guard.push(ruleset); + guard.sort_by_key(|existing| existing.layer); + } + + /// Lock the shared ruleset list for reading or writing. + /// + /// A poisoned lock (a panic held the guard mid-mutation) is recovered from: + /// the ruleset vec is plain data and a torn update sorts itself out on the + /// next `set_ruleset`, while refusing to answer policy checks would fail + /// closed for every command in the process. + fn lock_rulesets( + rulesets: &Arc>>, + ) -> std::sync::RwLockWriteGuard<'_, Vec> { + rulesets.write().unwrap_or_else(std::sync::PoisonError::into_inner) + } + + /// Read-only snapshot of the shared ruleset list. + fn read_rulesets(&self) -> std::sync::RwLockReadGuard<'_, Vec> { self.rulesets - .retain(|existing| existing.layer != ruleset.layer); - self.rulesets.push(ruleset); - self.rulesets.sort_by_key(|existing| existing.layer); + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner) } /// Resolve the effective trusted/denied prefix sets by merging all rulesets. @@ -368,17 +396,19 @@ impl ExecPolicyEngine { /// semantics: any matching deny prefix blocks the command regardless of layer. /// Trusted rules are only consulted after deny checks pass. fn resolve_prefixes(&self) -> (Vec, Vec) { - if self.rulesets.is_empty() { + let rulesets = self.read_rulesets(); + if rulesets.is_empty() { return (self.trusted_prefixes.clone(), self.denied_prefixes.clone()); } // Collect all trusted/denied across all layers, highest-priority last so they // shadow lower-priority entries with the same prefix. let mut trusted: Vec = vec![]; let mut denied: Vec = vec![]; - for rs in &self.rulesets { + for rs in rulesets.iter() { trusted.extend(rs.trusted_prefixes.iter().cloned()); denied.extend(rs.denied_prefixes.iter().cloned()); } + drop(rulesets); // Also merge legacy flat lists as user-layer. trusted.extend(self.trusted_prefixes.iter().cloned()); denied.extend(self.denied_prefixes.iter().cloned()); @@ -391,7 +421,8 @@ impl ExecPolicyEngine { .path .and_then(|path| normalize_workspace_relative_path(path, ctx.cwd)); - self.rulesets + let rulesets = self.read_rulesets(); + let matched = rulesets .iter() .flat_map(|ruleset| { ruleset @@ -422,7 +453,9 @@ impl ExecPolicyEngine { (None, _) => true, }) .max_by_key(|(layer, rule)| (*layer, rule.action, ask_rule_specificity(rule))) - .map(|(_, rule)| rule.clone()) + .map(|(_, rule)| (*rule).clone()); + drop(rulesets); + matched } /// Records an approval key for the current session so subsequent checks skip approval. From 17cf60b63682eacef857ead152ea743018ffd48a Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 1 Sep 2026 16:22:01 +0800 Subject: [PATCH 5/9] feat(subagent): enforce execpolicy on tool calls Nested sub-agent tool execution never consulted the session's exec-policy engine, so a command hard-denied on the main line could be delegated to a child and run anyway - a real escape hatch under parent auto-approve. SubAgentRuntime now carries the parent session's ExecPolicyEngine (with_exec_policy_engine) from every production construction site: the per-turn tool-registry runtime, the SpawnSubAgent background path, and the direct Workflow runtime. SubAgentToolRegistry stores the handle and, in execute, after the execution-envelope gate, evaluates the same typed execpolicy decision the parent turn loop makes by reusing exec_shell_ask_rule_decision / file_tool_ask_rule_decision through new pub(crate) _for_engine variants keyed on a bare engine handle. Block refuses with the main line's reason wording; Prompt, Allow, and no-rule pass through, because children have no prompt surface - the parent posture is evaluated as ApprovalMode::Auto so a typed ask rule degrades to pass-through, never to the Never-mode fail-closed block. An empty engine (the default) makes the gate a no-op, keeping behavior identical for embedders that never thread a ruleset. The handle shares the engine's live rulesets across clones (previous commit), so rules installed mid-session bind already-delegated calls instead of freezing a spawn-time snapshot. Tests cover deny/allow/empty-engine/prompt/file-path semantics and ruleset-update liveness, including forkguard_subagent_execpolicy_deny_matches_main_line. Signed-off-by: asto --- crates/tui/src/core/engine.rs | 61 +++++++- crates/tui/src/lib.rs | 6 +- crates/tui/src/tools/subagent/mod.rs | 61 ++++++++ crates/tui/src/tools/subagent/tests.rs | 196 +++++++++++++++++++++++++ 4 files changed, 315 insertions(+), 9 deletions(-) diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 6fd398d6d7..e7a3cd3aca 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -2607,6 +2607,10 @@ impl Engine { .with_speech_output_dir(self.config.speech_output_dir.clone()) .with_mcp_pool(mcp_pool) .with_parent_mode(self.current_mode) + // Typed permission rules must bind delegated calls like + // they bind the parent's own; the handle shares the + // live rulesets, so mid-session updates stay effective. + .with_exec_policy_engine(self.config.exec_policy_engine.clone()) // #4810: no `with_todos` here — this runtime *is* the // spawned background agent, and `background_runtime()` // gives it its own list. Binding the session list would @@ -4317,7 +4321,11 @@ impl Engine { .with_todos(self.config.todos.clone()) .with_parent_completion_tx(self.tx_subagent_completion.clone()) .with_runtime_cost_owner(self.config.compaction.runtime_cost_owner.as_deref()) - .with_parent_mode(input_policy.mode); + .with_parent_mode(input_policy.mode) + // Typed permission rules must bind delegated calls like they + // bind the parent's own; the handle shares the live rulesets, + // so mid-session updates stay effective. + .with_exec_policy_engine(self.config.exec_policy_engine.clone()); if matches!(input_policy.mode, AppMode::Plan) { rt.worker_profile = WorkerRuntimeProfile::for_role(FleetRole::Planner); } @@ -6131,7 +6139,7 @@ fn goal_objective_for_prompt( // outside messages[0]. #[derive(Debug, Clone, PartialEq, Eq)] -pub(super) enum ToolAskRuleDecision { +pub(crate) enum ToolAskRuleDecision { Allow, Prompt(String), Block(String), @@ -6244,6 +6252,26 @@ pub(super) fn exec_shell_ask_rule_decision( tool_input: &Value, workspace: &Path, approval_mode: crate::tui::approval::ApprovalMode, +) -> Option { + exec_shell_ask_rule_decision_for_engine( + &config.exec_policy_engine, + tool_name, + tool_input, + workspace, + approval_mode, + ) +} + +/// [`exec_shell_ask_rule_decision`] keyed on a bare [`ExecPolicyEngine`] +/// handle instead of the full [`EngineConfig`], so executors that hold only +/// the session engine (the nested sub-agent tool registry) evaluate exactly +/// the same typed exec-rule decision the main-line turn loop would. +pub(crate) fn exec_shell_ask_rule_decision_for_engine( + exec_policy_engine: &codewhale_execpolicy::ExecPolicyEngine, + tool_name: &str, + tool_input: &Value, + workspace: &Path, + approval_mode: crate::tui::approval::ApprovalMode, ) -> Option { let policy_tool_name = crate::tools::canonical_action::canonical_action_alias(tool_name, tool_input); @@ -6252,7 +6280,7 @@ pub(super) fn exec_shell_ask_rule_decision( } let command = tool_input.get("command").and_then(Value::as_str)?; tool_ask_rule_decision_for_context( - config, + exec_policy_engine, policy_tool_name, command, None, @@ -6267,13 +6295,31 @@ pub(super) fn file_tool_ask_rule_decision( tool_input: &Value, workspace: &Path, approval_mode: crate::tui::approval::ApprovalMode, +) -> Option { + file_tool_ask_rule_decision_for_engine( + &config.exec_policy_engine, + tool_name, + tool_input, + workspace, + approval_mode, + ) +} + +/// [`file_tool_ask_rule_decision`] keyed on a bare [`ExecPolicyEngine`] +/// handle; see [`exec_shell_ask_rule_decision_for_engine`]. +pub(crate) fn file_tool_ask_rule_decision_for_engine( + exec_policy_engine: &codewhale_execpolicy::ExecPolicyEngine, + tool_name: &str, + tool_input: &Value, + workspace: &Path, + approval_mode: crate::tui::approval::ApprovalMode, ) -> Option { let policy_tool_name = crate::tools::canonical_action::canonical_action_alias(tool_name, tool_input); let paths = file_tool_permission_paths(policy_tool_name, tool_input)?; if paths.is_empty() { return tool_ask_rule_decision_for_context( - config, + exec_policy_engine, policy_tool_name, "", None, @@ -6286,7 +6332,7 @@ pub(super) fn file_tool_ask_rule_decision( let mut all_allowed = true; for path in paths { match tool_ask_rule_decision_for_context( - config, + exec_policy_engine, policy_tool_name, "", Some(&path), @@ -6314,7 +6360,7 @@ pub(super) fn file_tool_ask_rule_decision( } fn tool_ask_rule_decision_for_context( - config: &EngineConfig, + exec_policy_engine: &codewhale_execpolicy::ExecPolicyEngine, tool_name: &str, command: &str, path: Option<&str>, @@ -6328,8 +6374,7 @@ fn tool_ask_rule_decision_for_context( | crate::tui::approval::ApprovalMode::Bypass | crate::tui::approval::ApprovalMode::Suggest => AskForApproval::OnFailure, }; - let decision = config - .exec_policy_engine + let decision = exec_policy_engine .check(ExecPolicyContext { command, cwd: cwd.as_ref(), diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs index 73e83b19c9..82b4414309 100644 --- a/crates/tui/src/lib.rs +++ b/crates/tui/src/lib.rs @@ -10510,7 +10510,11 @@ async fn build_direct_workflow_tool( .with_speech_output_dir(config.speech_output_dir()) .with_mcp_pool(mcp_pool) .with_todos(new_shared_todo_list()) - .with_parent_mode(mode); + .with_parent_mode(mode) + // Typed permission rules must bind delegated calls like they bind the + // parent's own; the handle shares the live rulesets, so mid-session + // updates stay effective. + .with_exec_policy_engine(config.exec_policy_engine.clone()); Ok(( crate::tools::workflow::WorkflowTool::new(manager, runtime).with_explicit_cli_approval(), diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index ec16708819..0055a4d93b 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -2217,6 +2217,13 @@ pub struct SubAgentRuntime { pub todos: SharedTodoList, /// Session mode of the orchestrating parent at spawn time (Wave 7 M4/M5). pub parent_mode: AppMode, + /// The parent session's exec-policy engine. Because the engine shares its + /// live rulesets across clones, every child registry holding this handle + /// evaluates the same typed permission rules — including rules installed + /// mid-session — that the parent turn loop enforces on its own tool calls. + /// Defaults to an empty engine (no rules), which leaves child behavior + /// unchanged for embedders that never thread one. + pub exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine, } impl SubAgentRuntime { @@ -2268,6 +2275,7 @@ impl SubAgentRuntime { speech_output_dir: None, todos: crate::tools::todo::new_shared_todo_list(), parent_mode: AppMode::Agent, + exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine::new(Vec::new(), Vec::new()), } } @@ -2278,6 +2286,20 @@ impl SubAgentRuntime { self } + /// Carry the parent session's exec-policy engine into child registries so + /// typed deny rules bind sub-agent tool calls the same way they bind the + /// parent's. The engine shares its live rulesets across clones, so this + /// handle tracks later `set_ruleset` updates instead of freezing a + /// spawn-time snapshot. + #[must_use] + pub fn with_exec_policy_engine( + mut self, + exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine, + ) -> Self { + self.exec_policy_engine = exec_policy_engine; + self + } + /// Match generated worker display names to the active session language. #[must_use] pub fn with_locale_tag(mut self, locale_tag: impl Into) -> Self { @@ -2555,6 +2577,7 @@ impl SubAgentRuntime { // opt-in forked child as immutable `fork_context` text. todos: crate::tools::todo::new_shared_todo_list(), parent_mode: self.parent_mode, + exec_policy_engine: self.exec_policy_engine.clone(), } } @@ -12704,6 +12727,11 @@ struct SubAgentToolRegistry { /// admitted worker identity. Production registries always enforce claims. enforce_write_claim: bool, registry: ToolRegistry, + /// The parent session's exec-policy engine (shared live rulesets; see + /// [`SubAgentRuntime::exec_policy_engine`]). Consulted by `execute` so a + /// command the parent turn loop would hard-deny cannot be delegated to + /// this child and run anyway. Empty by default → checks are no-ops. + exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine, } impl SubAgentToolRegistry { @@ -12809,6 +12837,7 @@ impl SubAgentToolRegistry { coordination_manager, enforce_write_claim: true, registry, + exec_policy_engine: runtime.exec_policy_engine.clone(), } } @@ -13319,6 +13348,38 @@ impl SubAgentToolRegistry { ) .map_err(|refusal| anyhow!(refusal))?; } + // Fork delta (execpolicy wiring): delegated calls run through the same + // typed execpolicy gate the parent turn loop applies between hooks and + // approval (`exec_shell_ask_rule_decision` / `file_tool_ask_rule_decision`, + // reused verbatim). Positioned after the execution envelope so this + // child's own posture still speaks first. Only a hard Block refuses — + // children have no prompt surface, so a Prompt decision passes like any + // other parent-auto-approved call; that is also why the parent posture + // here is evaluated as `ApprovalMode::Auto` (→ `OnFailure`), never the + // fail-closed `Never` mapping. The engine handle shares the parent's + // live rulesets, so a rule installed mid-session binds delegated calls + // too, and an empty engine (no rules) leaves this check a no-op. + let ask_rule_decision = crate::core::engine::exec_shell_ask_rule_decision_for_engine( + &self.exec_policy_engine, + name, + &input, + &self.registry.context().workspace, + crate::tui::approval::ApprovalMode::Auto, + ) + .or_else(|| { + crate::core::engine::file_tool_ask_rule_decision_for_engine( + &self.exec_policy_engine, + name, + &input, + &self.registry.context().workspace, + crate::tui::approval::ApprovalMode::Auto, + ) + }); + if let Some(crate::core::engine::ToolAskRuleDecision::Block(reason)) = ask_rule_decision { + // Mirror the main line's blocked refusal so a child model sees the + // same familiar wording the parent would have received. + return Err(anyhow!(reason)); + } let scope_aware_write = matches!( name, "write_file" | "edit_file" | "apply_patch" | "fim_edit" diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index f2c45cfb36..a14fcdb9f8 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -6306,6 +6306,201 @@ async fn scout_shell_respects_parent_shell_and_network_ceilings() { } } +/// An auto-approved Worker registry (shell-capable, everything else default) +/// carrying the given exec-policy engine — the smallest fixture that reaches +/// the delegated-call execpolicy gate past every posture gate. +fn auto_approved_worker_registry_with_engine( + tmp: &tempfile::TempDir, + exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine, +) -> SubAgentToolRegistry { + let mut runtime = + stub_runtime().with_agent_tool_surface_options(enabled_agent_surface_options()); + runtime.context = ToolContext::new(tmp.path().to_path_buf()); + runtime.context.auto_approve = true; + runtime.allow_shell = true; + runtime.worker_profile = WorkerRuntimeProfile::for_role(FleetRole::Worker); + SubAgentToolRegistry::new( + runtime.with_exec_policy_engine(exec_policy_engine), + FleetRole::Worker, + None, + crate::tools::todo::new_shared_todo_list(), + crate::tools::plan::new_shared_plan_state(), + ) +} + +/// Fork-delta behavior test: a command the parent session's execpolicy +/// hard-denies must fail closed inside the child with the main line's deny +/// wording — the auto-approve escape hatch this wiring closes — while a +/// command no rule names still dispatches for real through the same registry. +#[tokio::test] +async fn forkguard_subagent_execpolicy_deny_matches_main_line() { + let tmp = tempdir().expect("tempdir"); + let engine = codewhale_execpolicy::ExecPolicyEngine::with_rulesets(vec![ + codewhale_execpolicy::Ruleset::user(Vec::new(), vec!["sudo".to_string()]), + ]); + let registry = auto_approved_worker_registry_with_engine(&tmp, engine); + + let error = registry + .execute( + "agent_policy", + "Bash", + json!({"action": "run", "command": "sudo --version"}), + ) + .await + .expect_err("a main-line deny rule must block delegated calls too") + .to_string(); + assert!( + error.contains("denied prefix rule 'sudo'"), + "child must see the main-line deny reason: {error}" + ); + + let output = registry + .execute( + "agent_policy", + "Bash", + json!({"action": "run", "command": "ls"}), + ) + .await + .expect("an allowed command must pass the execpolicy gate untouched"); + assert!( + !output.starts_with("Error:"), + "allowed command must really dispatch: {output}" + ); +} + +/// Without a ruleset the delegated path is byte-identical to the unwired +/// behavior: the gate is a no-op, and the very same call that a deny rule +/// blocks runs when the rule is absent. +#[tokio::test] +async fn subagent_execpolicy_empty_ruleset_keeps_auto_approve_behavior() { + let tmp = tempdir().expect("tempdir"); + let registry = auto_approved_worker_registry_with_engine( + &tmp, + codewhale_execpolicy::ExecPolicyEngine::new(Vec::new(), Vec::new()), + ); + let output = registry + .execute( + "agent_policy", + "Bash", + json!({"action": "run", "command": "echo execpolicy_unblocked"}), + ) + .await + .expect("an empty engine must leave the delegated call unchanged"); + assert!(output.contains("execpolicy_unblocked"), "{output}"); + + let denied_registry = auto_approved_worker_registry_with_engine( + &tmp, + codewhale_execpolicy::ExecPolicyEngine::with_rulesets(vec![ + codewhale_execpolicy::Ruleset::user(Vec::new(), vec!["echo".to_string()]), + ]), + ); + let error = denied_registry + .execute( + "agent_policy_denied", + "Bash", + json!({"action": "run", "command": "echo execpolicy_unblocked"}), + ) + .await + .expect_err("the deny rule, not the wiring, must be what blocks") + .to_string(); + assert!(error.contains("denied prefix rule 'echo'"), "{error}"); +} + +/// A typed Deny rule on a file path blocks the child's File read of that path +/// while a workspace-relative read outside the rule still lands. +#[tokio::test] +async fn subagent_execpolicy_blocks_denied_file_read_but_allows_workspace_relative_read() { + let tmp = tempdir().expect("tempdir"); + std::fs::create_dir_all(tmp.path().join("secret")).expect("secret dir"); + std::fs::write(tmp.path().join("secret/key.txt"), "rot thirteen").expect("key fixture"); + std::fs::write(tmp.path().join("notes.txt"), "plain notes").expect("notes fixture"); + let mut rule = codewhale_execpolicy::ToolAskRule::file_path("read_file", "secret/key.txt"); + rule.action = codewhale_execpolicy::PermissionAction::Deny; + let engine = codewhale_execpolicy::ExecPolicyEngine::with_rulesets(vec![ + codewhale_execpolicy::Ruleset::user(Vec::new(), Vec::new()).with_ask_rules(vec![rule]), + ]); + let registry = auto_approved_worker_registry_with_engine(&tmp, engine); + + let error = registry + .execute( + "agent_policy", + "File", + json!({"action": "read", "path": "secret/key.txt"}), + ) + .await + .expect_err("a denied path must not be readable through the child") + .to_string(); + assert!(error.contains("explicitly denies"), "{error}"); + + let output = registry + .execute( + "agent_policy", + "File", + json!({"action": "read", "path": "notes.txt"}), + ) + .await + .expect("a workspace-relative read outside the deny rule passes"); + assert!(output.contains("plain notes"), "{output}"); +} + +/// Typed ask rules are prompt decisions on the main line. A child has no +/// prompt surface, so the decision must degrade to pass-through — the call +/// runs — never to a refusal. +#[tokio::test] +async fn subagent_execpolicy_prompt_decision_does_not_block_delegated_calls() { + let tmp = tempdir().expect("tempdir"); + let engine = codewhale_execpolicy::ExecPolicyEngine::with_rulesets(vec![ + codewhale_execpolicy::Ruleset::user(Vec::new(), Vec::new()).with_ask_rules(vec![ + codewhale_execpolicy::ToolAskRule::exec_shell("echo askme"), + ]), + ]); + let registry = auto_approved_worker_registry_with_engine(&tmp, engine); + + let output = registry + .execute( + "agent_policy", + "Bash", + json!({"action": "run", "command": "echo askme"}), + ) + .await + .expect("an ask-rule (prompt) decision must not block a child"); + assert!(output.contains("askme"), "{output}"); +} + +/// The registry's engine handle is the parent's live engine, not a snapshot: +/// a ruleset installed after the registry was built binds its delegated calls. +#[tokio::test] +async fn subagent_execpolicy_ruleset_update_binds_already_delegated_calls() { + let tmp = tempdir().expect("tempdir"); + let mut engine = codewhale_execpolicy::ExecPolicyEngine::new(Vec::new(), Vec::new()); + let registry = auto_approved_worker_registry_with_engine(&tmp, engine.clone()); + + let output = registry + .execute( + "agent_policy", + "Bash", + json!({"action": "run", "command": "echo not_yet_denied"}), + ) + .await + .expect("no rule exists yet, so the delegated call runs"); + assert!(output.contains("not_yet_denied"), "{output}"); + + engine.set_ruleset(codewhale_execpolicy::Ruleset::user( + Vec::new(), + vec!["echo".to_string()], + )); + let error = registry + .execute( + "agent_policy", + "Bash", + json!({"action": "run", "command": "echo not_yet_denied"}), + ) + .await + .expect_err("a mid-session rule must reach the already-built registry") + .to_string(); + assert!(error.contains("denied prefix rule 'echo'"), "{error}"); +} + #[test] fn implementer_catalog_inherits_patch_and_fim_when_enabled() { let tmp = tempdir().expect("tempdir"); @@ -10761,6 +10956,7 @@ pub(crate) fn stub_runtime() -> SubAgentRuntime { tool_timeout: DEFAULT_TOOL_TIMEOUT, speech_output_dir: None, todos: crate::tools::todo::new_shared_todo_list(), + exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine::new(Vec::new(), Vec::new()), } } From 817d46a3517c5fb69923312f8125144a8183b9ec Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 1 Sep 2026 16:22:14 +0800 Subject: [PATCH 6/9] style(execpolicy): wrap lock guard expression rustfmt line-wrap only, no behavior change. Signed-off-by: asto --- crates/execpolicy/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs index 5b696fa0a6..a6ddf737f3 100644 --- a/crates/execpolicy/src/lib.rs +++ b/crates/execpolicy/src/lib.rs @@ -379,7 +379,9 @@ impl ExecPolicyEngine { fn lock_rulesets( rulesets: &Arc>>, ) -> std::sync::RwLockWriteGuard<'_, Vec> { - rulesets.write().unwrap_or_else(std::sync::PoisonError::into_inner) + rulesets + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner) } /// Read-only snapshot of the shared ruleset list. From 5313d018999f801e1c0fe8f4ece6ccdd70b46532 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 1 Sep 2026 16:37:16 +0800 Subject: [PATCH 7/9] feat(execpolicy): match absolute path rules exactly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typed path rules previously only matched after workspace-relative normalization, which requires the call to live inside the workspace — a rule pinning an absolute location (a real home, /root, a Windows profile, or a literal ~ spelling passed through unexpanded) could never match, leaving home-absolute File-tool reads unmatchable. Add a rooted-rule-only exact-match fallback: separators fold to '/', case folds on case-insensitive platforms, and a relative rule keeps its workspace-relative semantics untouched. No wildcards, so the deny direction keeps its precision. Signed-off-by: asto --- crates/execpolicy/src/lib.rs | 178 +++++++++++++++++++++++++++++++++-- 1 file changed, 171 insertions(+), 7 deletions(-) diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs index a6ddf737f3..a659fcf3b9 100644 --- a/crates/execpolicy/src/lib.rs +++ b/crates/execpolicy/src/lib.rs @@ -444,13 +444,24 @@ impl ExecPolicyEngine { None => true, }) .filter(|(_, rule)| match (rule.path.as_deref(), ctx.path) { - (Some(pattern), Some(_)) => match ( - normalize_workspace_relative_path(pattern, ctx.cwd), - normalized_path.as_deref(), - ) { - (Some(pattern), Some(path)) => pattern == path, - _ => false, - }, + (Some(pattern), Some(call_path)) => { + let ws_rule = normalize_workspace_relative_path(pattern, ctx.cwd); + match (ws_rule, normalized_path.as_deref()) { + // Workspace-relative normalization fails for a call + // outside the workspace or a rule that names one, and + // on a POSIX host a Windows-spelled rule/call pair + // parses as unrelated relative forms. A rule spelling + // an ABSOLUTE path must still be able to match such a + // call exactly, or pinned locations (a real home, + // `/root`, a Windows profile) are unmatchable. The + // helper only fires for rooted rules, so relative + // semantics are unchanged. + (Some(ws_rule), Some(ws_call)) => { + ws_rule == ws_call || absolute_path_rule_matches(pattern, call_path) + } + _ => absolute_path_rule_matches(pattern, call_path), + } + } (Some(_), None) => false, (None, _) => true, }) @@ -1117,6 +1128,33 @@ fn is_windows_absolute_path(value: &str) -> bool { bytes.len() >= 3 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' && bytes[2] == b'/' } +/// Exact-match fallback for a typed path rule that names an ABSOLUTE path. +/// +/// The primary match normalizes both sides to workspace-relative form, which +/// only succeeds when the call lives inside the workspace — so a rule pinning +/// a location outside it (a real home, `/root`, another user's home, or a +/// literal `~` spelling the tool passed through unexpanded) could never match. +/// This fallback fires only when workspace normalization failed on either +/// side, and only for a ROOTED rule (leading `/`, `~`, or a Windows drive): +/// separators fold to `/`, case folds on case-insensitive platforms, and the +/// comparison is plain equality. A relative rule never reaches it, so +/// workspace-relative semantics are unchanged, and because there are no +/// wildcards the deny direction keeps its precision while the allow direction +/// can only ever match the exact path the rule spells. +fn absolute_path_rule_matches(rule_path: &str, call_path: &str) -> bool { + let fold = |value: &str| { + let value = value.trim().replace('\\', "/"); + if platform_paths_are_case_insensitive() { + value.to_ascii_lowercase() + } else { + value + } + }; + let rule = fold(rule_path); + let rooted = rule.starts_with('/') || rule.starts_with("~/") || is_windows_absolute_path(&rule); + rooted && rule == fold(call_path) +} + fn has_windows_drive_prefix(value: &str) -> bool { let bytes = value.as_bytes(); bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' @@ -2207,6 +2245,132 @@ mod tests { assert!(decision.requires_approval); } + #[test] + fn typed_ask_absolute_path_rule_matches_absolute_call_outside_workspace() { + let engine = + ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_ask_rules( + vec![ToolAskRule { + tool: "read_file".into(), + command: None, + command_exact: false, + path: Some("/root/.ssh/config".into()), + workspace: None, + action: PermissionAction::Deny, + }], + )]); + + // An absolute rule must reach a call outside the workspace that the + // workspace-relative normalization cannot express. + let decision = engine + .check(ExecPolicyContext { + command: "", + cwd: "/workspace", + tool: Some("read_file"), + path: Some("/root/.ssh/config"), + ask_for_approval: AskForApproval::OnFailure, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + assert_eq!(decision.matched_action, Some(PermissionAction::Deny)); + + // A different absolute path must not match. + let decision = engine + .check(ExecPolicyContext { + command: "", + cwd: "/workspace", + tool: Some("read_file"), + path: Some("/root/.ssh/known_hosts"), + ask_for_approval: AskForApproval::OnFailure, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + assert_eq!(decision.matched_rule, None); + } + + #[test] + fn typed_ask_literal_tilde_rule_matches_unexpanded_call_spelling() { + let engine = + ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_ask_rules( + vec![ToolAskRule { + tool: "read_file".into(), + command: None, + command_exact: false, + path: Some("~/.ssh/config".into()), + workspace: None, + action: PermissionAction::Deny, + }], + )]); + + let decision = engine + .check(ExecPolicyContext { + command: "", + cwd: "/workspace", + tool: Some("read_file"), + path: Some("~/.ssh/config"), + ask_for_approval: AskForApproval::OnFailure, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + assert_eq!(decision.matched_action, Some(PermissionAction::Deny)); + } + + #[test] + fn typed_ask_relative_path_rule_still_rejects_absolute_call() { + // The absolute fallback is rooted-rule-only: a relative rule keeps + // its workspace-relative semantics and must not reach an absolute + // call path through it. + let engine = ExecPolicyEngine::with_rulesets(vec![ + Ruleset::user(vec![], vec![]) + .with_ask_rules(vec![ToolAskRule::file_path("edit_file", "src/a.rs")]), + ]); + + let decision = engine + .check(ExecPolicyContext { + command: "", + cwd: "/workspace", + tool: Some("edit_file"), + path: Some("/src/a.rs"), + ask_for_approval: AskForApproval::OnFailure, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + assert_eq!(decision.matched_rule, None); + } + + #[test] + fn typed_ask_absolute_path_rule_folds_separators_and_case_on_windows() { + let engine = + ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_ask_rules( + vec![ToolAskRule { + tool: "read_file".into(), + command: None, + command_exact: false, + path: Some("C:/Users/u/.aws/credentials".into()), + workspace: None, + action: PermissionAction::Deny, + }], + )]); + + let decision = engine + .check(ExecPolicyContext { + command: "", + cwd: r"C:\workspace", + tool: Some("read_file"), + path: Some(r"C:\Users\U\.AWS\credentials"), + ask_for_approval: AskForApproval::OnFailure, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + // The rule folds `C:/Users/u/...` and the call folds `C:\Users\U\...` + // to the same form on a case-insensitive platform; on a + // case-sensitive one the case difference is a different file. + if platform_paths_are_case_insensitive() { + assert_eq!(decision.matched_action, Some(PermissionAction::Deny)); + } else { + assert_eq!(decision.matched_rule, None); + } + } + // ── deny / allow action tests ────────────────────────────────────────── #[test] From aa0bf1e4d2990fe25d0030804fb390c783e4c510 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 2 Sep 2026 00:08:31 +0800 Subject: [PATCH 8/9] fix(subagent): refuse approval-gated calls in children A delegated call matching a typed Ask rule used to pass unconditionally (children were treated like a parent-auto-approved call). Mirror the main line's #3790 posture authority instead: under parent auto-approve the ask rule auto-runs and still passes; under every prompting posture and the fail-closed Never session the child now refuses, because it has no surface to show the approval prompt the rule demands. The engine still maps to OnFailure here, so the Never refusal wording differs from the main line's Forbidden wording, but the refusal outcome matches. The deny face is unchanged: hard Blocks refuse exactly as before. The forkguard behavior test now covers both ask-rule faces (refusal under a prompting parent, pass under auto-approve) alongside the existing deny and allow cases. --- crates/tui/src/tools/subagent/mod.rs | 45 ++++++++++++++----- crates/tui/src/tools/subagent/tests.rs | 60 +++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 12 deletions(-) diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 0055a4d93b..b08ab6b0dc 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -13352,13 +13352,20 @@ impl SubAgentToolRegistry { // typed execpolicy gate the parent turn loop applies between hooks and // approval (`exec_shell_ask_rule_decision` / `file_tool_ask_rule_decision`, // reused verbatim). Positioned after the execution envelope so this - // child's own posture still speaks first. Only a hard Block refuses — - // children have no prompt surface, so a Prompt decision passes like any - // other parent-auto-approved call; that is also why the parent posture - // here is evaluated as `ApprovalMode::Auto` (→ `OnFailure`), never the - // fail-closed `Never` mapping. The engine handle shares the parent's - // live rulesets, so a rule installed mid-session binds delegated calls - // too, and an empty engine (no rules) leaves this check a no-op. + // child's own posture still speaks first. A hard Block always refuses. + // A Prompt decision follows the main line's #3790 rule — the approval + // posture is the authority: when the inherited session auto-approves + // (YOLO), the main line would auto-run, so the call passes; otherwise + // the main line would surface an approval prompt, which a child has + // no surface to do, so refusing there is the fail-closed answer for + // every prompting posture and for the fail-closed `Never` session + // alike (the Never wording differs, since the engine still maps to + // `OnFailure` here, but the refusal outcome matches). All non-Never + // modes map to `OnFailure`, so `ApprovalMode::Auto` is + // decision-equivalent to the parent's mode whenever auto-approve is + // on. The engine handle shares the parent's live rulesets, so a rule + // installed mid-session binds delegated calls too, and an empty + // engine (no rules) leaves this check a no-op. let ask_rule_decision = crate::core::engine::exec_shell_ask_rule_decision_for_engine( &self.exec_policy_engine, name, @@ -13375,10 +13382,26 @@ impl SubAgentToolRegistry { crate::tui::approval::ApprovalMode::Auto, ) }); - if let Some(crate::core::engine::ToolAskRuleDecision::Block(reason)) = ask_rule_decision { - // Mirror the main line's blocked refusal so a child model sees the - // same familiar wording the parent would have received. - return Err(anyhow!(reason)); + match ask_rule_decision { + Some(crate::core::engine::ToolAskRuleDecision::Block(reason)) => { + // Mirror the main line's blocked refusal so a child model sees + // the same familiar wording the parent would have received. + return Err(anyhow!(reason)); + } + // The child cannot show the approval prompt the main line would + // show for this rule, so without parent auto-approve the only + // correct answer is to refuse and point the model at the main + // conversation. + Some(crate::core::engine::ToolAskRuleDecision::Prompt(reason)) + if !self.auto_approve => + { + return Err(anyhow!(format!( + "Delegated tool call requires approval: {reason}. Sub-agents cannot show an approval prompt; run this tool call in the main conversation so it can be approved." + ))); + } + // `Allow` is a user-authored allow rule, and a Prompt under parent + // auto-approve matches the main line's auto-run: both pass. + _ => {} } let scope_aware_write = matches!( name, diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index a14fcdb9f8..ae18b5582d 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -6312,11 +6312,23 @@ async fn scout_shell_respects_parent_shell_and_network_ceilings() { fn auto_approved_worker_registry_with_engine( tmp: &tempfile::TempDir, exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine, +) -> SubAgentToolRegistry { + worker_registry_with_engine_and_approval(tmp, exec_policy_engine, true) +} + +/// The same fixture with the parent session's approval posture under the +/// caller's control: `false` models every prompting/fail-closed posture +/// (Suggest/Auto/Never), where the main line would surface an approval +/// prompt the child has no surface for. +fn worker_registry_with_engine_and_approval( + tmp: &tempfile::TempDir, + exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine, + auto_approve: bool, ) -> SubAgentToolRegistry { let mut runtime = stub_runtime().with_agent_tool_surface_options(enabled_agent_surface_options()); runtime.context = ToolContext::new(tmp.path().to_path_buf()); - runtime.context.auto_approve = true; + runtime.context.auto_approve = auto_approve; runtime.allow_shell = true; runtime.worker_profile = WorkerRuntimeProfile::for_role(FleetRole::Worker); SubAgentToolRegistry::new( @@ -6332,6 +6344,10 @@ fn auto_approved_worker_registry_with_engine( /// hard-denies must fail closed inside the child with the main line's deny /// wording — the auto-approve escape hatch this wiring closes — while a /// command no rule names still dispatches for real through the same registry. +/// A typed ask rule mirrors the main line's #3790 posture authority: under a +/// prompting/fail-closed parent the child refuses (it cannot show the +/// approval prompt), while under an auto-approving parent it passes like the +/// main line's auto-run. #[tokio::test] async fn forkguard_subagent_execpolicy_deny_matches_main_line() { let tmp = tempdir().expect("tempdir"); @@ -6366,6 +6382,48 @@ async fn forkguard_subagent_execpolicy_deny_matches_main_line() { !output.starts_with("Error:"), "allowed command must really dispatch: {output}" ); + + // Ask-rule face: a prompting/fail-closed parent session cannot let the + // child run what the main line would only run after approval. + let ask_engine = codewhale_execpolicy::ExecPolicyEngine::with_rulesets(vec![ + codewhale_execpolicy::Ruleset::user(Vec::new(), Vec::new()) + .with_ask_rules(vec![codewhale_execpolicy::ToolAskRule::exec_shell("curl")]), + ]); + let prompting = worker_registry_with_engine_and_approval(&tmp, ask_engine.clone(), false); + let error = prompting + .execute( + "agent_policy", + "Bash", + json!({"action": "run", "command": "curl --version"}), + ) + .await + .expect_err("an ask rule under a non-auto parent must refuse in the child") + .to_string(); + assert!( + error.contains("requires approval"), + "refusal must point at the approval surface: {error}" + ); + + let output = prompting + .execute( + "agent_policy", + "Bash", + json!({"action": "run", "command": "ls"}), + ) + .await + .expect("a command no rule names must dispatch regardless of posture"); + assert!(!output.starts_with("Error:"), "{output}"); + + let yolo = worker_registry_with_engine_and_approval(&tmp, ask_engine, true); + let output = yolo + .execute( + "agent_policy", + "Bash", + json!({"action": "run", "command": "curl --version"}), + ) + .await + .expect("under parent auto-approve the ask rule must pass like the main line"); + assert!(!output.starts_with("Error:"), "{output}"); } /// Without a ruleset the delegated path is byte-identical to the unwired From aaae5133bdcac60bbefc141e7030f1022c681f67 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 2 Sep 2026 00:08:31 +0800 Subject: [PATCH 9/9] test(execpolicy): pin absolute fallback exact matching Pin the never-matchable-traversal stance through the rooted absolute fallback and the tilde-rooted channel: traversal spellings of an exact rule path must stay unmatchable. Also document that approved_for_session is deliberately clone-private (a remembered grant must not authorize delegated calls in cloned executors). --- crates/execpolicy/src/lib.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs index a659fcf3b9..0f7a40b3b4 100644 --- a/crates/execpolicy/src/lib.rs +++ b/crates/execpolicy/src/lib.rs @@ -324,6 +324,9 @@ pub struct ExecPolicyEngine { /// Legacy flat lists kept for backward compatibility with `new()`. trusted_prefixes: Vec, denied_prefixes: Vec, + /// Deliberately clone-private, unlike `rulesets`: a remembered grant is a + /// decision the parent session made for its own calls, so it must not + /// silently authorize a delegated call in a cloned executor. approved_for_session: HashSet, /// Arity dictionary for command-prefix allow-rule matching. arity_dict: BashArityDict, @@ -2285,6 +2288,22 @@ mod tests { }) .unwrap(); assert_eq!(decision.matched_rule, None); + + // The fallback is exact: a traversal spelling of the same file is a + // different token string and must stay unmatchable (the documented + // "traversal is never matchable" stance, pinned through the rooted + // fallback too). + let decision = engine + .check(ExecPolicyContext { + command: "", + cwd: "/workspace", + tool: Some("read_file"), + path: Some("/root/../root/.ssh/config"), + ask_for_approval: AskForApproval::OnFailure, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + assert_eq!(decision.matched_rule, None); } #[test] @@ -2312,6 +2331,20 @@ mod tests { }) .unwrap(); assert_eq!(decision.matched_action, Some(PermissionAction::Deny)); + + // The tilde-rooted channel is exact as well: a traversal spelling of + // the same file must not match (never-matchable-traversal stance). + let decision = engine + .check(ExecPolicyContext { + command: "", + cwd: "/workspace", + tool: Some("read_file"), + path: Some("~/.ssh/../ssh/config"), + ask_for_approval: AskForApproval::OnFailure, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + assert_eq!(decision.matched_rule, None); } #[test]