feat(execpolicy): phase-2 deny-matching expressiveness + subagent wiring - #37
feat(execpolicy): phase-2 deny-matching expressiveness + subagent wiring#37asto18089 wants to merge 9 commits into
Conversation
|
Thanks @asto18089 for taking the time to contribute. This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered. Please read |
|
Parent-side consumer PR: Pinvou/pinvou-agent#396 (phase-2 deny-face scope alignment — safety_deny_rules 17,971 → 24,061 rules consuming all of the new expressiveness, super-permission toggle serialization, gitlink → |
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 <asto18089@126.com>
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 <asto18089@126.com>
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 <asto18089@126.com>
The layered rulesets now live behind an Arc<RwLock<..>> 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 <asto18089@126.com>
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 <asto18089@126.com>
rustfmt line-wrap only, no behavior change. Signed-off-by: asto <asto18089@126.com>
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 <asto18089@126.com>
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 Hmbown#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.
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).
5de1bc1 to
aaae513
Compare
Audit reviewAudited as the foundation prerequisite of Pinvou/pinvou-agent#396 (parent-side gitlink = this head Verified (actually executed):
Nits (none blocking):
CI note: the |
Motivation
Generic capabilities required by the parent repo's phase-2 scope-alignment work (safety deny ruleset). Every change here is embedder-generic — no Pinvou-specific semantics enter the foundation.
Base
Rebased onto
pinjou3-clean@ r13 (f853f8f15, the GAIA-isolation baseline) so the parent repo's gitlink can advance without regressing #32; two review-round commits were added on top of the original seven.Changes (9 commits)
feat(execpolicy): skip cmd.exe single-letter slash flags—denied_prefix_matchesnow skips/xflags (single slash + exactly one ASCII letter, e.g./f /s /q) in any position/order, in addition to-flags. Multi-char tokens (/tmp,/etc) remain strictly positional: treating them as flags would letcp /tmp/new_key ~/.ssh/id_rsamatch acp ~/.ssh/id_rsa-shaped exfil rule and regress the documented write-into-sensitive allowance. Collapses canonical cmd.exe sequence enumeration at the ruleset layer.feat(execpolicy): add middle wildcard token to deny rules— a rule token*matches zero or more consecutive command tokens (trailing*degrades to prefix semantics). DFS branches(i, j+1)/(i+1, j)bounded by the existingseenset. This makes argument-position vectors expressible:grep * ~/.ssh/id_rsa,find * -name id_rsa,dd * of=<target>. Module docs state the engine is permissive and rule authors own false-positive discipline.feat(execpolicy): fold .exe suffix on deny command word— the anchor token folds one trailing.exe(cat.exematches rulecat); rules that themselves end in.exe(control.exe) keep requiring the suffix.feat(execpolicy): share live rulesets across clones—ExecPolicyEngine.rulesetsbecomesArc<RwLock<...>>soset_rulesetthrough any clone is observed by every clone. Chosen over changing thepub exec_policy_enginefield type (which would ripple into ~10 sites incl. cross-cratecodewhale-config) while guaranteeing the same liveness.feat(subagent): enforce execpolicy on tool calls— nested subagent tool calls now pass through the same execpolicy decision as the main line, evaluated after the execution-envelope gate, reusing the main-line helpers verbatim (exec_shell_ask_rule_decision_for_engine/file_tool_ask_rule_decision_for_engine).Block→ child-visible error with the main-line wording; aPromptdecision follows the main line's posture authority (Permissions are over-abstracted: make the MODE the single authority (rip out auto-review/Auto; defer to 0.8.67) Hmbown/Codewhale#3790): it passes under parent auto-approve (YOLO, where the main line would auto-run) and refuses under every prompting posture and the fail-closedNeversession, because a child has no surface to show the approval prompt the rule demands (theNeverrefusal wording differs from the main line'sForbiddenwording, the outcome matches);Allowpasses; an empty/default engine is a byte-identical no-op for every existing embedder. Closes the escape hatch where a main-line-denied or approval-gated call (e.g.cat ~/.ssh/id_rsa, or a typed ask rule) could simply be delegated to a subagent. The live-sharing change in (4) is what makes mid-session ruleset refreshes bind already-spawned children.fix(subagent): refuse approval-gated calls in children— implements the ask-face posture alignment above and extendsforkguard_subagent_execpolicy_deny_matches_main_linewith both ask-rule faces (refusal under a prompting parent, pass under auto-approve).test(execpolicy): pin absolute fallback exact matching— pins the never-matchable-traversal stance through the rooted absolute fallback and the tilde-rooted channel; documents thatapproved_for_sessionis deliberately clone-private (a remembered grant must not authorize delegated calls in cloned executors).feat(execpolicy): match absolute path rules exactly— typed path rules previously only matched after workspace-relative normalization, so a rule pinning an absolute location (a real home,/root, a Windows profile, a literal~spelling) could never match. Adds a rooted-rule-only exact fallback (separators fold to/, case folds on case-insensitive platforms); relative rules keep their semantics untouched. No wildcards — deny precision preserved.Tests
codewhale-execpolicy: 118 lib tests green (10 new: slash-flag skip + the multi-char POSIX-path regression guard, wildcard anchoring/zero-match/leading semantics,.exefolding positives and negatives, absolute-path matching incl. tilde literals, rooted-only fallback, traversal rejections).forkguard_58/58 (--test-threads=1, default features) on the rebased head; the feature-gated GAIA fork-guard gates pass via the parent repo's full./scripts/fork-guard.sh. At the pre-rebase base,codewhale-tui --libwas 10,152 passed with only pre-existing environmental failures (verified red on the pristine base too):tools::terminal_session(5),runtime_api::skill_lifecycle_uninstall_removes_installed_skill(1);runtime_threads(3) is a parallel-load flake that passes in isolation.forkguard_subagent_execpolicy_deny_matches_main_line, empty-ruleset auto-approve preservation, denied File read vs workspace-relative pass, prompt-decision pass-through, liveness of mid-session ruleset updates for delegated calls.forkguard_suite 58 green (--test-threads=1).cargo fmt(style_edition 2024) andcargo clippy --lib --no-depsclean for both crates.Upstream note
Items 1-3 and 6 are candidates for upstream contribution (generic token-channel expressiveness). Item 5 is consumed by the parent repo's fork theme T2 (tool compatibility & command execution safety); the parent PR will carry the
docs/fork-modifications.mdentry and fingerprint updates.