From eefd11fda7c3fa9281080ed3460ab2ea6edafb20 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 13 Oct 2026 21:15:59 -0700 Subject: [PATCH 1/8] Fail Closed on Words the Shell Rewrites Before Git Reads Them in the Write Guard The push and include pre-filters also read each command with its quotes and backslashes removed and git in any case, so a quote-split push word, a quote-split include key, and an uppercase GIT reach the checks. A new deny fails closed where a run-time word sits where a git command's subcommand goes, among a push's words, or among a commit's global options or arguments, and where a comment sits among a push's words. A directory option's value, a double-quoted commit option value, a commit path after --, and a redirection target are left alone. Name the cleanup subcommand beside reply in the managed pr_review.py permission rule's description. Closes #2959 Closes #2961 Closes #2968 Co-Authored-By: Claude Opus 5.5 --- host-setup/agent-safety/README.md | 8 +- host-setup/agent-safety/claude/README.md | 4 +- .../agent-safety/claude/gh-write-guard.py | 475 +++++++++++++++++- host-setup/agent-safety/claude/install.py | 5 +- 4 files changed, 476 insertions(+), 16 deletions(-) diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md index c9b5fb61d..9405e2c7b 100644 --- a/host-setup/agent-safety/README.md +++ b/host-setup/agent-safety/README.md @@ -80,7 +80,13 @@ text says, because the harm it covers was never in the text. `HEAD`/`@` refspec whose directory or branch may change before it runs. Such a push's local branch is checked under its whole name, such as `release/1.0`. It is also checked with its first segment dropped. A remote-tracking push target such as `origin/feature/x` is checked with its - remote dropped. The + remote dropped. A push or commit holding a word the shell rewrites or drops before git reads it + is denied too. That is a `$` or backtick word among a push's words, among a commit's global + options and arguments, or where any git command's subcommand goes, or a comment among a push's + words. A directory option's value, a commit option's double-quoted value, and a commit path + after `--` are read as they stand. The push and include checks also read each command with its + quotes and backslashes removed and `git` in any case, so a `p''ush` or an uppercase `GIT` + reaches them. The workaround is an explicit refspec run as its own command. Every other requirement here favors precision over recall throughout, denying only a positively-identified dangerous shape, since a hook that fails closed on an unrelated resolution failure blocks diff --git a/host-setup/agent-safety/claude/README.md b/host-setup/agent-safety/claude/README.md index bd8464d61..5372cefa9 100644 --- a/host-setup/agent-safety/claude/README.md +++ b/host-setup/agent-safety/claude/README.md @@ -12,7 +12,7 @@ installation and operational detail. Codex and opencode have no equivalent hook Into `~/.claude/` (or `%USERPROFILE%\.claude\` on Windows): -- **`hooks/gh-write-guard.py`**: a PreToolUse hook that denies four classes of dangerous action. First, the GitHub **write** footguns behind the cross-repo comment incident: a state-changing `gh` call whose output is discarded, a GraphQL mutation passing a **literal** node id instead of a `$variable`, and a `gh` write whose explicit target is under an owner other than the checkout origin's. Sibling repositories under the same owner are allowed, since the harm this guards is reaching a stranger's repository rather than working across one maintainer's own fleet, and a different owner is allowed only when the maintainer names it in `GH_WRITE_GUARD_ALLOW` (an `owner/repo` list, where `owner/*` grants a whole owner). That variable is read from the environment the session was launched with, which is the one channel an agent cannot set for itself: a hook runs as its own process, so an inline `VAR=x cmd` prefix or an `export` inside a Bash call never reaches it. Second, a **git operation that bypasses a repository protection**. The branch-rule cases (a direct push, a force-push, or a branch delete to a branch that requires a pull request, a force-push where history is protected, a branch delete where deletion is blocked) are judged per refspec against the branch's **live** rules, so a code-style `develop` is denied while a config-style `develop` is allowed with no per-repo configuration. A push to a protected-default branch fails closed when its rules cannot be determined, either because the API is unreachable or because the checkout's origin cannot be resolved to query them. A push whose target the hook cannot read, such as one in text it does not parse, one carrying a `-c` config override, a git command an `include.path`/`includeIf` override may define as a push alias, or a git command that runs other git commands under one, is denied whatever branch it names. The explicit-bypass flags are denied **unconditionally**, since the flag is itself the bypass and needs no branch query: `gh pr merge --admin` overrides the server-side merge gate, and `git commit`/`git push --no-verify` skips the local git hooks, as does a `git -c core.hooksPath=` override on either, which points git at another hooks directory, and an `include.path`/`includeIf` override on either, whose file may do the same. These are read in a command the hook parses, a wrapper shell's body among them, and not inside a quoted `eval` argument. Third, a **mutating git operation run directly against a primary checkout** (`checkout`/`switch`/`pull`/`reset`/`rebase`/`merge`/`cherry-pick`/`revert`/`restore`/`add`/`commit`/`rm`/`mv`/`apply`/`am`/`push`/`stash` (anything but `list`/`show`)/`clean -f`/`worktree remove -f`), the harm behind a second incident where an agent reused the maintainer's own primary checkout instead of a worktree despite having read the prose rule against it. "Primary" is decided by comparing `git rev-parse --git-dir` against `--git-common-dir`, not a `.git`-is-a-directory guess. `worktree add`/`list`/`prune`, an unforced `worktree remove`, `merge --ff-only`/`pull --ff-only`, and a `checkout `/`switch ` carrying no force flag (bundled or attached forms like `-qf`/`-Bname` recognized, not only an exact token) stay exempt even in a primary checkout, matching the documented base-clone cleanup step the `repo-worktree` skill already carries. A subcommand this rule does not recognize is resolved through a chain of git aliases, inline and persisted, before falling through, so a custom alias cannot hide a denied builtin behind an unrecognized name. A `!`-prefixed shell alias is denied outright rather than interpreted. A directory named with a `..` component is not resolved, so a mutating command naming one is denied. That holds for `-C`, any `cd`, `pushd`, `popd`, or `chdir` word in the command, `--work-tree`, `--git-dir`, and a `GIT_DIR=`/`GIT_WORK_TREE=` prefix. The target directory follows real git's own `-C`/`--work-tree`/`--git-dir` priority, not a last-option-wins scan. A different owner-style grant, `GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT`, follows the same channel as `GH_WRITE_GUARD_ALLOW`. Reads and everything else pass through. Fourth, an **unbounded shell wait**, which the root spec's requirement 7 defines in full rather than this bullet redefining it. In outline: a loop that sleeps, carrying no bound the rule recognizes. It is the shape an agent writes when a brief tells it to poll. It is also the harm behind a third incident, where seven such loops outlived the subagents that wrote them and the run that dispatched those subagents. A shell started by a tool call runs in a session of its own, so the agent could not have reaped it even had it tried. A nested loop is judged on its own terms, and a command that forks anything out of a `timeout`'s reach, by backgrounding it or by `setsid`, is bounded by nothing. A heredoc body is data rather than a command line unless a shell is what it is fed to, so a document quoting the forbidden shape is written rather than denied. It fires even in autonomous / bypass-permissions sessions, which is how the first incident happened. See the root spec's requirements 6 and 7 for the exact rules. +- **`hooks/gh-write-guard.py`**: a PreToolUse hook that denies four classes of dangerous action. First, the GitHub **write** footguns behind the cross-repo comment incident: a state-changing `gh` call whose output is discarded, a GraphQL mutation passing a **literal** node id instead of a `$variable`, and a `gh` write whose explicit target is under an owner other than the checkout origin's. Sibling repositories under the same owner are allowed, since the harm this guards is reaching a stranger's repository rather than working across one maintainer's own fleet, and a different owner is allowed only when the maintainer names it in `GH_WRITE_GUARD_ALLOW` (an `owner/repo` list, where `owner/*` grants a whole owner). That variable is read from the environment the session was launched with, which is the one channel an agent cannot set for itself: a hook runs as its own process, so an inline `VAR=x cmd` prefix or an `export` inside a Bash call never reaches it. Second, a **git operation that bypasses a repository protection**. The branch-rule cases (a direct push, a force-push, or a branch delete to a branch that requires a pull request, a force-push where history is protected, a branch delete where deletion is blocked) are judged per refspec against the branch's **live** rules, so a code-style `develop` is denied while a config-style `develop` is allowed with no per-repo configuration. A push to a protected-default branch fails closed when its rules cannot be determined, either because the API is unreachable or because the checkout's origin cannot be resolved to query them. A push whose target the hook cannot read, such as one in text it does not parse, one carrying a `-c` config override, a git command an `include.path`/`includeIf` override may define as a push alias, or a git command that runs other git commands under one, is denied whatever branch it names. So is a push or commit holding a word the shell rewrites or drops before git reads it, such as a `$` or backtick refspec or subcommand, or a comment among a push's words. The explicit-bypass flags are denied **unconditionally**, since the flag is itself the bypass and needs no branch query: `gh pr merge --admin` overrides the server-side merge gate, and `git commit`/`git push --no-verify` skips the local git hooks, as does a `git -c core.hooksPath=` override on either, which points git at another hooks directory, and an `include.path`/`includeIf` override on either, whose file may do the same. These are read in a command the hook parses, a wrapper shell's body among them, and not inside a quoted `eval` argument. Third, a **mutating git operation run directly against a primary checkout** (`checkout`/`switch`/`pull`/`reset`/`rebase`/`merge`/`cherry-pick`/`revert`/`restore`/`add`/`commit`/`rm`/`mv`/`apply`/`am`/`push`/`stash` (anything but `list`/`show`)/`clean -f`/`worktree remove -f`), the harm behind a second incident where an agent reused the maintainer's own primary checkout instead of a worktree despite having read the prose rule against it. "Primary" is decided by comparing `git rev-parse --git-dir` against `--git-common-dir`, not a `.git`-is-a-directory guess. `worktree add`/`list`/`prune`, an unforced `worktree remove`, `merge --ff-only`/`pull --ff-only`, and a `checkout `/`switch ` carrying no force flag (bundled or attached forms like `-qf`/`-Bname` recognized, not only an exact token) stay exempt even in a primary checkout, matching the documented base-clone cleanup step the `repo-worktree` skill already carries. A subcommand this rule does not recognize is resolved through a chain of git aliases, inline and persisted, before falling through, so a custom alias cannot hide a denied builtin behind an unrecognized name. A `!`-prefixed shell alias is denied outright rather than interpreted. A directory named with a `..` component is not resolved, so a mutating command naming one is denied. That holds for `-C`, any `cd`, `pushd`, `popd`, or `chdir` word in the command, `--work-tree`, `--git-dir`, and a `GIT_DIR=`/`GIT_WORK_TREE=` prefix. The target directory follows real git's own `-C`/`--work-tree`/`--git-dir` priority, not a last-option-wins scan. A different owner-style grant, `GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT`, follows the same channel as `GH_WRITE_GUARD_ALLOW`. Reads and everything else pass through. Fourth, an **unbounded shell wait**, which the root spec's requirement 7 defines in full rather than this bullet redefining it. In outline: a loop that sleeps, carrying no bound the rule recognizes. It is the shape an agent writes when a brief tells it to poll. It is also the harm behind a third incident, where seven such loops outlived the subagents that wrote them and the run that dispatched those subagents. A shell started by a tool call runs in a session of its own, so the agent could not have reaped it even had it tried. A nested loop is judged on its own terms, and a command that forks anything out of a `timeout`'s reach, by backgrounding it or by `setsid`, is bounded by nothing. A heredoc body is data rather than a command line unless a shell is what it is fed to, so a document quoting the forbidden shape is written rather than denied. It fires even in autonomous / bypass-permissions sessions, which is how the first incident happened. See the root spec's requirements 6 and 7 for the exact rules. - **`hooks/tool-containment.py`**: a shell prefix, registered as `CLAUDE_CODE_SHELL_PREFIX` in the `env` block of `settings.json`, that runs every Bash tool call inside a transient systemd user scope carrying `TasksMax`, `MemoryMax`, and `MemorySwapMax=0`, the last so a runaway at the memory ceiling is killed rather than spilled into the host's swap. A runaway fan-out then fails at the ceiling with `fork: Resource temporarily unavailable` instead of growing until the host has to be reset, the harm behind the probe-shim incident in the root spec's requirement 9. The guard above reads the command text, and that incident's recursion lived in a script file the command only named. The ceilings default to 8192 tasks and 25% of RAM. A `claude` session started from a tool call is nested in the session that ran it. Every scope of such a chain also joins one slice, `claude-tree-.slice`. That slice carries a total ceiling for the whole tree, 32768 tasks and 50% of RAM. The root's id reaches nested sessions as `AGENT_CONTAINMENT_ROOT`, which every contained command inherits. The tree's ceiling is a runtime drop-in put on when its slice is first created, and the sweep below reverts it. The maintainer raises any ceiling in the same `env` block. An inline `VAR=x` in a command cannot change one, since the prefix reads it before the command runs. The variables are `AGENT_CONTAINMENT_TASKS_MAX` and `AGENT_CONTAINMENT_MEMORY_MAX` for a scope, and `AGENT_CONTAINMENT_TREE_TASKS_MAX` and `AGENT_CONTAINMENT_TREE_MEMORY_MAX` for the tree. A tree ceiling set below a scope's binds first. Each scope is named `claude-tool---`, so the sweep below can stop one session's tool calls and nothing else. Its description records the pid of the agent that ran it. That record is how the sweep tells a scope whose session has ended from one whose session still runs. The installer sets the prefix only where it can contain, per the Linux bullet under "Scope and Limits", and never over a `CLAUDE_CODE_SHELL_PREFIX` value that names anything else. - **`hooks/stray-process-sweep.py`**: a SessionEnd hook in two phases. First, it stops every `claude-tool--*` scope this session left loaded and reports what each held, since the scope's name proves which session started it. It then stops every scope in this session's tree whose recorded agent is no longer running, and names each one's session. That is how a nested session's scopes end, since stopping the parent's scopes kills the nested `claude` before its own sweep runs. Each stop can end an agent nested deeper, so it judges again, up to four rounds. The root session then stops its tree's slice and reverts the ceiling put on it, once nothing is left in it. Second, it reports, and never ends, the process trees outside those scopes that outlive the session. The guard above stops a leak from being written. This finds the ones already running: the ones started before the guard was installed, and the shapes the guard deliberately does not reach. It reports every descendant of the agent process running outside the agent's own session. It names each subtree root's command and age, and hands over the `kill` line. This phase kills nothing itself. A long build backgrounded on purpose reads identically to a leaked wait from there. What is still wanted on the machine is the maintainer's call. A SessionEnd hook cannot block, and its JSON output is discarded, so stderr on exit 2 is the one channel that reaches the maintainer. The sweep uses that for a report rather than for a refusal. It registers under a group carrying no matcher, which fires on every exit reason. It also carries an explicit timeout, since the event's own budget is 1.5 seconds and only a declared per-hook timeout raises it. See the root spec's requirement 8. - **The whole of `CLAUDE.md`, rendered on every run**, loaded into every session on the machine (including ad-hoc work outside any project). It holds a header, two marker-delimited blocks of host-wide content, and the host-local file. Nothing else survives an install. Content outside the blocks was never written or checked, and it restated fleet rules in wording the fleet had since changed. @@ -21,7 +21,7 @@ Into `~/.claude/` (or `%USERPROFILE%\.claude\` on Windows): - A local file that cannot be read, is not a regular file, or is not UTF-8 stops the install before anything changes. So does one carrying this kit's own markers, since appending it would duplicate a block. - The `agent-safety` block carries restrictions alone, so nothing in it can widen a permission. `Repository Boundaries and Write Safety` mirrors the committed `GOVERNANCE.md` section of the same name, carrying as behavioral guidance its write rules and its rules that a task works in its own checkout and isolates into its own worktree before its first file edit. `Data in Agent-Authored Text` mirrors `GOVERNANCE.md` "Representative Data in Agent-Authored Text", so an agent illustrates with constructed data rather than data observed on the machine. `Authorization Scope and Memory Hygiene` bounds how a granted permission is recorded and read. Each mirrors a rule that otherwise reaches fleet repos only. - The `fleet-bootstrap` block carries `Fleet Governance Entry Point`, which names the template repository and routes by the state a repository is actually in. It is separate precisely because it enables rather than restricts, and the safety block's own text says nothing in it widens a permission, so merging the two would contradict that. It is host-wide rather than per repository because the repositories that most need it are the ones carrying no instruction set to point the way, and it mirrors the byte-locked `AGENTS.md` "Fleet Bootstrap" section that a conformant repository carries. -- **The permission rules this kit owns, merged into `settings.json`** beside the hook registration. Each is declared as a prefix and a rule, and a re-run drops every rule the prefix owns before adding the current one, so a rule whose spelling changes updates in place rather than accumulating beside the version it replaced. Ownership requires a rule-syntax delimiter after the prefix, since the prefix ends at the script name and a bare prefix test would also claim a longer path such as `pr_review.py-custom`, so a rule written by hand for a different script is never touched. These widen rather than restrict, which is why they are their own component for the same reason the `fleet-bootstrap` block is separate from the `agent-safety` one. Today the list holds one rule, for `scripts/pr_review.py`, the review loop's reply and resolve. Driving that loop by hand needs a raw GraphQL mutation carrying a node id, which is the shape that reached a stranger's repository, where the script queries the id itself and takes no argument an id fits in. What the rule decides is which command runs without a prompt, and it matches the command text rather than the directory the command runs in, so it reaches a `scripts/pr_review.py` in any checkout that carries one. An absolute path would not narrow that, since the hub is reached as a checkout of the caller's own and its location differs per task, so pinning one path would name a checkout the next task does not use. What bounds it is the rule that an agent reaches the hub as a checkout of its own, fetched immediately before it is read, rather than a copy it happens to find on disk, which the `fleet-bootstrap` block beside this carries and [`GOVERNANCE.md`][governance] "Hub-Hosted Tooling" states in full. +- **The permission rules this kit owns, merged into `settings.json`** beside the hook registration. Each is declared as a prefix and a rule, and a re-run drops every rule the prefix owns before adding the current one, so a rule whose spelling changes updates in place rather than accumulating beside the version it replaced. Ownership requires a rule-syntax delimiter after the prefix, since the prefix ends at the script name and a bare prefix test would also claim a longer path such as `pr_review.py-custom`, so a rule written by hand for a different script is never touched. These widen rather than restrict, which is why they are their own component for the same reason the `fleet-bootstrap` block is separate from the `agent-safety` one. Today the list holds one rule, for `scripts/pr_review.py`, which covers the review loop's `reply` and resolve and the `cleanup` delete of a merged pull request's head branch. Driving that loop by hand needs a raw GraphQL mutation carrying a node id, which is the shape that reached a stranger's repository, where the script queries the id itself and takes no argument an id fits in. The `:*` rule approves any arguments, so it approves a `cleanup` call whatever branch or pull request it names, and every check the delete makes lives inside the subcommand rather than in the rule. What the rule decides is which command runs without a prompt, and it matches the command text rather than the directory the command runs in, so it reaches a `scripts/pr_review.py` in any checkout that carries one. An absolute path would not narrow that, since the hub is reached as a checkout of the caller's own and its location differs per task, so pinning one path would name a checkout the next task does not use. What bounds it is the rule that an agent reaches the hub as a checkout of its own, fetched immediately before it is read, rather than a copy it happens to find on disk, which the `fleet-bootstrap` block beside this carries and [`GOVERNANCE.md`][governance] "Hub-Hosted Tooling" states in full. See [`../README.md`][spec] "Requirements" for which of these each rule implements, and "Auditing an Implementation Against This Spec" for how to check this hook still satisfies them after a change. diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index 75b499b07..e5f117189 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -2216,6 +2216,27 @@ def _is_short_u(arg): _INLINE_PUSH_ALIAS = re.compile(r"alias\..*\bpush\b", re.DOTALL | re.IGNORECASE) _GIT_INCLUDE = re.compile(r"git\b.*?include", re.DOTALL | re.IGNORECASE) _GIT_INCLUDE_KEY = re.compile(r"git\b.*?include(?:\.path|if\.)", re.DOTALL | re.IGNORECASE) +_GIT_PUSH_UNQUOTED = re.compile(r"(?i:git)\b.*?\bpush\b", re.DOTALL) +_QUOTING_CHARS = re.compile(r"[\"'\\]") + + +def _unquoted(text): + """`text` with every quote and backslash removed, which joins the words the shell joins.""" + return _QUOTING_CHARS.sub("", text) + + +def _names_push(text): + """True when `text` spells a git push as written, or once its quoting is removed. + + The shell joins `p''ush` into `push` before git reads it. The second reading matches `git` + without case, as `_is_git_exe` does, since a case-insensitive shell runs `GIT` as git. + """ + return bool(_GIT_PUSH.search(text) or _GIT_PUSH_UNQUOTED.search(_unquoted(text))) + + +def _names_include(text, pattern=_GIT_INCLUDE): + """True when `pattern` matches `text` as written, or once its quoting is removed.""" + return bool(pattern.search(text) or pattern.search(_unquoted(text))) def _includes_config_file(value): @@ -2267,7 +2288,7 @@ def _unreadable_push(cmd): """ overflow = [] texts = [cmd, *_embedded_wrapper_commands(cmd, overflow=overflow)] - if any(_GIT_PUSH.search(t) or _GIT_INCLUDE_KEY.search(t) for t in overflow): + if any(_names_push(t) or _names_include(t, _GIT_INCLUDE_KEY) for t in overflow): return "wrapper shells nested deeper than the guard reads" budget = _EVAL_READ_BUDGET for text in texts: @@ -2278,26 +2299,30 @@ def _unreadable_push(cmd): for k, t in enumerate(toks): if t == "eval" and k >= eval_end: if budget <= 0: - if _GIT_INCLUDE_KEY.search(" ".join(toks[k:])): + if _names_include(" ".join(toks[k:]), _GIT_INCLUDE_KEY): return "an include override inside more `eval` text than the guard reads" eval_end = n else: payload, eval_end = _eval_payload(toks, k, quoted) budget -= len(payload) + _EVAL_READ_COST - if _GIT_INCLUDE.search(payload): + if _names_include(payload): texts.append(payload) texts.extend(_embedded_wrapper_commands(payload, overflow=overflow)) - if any(_GIT_PUSH.search(o) or _GIT_INCLUDE_KEY.search(o) for o in overflow): + if any( + _names_push(o) or _names_include(o, _GIT_INCLUDE_KEY) for o in overflow + ): return "wrapper shells nested deeper than the guard reads" if t in ("eval", "source", ".") or _is_shell_wrapper_exe(t): args, _ = _collect_arglist(toks, k + 1) if _is_shell_wrapper_exe(t): args = _wrapper_unread_args(args) - if any(_GIT_PUSH.search(a) for a in args): + if any(_names_push(a) for a in args): return ( f"a push inside an argument of `{t}` the guard does not read as a command" ) - if _is_shell_wrapper_exe(t) and any(_GIT_INCLUDE_KEY.search(a) for a in args): + if _is_shell_wrapper_exe(t) and any( + _names_include(a, _GIT_INCLUDE_KEY) for a in args + ): return ( f"an include override inside an argument of `{t}` the guard does not read " "as a command" @@ -2307,7 +2332,12 @@ def _unreadable_push(cmd): j = _git_subcommand_index(toks, k) opts = toks[k + 1 : j] values = _git_config_overrides(opts) - if any(_GIT_PUSH.search(v) or _INLINE_PUSH_ALIAS.match(v) for v in values): + if any( + _names_push(v) + or _INLINE_PUSH_ALIAS.match(v) + or _INLINE_PUSH_ALIAS.match(_unquoted(v)) + for v in values + ): return "a push inside a git `-c` config value" config = bool(values) if j >= n: @@ -2333,13 +2363,245 @@ def _unreadable_push(cmd): ) if toks[j] in _COMMAND_RUNNING_GIT_SUBS: args, _ = _collect_arglist(toks, j + 1) - if config and any(a == "push" or _GIT_PUSH.search(a) for a in args): + if config and any(a == "push" or _names_push(a) for a in args): return f"a push run by `git {toks[j]}` under a `-c`/`--config-env` override" - if any(_GIT_PUSH.search(a) for a in args): + if any(_names_push(a) for a in args): return f"a push inside one quoted argument of `git {toks[j]}`" return None +_GIT_DIR_VALUE_OPTS = frozenset({"-C", "--git-dir", "--work-tree"}) +_COMMIT_VALUE_OPTS = frozenset( + {"-m", "--message", "-F", "--file", "-C", "--reuse-message", "-c", "--reedit-message"} + | {"--author", "--date", "-t", "--template", "--cleanup", "--fixup", "--squash", "--trailer"} +) +_COMMIT_VALUE_LETTERS = "mFCct" + + +def _is_run_time_word(tok): + """True when the shell may rewrite a token at run time, a `$` or a backtick in it or a lone parenthesis run.""" + return "$" in tok or "`" in tok or (tok != "" and set(tok) <= {"(", ")"}) + + +def _region_end(toks, k): + """The index of the separator after `toks[k]` that ends its command. + + A parenthesis the command opens, as a `$(...)` in one of its words does, is read through to the + one closing it, separators inside included. A closing one it did not open ends the command, as + a subshell's does. + """ + n = len(toks) + depth = 0 + e = k + 1 + while e < n: + t = toks[e] + if _is_separator(t): + if depth == 0 and t[0] not in "()": + break + depth += t.count("(") - t.count(")") + if depth < 0 or (depth == 0 and not set(t) <= {"(", ")"}): + break + e += 1 + return e + + +def _span_end(toks, i, end): + """The index after a run-time word starting at `toks[i]` that spans several tokens, or `i + 1`. + + A `$(...)` and a backtick substitution holding a space each lex as several tokens. The token at + `end` is read too, since a fused `))` closes the span and the group holding the command at once. + None says the span does not close by then. + """ + t = toks[i] + if t.endswith("$") and i + 1 < end and toks[i + 1].startswith("("): + depth = 0 + for x in range(i + 1, min(end + 1, len(toks))): + if _is_shell_op(toks[x]): + depth += toks[x].count("(") - toks[x].count(")") + if depth <= 0: + return min(x + 1, end) + return None + if t.count("`") % 2: + for x in range(i + 1, end): + if toks[x].count("`") % 2: + return x + 1 + return None + return i + 1 + + +def _is_commit_value_option(tok): + """True when a commit argument takes the next word as its value, as `-m` or a cluster such as `-am` does.""" + if tok in _COMMIT_VALUE_OPTS: + return True + return ( + len(tok) > 1 + and tok[0] == "-" + and tok[1] != "-" + and tok[1:].isalpha() + and tok[-1] in _COMMIT_VALUE_LETTERS + ) + + +def _holds_attached_value(tok): + """True when a run-time word sits after an option name that takes it as an attached value. + + That is `--name=`, or a short commit value option such as `-m`. + """ + cut = min((tok.index(c) for c in "$`" if c in tok), default=len(tok)) + literal = tok[:cut] + if literal.startswith("--") and "=" in literal: + return True + return len(literal) >= 2 and literal[0] == "-" and literal[1] in _COMMIT_VALUE_LETTERS + + +def _expands_unquoted(raw): + """True when a token as written holds a `$` or a backtick outside double quotes. + + Word splitting may split what such an expansion gives into more arguments. One inside a + `$(...)` that double quotes hold is still inside those quotes. + """ + stack = [] + i = 0 + while i < len(raw): + ch = raw[i] + top = stack[-1] if stack else None + if top == "'": + if ch == "'": + stack.pop() + elif ch == "\\": + i += 1 + elif top == '"': + if ch == '"': + stack.pop() + elif raw.startswith("$(", i): + stack.append("(") + i += 1 + elif ch == "`": + stack.append("`") + elif ch in "'\"": + stack.append(ch) + elif (top == "(" and ch == ")") or (top == "`" and ch == "`"): + stack.pop() + elif ch in "$`" and '"' not in stack: + return True + elif raw.startswith("$(", i): + stack.append("(") + i += 1 + elif ch == "`": + stack.append("`") + i += 1 + return False + + +def _split_free(x, toks, mask, raw): + """True when word splitting cannot split `toks[x]`, read from the quote-keeping token where known. + + Where that token is unknown, a token opening with `$(` and holding more is read as quoted. The + operator lex and `_context_lex` each split an unquoted `$(` at its parenthesis, and the token + union `_shell_tokens` returns then holds a reading that denies it. + """ + if raw is not None: + return not _expands_unquoted(raw[x]) + tok = toks[x] + return ( + bool(mask and mask[x]) + or not _expands_unquoted(tok) + or (len(tok) > 2 and tok.startswith("$(")) + ) + + +def _rewritten_git_words(cmd): + """Name the form when a git push or commit holds a word the shell rewrites or drops, else None. + + The guard reads each word as written, while the shell expands a `$` or a backtick word and drops + a comment before git reads any of them. Modeling either is open-ended, so the command fails closed. + + - a run-time word where the subcommand goes, on any git command, since it may expand to `push` or + to an option that moves the subcommand, as a backtick word ending the global options does + - a run-time word among a push's or a commit's global options, since `-c "$X"` may set + `core.hooksPath` + - a run-time word among a push's arguments, since it may name any refspec or remote + - a run-time word among a commit's arguments, since it may expand to `--no-verify` + - a comment among a push's words, since the shell drops what the guard reads as a refspec or a + flag there, which can turn a bare push into one naming another branch + + A comment on a commit only adds words the guard reads, which can only add a deny, so it is left + alone. Four kinds of run-time word are left alone too. One is the value of a global option + naming a directory, unquoted too, since `-C $dir` is the common spelling, and what word + splitting makes of an unquoted one is not read. One is a commit option's value + whose expansions sit inside double quotes, since word splitting cannot then make it an option, + as in `-m "$(cat <<'EOF' ...)"`. One is a commit path after `--`. One is a redirection target. + Only a git word in a command's position counts, so a `grep git "$dir"` is no git command, and + that test is `_runs_as_command`'s. A git word right after an unquoted `(` counts too, since a + process substitution such as `<(git ...)` runs it whatever command reads the result. + """ + for text in [cmd, *_embedded_wrapper_commands(cmd)]: + toks = _shell_tokens(text) + mask = raw = None + masked = False + for k, t in enumerate(toks): + if not _is_git_exe(t): + continue + end = _region_end(toks, k) + j = min(_git_subcommand_index(toks, k), end) + if j >= end: + continue + if not masked: + mask, raw, masked = _quoted_mask(text, toks), _quote_kept_tokens(text, toks), True + opened = k and toks[k - 1].endswith("(") and not (mask and mask[k - 1]) + if not opened and not _runs_as_command(toks, k, mask): + continue + sub = toks[j] + if _is_run_time_word(sub): + return f"a run-time word, `{sub}`, where the subcommand of `git` goes" + if sub not in ("push", "commit"): + continue + if sub == "push" and any( + toks[x].startswith("#") and not (mask and mask[x]) for x in range(k + 1, end) + ): + return "a comment among the words of `git push`" + x = k + 1 + while x < j: + opt = toks[x] + if opt in _GIT_DIR_VALUE_OPTS and x + 1 < j and _span_end(toks, x + 1, j) == x + 2: + x += 2 + continue + if not opt.startswith(("--git-dir=", "--work-tree=")) and _is_run_time_word(opt): + return f"a run-time word, `{opt}`, among the global options of `git {sub}`" + x += 1 + + x = j + 1 + after_dashes = False + while x is not None and x < end: + arg = toks[x] + if _is_redir_op(arg): + x += 1 + if x < end and not _is_shell_op(toks[x]): + x = _span_end(toks, x, end) + continue + if sub == "commit": + after_dashes = after_dashes or arg == "--" + if after_dashes: + x += 1 + continue + if ( + _is_commit_value_option(arg) + and x + 1 < end + and _split_free(x + 1, toks, mask, raw) + ): + x += 2 + continue + if _holds_attached_value(arg) and _split_free(x, toks, mask, raw): + x += 1 + continue + if _is_run_time_word(arg): + return f"a run-time word, `{arg}`, among the arguments of `git {sub}`" + x += 1 + if x is None: + return f"a redirection target that does not close among the words of `git {sub}`" + return None + + def _push_invocations(cmd): """Every `git push` as (c_dirs, work_tree, leading_cd, cd_unreliable, args), wrappers included. @@ -4193,10 +4455,19 @@ def classify( dec, reason = _check_bypass_flags(cmd) if dec == "deny": return dec, reason + # A word the shell rewrites or drops before git reads it hides what a push targets or what a commit skips, so it fails closed beside the readings below. + rewritten = _rewritten_git_words(cmd) + if rewritten: + return "deny", ( + f"This command holds {rewritten}, which the shell rewrites or drops before git reads " + "it, so the guard cannot tell what the command runs. Failing closed. Run the push or " + "commit as its own command with every word written out, and a push with an explicit " + "refspec (git push origin :refs/heads/)." + _handoff(cmd) + ) # The `_push_targets` helper tokenizes with shlex and keys off a real `git push` argv adjacency, so a push named only inside a quoted argument yields no target. # The raw substring is just a cheap pre-filter. - if _GIT_PUSH.search(cmd) or _GIT_INCLUDE.search(cmd): - if split_word and _GIT_PUSH.search(cmd): + if _names_push(cmd) or _names_include(cmd): + if split_word and _names_push(cmd): return "deny", ( "This push sits in a command with a word split by a backslash-newline, which the shell joins " "into one word the guard cannot read, so a hidden cd could redirect the push. Write each " @@ -5696,6 +5967,188 @@ def classify( "allow", "control: an --exec-path with no include pulls in no alias", ), + ( + "git p''ush origin develop", + None, + {"develop": _CODE_RULES}, + "deny", + "a push word split by quotes is read as the push the shell joins it into", + ), + ( + 'git "pu"sh origin develop', + None, + {"develop": _CODE_RULES}, + "deny", + "a push word split by a double-quoted part is read the same way", + ), + ( + "GIT push origin develop", + None, + {"develop": _CODE_RULES}, + "deny", + "an uppercase GIT reaches the push check, as a case-insensitive shell runs it as git", + ), + ( + "git -c 'inc''lude.path=/example/config' p", + "feature/x", + {"feature/x": set()}, + "deny", + "an include key split by quotes reaches the include check", + ), + ( + "eval \"git p''ush origin develop\"", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a quote-split push inside an eval argument is still a push the guard does not read", + ), + ( + "git push origin $(echo develop)", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a substitution as a refspec is a run-time word the guard cannot read", + ), + ( + "git push origin `echo develop`", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a backtick refspec is a run-time word too", + ), + ( + "B=develop; git push origin $B", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a variable refspec is a run-time word too", + ), + ( + 'git push origin "$B"', + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a quoted variable refspec still expands at run time", + ), + ( + "git push origin dev${X}elop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "an empty expansion inside a refspec is a run-time word", + ), + ( + "git push origin $'develop'", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "an ANSI-C quoted refspec is a run-time word", + ), + ( + "bash -c 'git push origin \"$1\"' _ develop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a positional parameter as a refspec in a wrapper body is a run-time word", + ), + ( + "git $(echo push) origin develop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a substitution where the subcommand goes may expand to push", + ), + ( + "git push # feature/y", + "develop", + {"develop": _CODE_RULES, "feature/y": set()}, + "deny", + "a comment's words are not read as the refspecs of a bare push of develop", + ), + ( + "git -c core.hooksPath=/dev/null `true` commit -m x", + "feature/x", + {"feature/x": set()}, + "deny", + "a backtick word ending the global options does not hide a hooks-path override on a commit", + ), + ( + "git commit $(echo --no-verify) -m x", + "feature/x", + {"feature/x": set()}, + "deny", + "a substitution among a commit's arguments may expand to --no-verify", + ), + ( + "git commit -m $MSG", + "feature/x", + {"feature/x": set()}, + "deny", + "an unquoted commit message variable may split into more options", + ), + ( + 'git -c "$X" commit -m x', + "feature/x", + {"feature/x": set()}, + "deny", + "a run-time config override on a commit may set core.hooksPath", + ), + ( + 'git -C "$W" push origin feature/x', + None, + {"feature/x": set()}, + "allow", + "control: a run-time directory value leaves an explicit push readable", + ), + ( + "git commit -m \"$(cat <<'EOF'\nFix the example's edge\n\nCo-Authored-By: Example \nEOF\n)\"", + "feature/x", + {"feature/x": set()}, + "allow", + "control: a double-quoted heredoc commit message cannot split into options", + ), + ( + 'git commit -m "$(cat <<\'EOF\'\nQuote " once\nEOF\n)"', + "feature/x", + {"feature/x": set()}, + "allow", + "control: a heredoc commit message holding one double quote is still read as quoted", + ), + ( + 'git commit -m "$MSG" -- "$F"', + "feature/x", + {"feature/x": set()}, + "allow", + "control: a quoted message variable and a path after -- leave a commit readable", + ), + ( + 'git commit -m "fix" # note', + "feature/x", + {"feature/x": set()}, + "allow", + "control: a comment on a commit only adds words the guard reads", + ), + ( + 'grep -rn git "$W"', + "feature/x", + {"feature/x": set()}, + "allow", + "control: git as an argument of grep is no git command", + ), + ( + 'git push origin feature/x 2>&1 | tee "$LOG"', + None, + {"feature/x": set()}, + "allow", + "control: a run-time redirection target is not a refspec", + ), + ( + "(git push origin feature/x)", + None, + {"feature/x": set()}, + "allow", + "control: a subshell's closing parenthesis ends the push", + ), ] # (command, cwd, {dir: is_primary_or_None}, {(dir, ref): resolves_as_ref} or None, expected_decision, label) -- is_primary is True (a primary checkout), False (a linked worktree), or None (unresolved, for example when no repository exists or the git query itself fails, such as on a pre-2.31 git lacking `rev-parse --path-format`). diff --git a/host-setup/agent-safety/claude/install.py b/host-setup/agent-safety/claude/install.py index 2802cf118..7299bc2de 100755 --- a/host-setup/agent-safety/claude/install.py +++ b/host-setup/agent-safety/claude/install.py @@ -212,8 +212,9 @@ def at(data, path): # Ownership needs a delimiter after the prefix, so a longer path such as `pr_review.py-custom` is not claimed. # These widen rather than restrict, so they stay their own step for the reason the two CLAUDE.md blocks stay separate. MANAGED_PERMISSIONS = [ - # The review loop's reply and resolve, the one write in that loop an agent performs. - # Driving it by hand needs a raw GraphQL mutation carrying a node id, which is the shape to avoid. + # The review loop's reply and resolve, and the `cleanup` delete of a merged pull request's head branch. + # Driving either by hand needs a raw mutation carrying an id, which is the shape to avoid. + # The `:*` rule approves any arguments, so every check the delete makes lives inside the `cleanup` subcommand. # The rule decides which command skips a prompt, and it bounds no checkout, since it matches the text. ("Bash(python3 scripts/pr_review.py", "Bash(python3 scripts/pr_review.py:*)"), ] From 78a6b61b4a038b1cc8ef80394197648d1f766c30 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 13 Oct 2026 21:41:16 -0700 Subject: [PATCH 2/8] Close the Local Review's Gaps in the Shell-Rewrite Deny A commit option value is split-free only where its quote-keeping token shows every expansion inside double quotes, or where it is one closed $(...) word. A git with no subcommand word denies an unquoted run-time option value. An abbreviated long value option takes the next word as its value. A brace expansion is a run-time word. A redirection target spanning several words, which the push and bypass readings stop at, denies. A process substitution no longer ends the scanned command. Co-Authored-By: Claude Opus 5.5 --- host-setup/agent-safety/README.md | 15 +- host-setup/agent-safety/claude/README.md | 4 +- .../agent-safety/claude/gh-write-guard.py | 168 +++++++++++++----- 3 files changed, 138 insertions(+), 49 deletions(-) diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md index 9405e2c7b..67d5e605a 100644 --- a/host-setup/agent-safety/README.md +++ b/host-setup/agent-safety/README.md @@ -80,13 +80,14 @@ text says, because the harm it covers was never in the text. `HEAD`/`@` refspec whose directory or branch may change before it runs. Such a push's local branch is checked under its whole name, such as `release/1.0`. It is also checked with its first segment dropped. A remote-tracking push target such as `origin/feature/x` is checked with its - remote dropped. A push or commit holding a word the shell rewrites or drops before git reads it - is denied too. That is a `$` or backtick word among a push's words, among a commit's global - options and arguments, or where any git command's subcommand goes, or a comment among a push's - words. A directory option's value, a commit option's double-quoted value, and a commit path - after `--` are read as they stand. The push and include checks also read each command with its - quotes and backslashes removed and `git` in any case, so a `p''ush` or an uppercase `GIT` - reaches them. The + remote dropped. A git command holding a word the shell rewrites or drops before git reads it + is denied too. That word is a `$`, backtick, or brace-expansion word. It is denied where any + git command's subcommand goes, and among a push's words or a commit's. A redirection target + spanning several words on either is denied too. So is a comment among a push's words. Some + words are read as they stand. They are a directory option's value, a commit option's + double-quoted value, a commit path after `--`, and a redirection target held in one word. The + push and include checks also read each command with its quotes and backslashes removed and + `git` in any case, so a `p''ush` or an uppercase `GIT` reaches them. The workaround is an explicit refspec run as its own command. Every other requirement here favors precision over recall throughout, denying only a positively-identified dangerous shape, since a hook that fails closed on an unrelated resolution failure blocks diff --git a/host-setup/agent-safety/claude/README.md b/host-setup/agent-safety/claude/README.md index 5372cefa9..95c2551ea 100644 --- a/host-setup/agent-safety/claude/README.md +++ b/host-setup/agent-safety/claude/README.md @@ -12,7 +12,7 @@ installation and operational detail. Codex and opencode have no equivalent hook Into `~/.claude/` (or `%USERPROFILE%\.claude\` on Windows): -- **`hooks/gh-write-guard.py`**: a PreToolUse hook that denies four classes of dangerous action. First, the GitHub **write** footguns behind the cross-repo comment incident: a state-changing `gh` call whose output is discarded, a GraphQL mutation passing a **literal** node id instead of a `$variable`, and a `gh` write whose explicit target is under an owner other than the checkout origin's. Sibling repositories under the same owner are allowed, since the harm this guards is reaching a stranger's repository rather than working across one maintainer's own fleet, and a different owner is allowed only when the maintainer names it in `GH_WRITE_GUARD_ALLOW` (an `owner/repo` list, where `owner/*` grants a whole owner). That variable is read from the environment the session was launched with, which is the one channel an agent cannot set for itself: a hook runs as its own process, so an inline `VAR=x cmd` prefix or an `export` inside a Bash call never reaches it. Second, a **git operation that bypasses a repository protection**. The branch-rule cases (a direct push, a force-push, or a branch delete to a branch that requires a pull request, a force-push where history is protected, a branch delete where deletion is blocked) are judged per refspec against the branch's **live** rules, so a code-style `develop` is denied while a config-style `develop` is allowed with no per-repo configuration. A push to a protected-default branch fails closed when its rules cannot be determined, either because the API is unreachable or because the checkout's origin cannot be resolved to query them. A push whose target the hook cannot read, such as one in text it does not parse, one carrying a `-c` config override, a git command an `include.path`/`includeIf` override may define as a push alias, or a git command that runs other git commands under one, is denied whatever branch it names. So is a push or commit holding a word the shell rewrites or drops before git reads it, such as a `$` or backtick refspec or subcommand, or a comment among a push's words. The explicit-bypass flags are denied **unconditionally**, since the flag is itself the bypass and needs no branch query: `gh pr merge --admin` overrides the server-side merge gate, and `git commit`/`git push --no-verify` skips the local git hooks, as does a `git -c core.hooksPath=` override on either, which points git at another hooks directory, and an `include.path`/`includeIf` override on either, whose file may do the same. These are read in a command the hook parses, a wrapper shell's body among them, and not inside a quoted `eval` argument. Third, a **mutating git operation run directly against a primary checkout** (`checkout`/`switch`/`pull`/`reset`/`rebase`/`merge`/`cherry-pick`/`revert`/`restore`/`add`/`commit`/`rm`/`mv`/`apply`/`am`/`push`/`stash` (anything but `list`/`show`)/`clean -f`/`worktree remove -f`), the harm behind a second incident where an agent reused the maintainer's own primary checkout instead of a worktree despite having read the prose rule against it. "Primary" is decided by comparing `git rev-parse --git-dir` against `--git-common-dir`, not a `.git`-is-a-directory guess. `worktree add`/`list`/`prune`, an unforced `worktree remove`, `merge --ff-only`/`pull --ff-only`, and a `checkout `/`switch ` carrying no force flag (bundled or attached forms like `-qf`/`-Bname` recognized, not only an exact token) stay exempt even in a primary checkout, matching the documented base-clone cleanup step the `repo-worktree` skill already carries. A subcommand this rule does not recognize is resolved through a chain of git aliases, inline and persisted, before falling through, so a custom alias cannot hide a denied builtin behind an unrecognized name. A `!`-prefixed shell alias is denied outright rather than interpreted. A directory named with a `..` component is not resolved, so a mutating command naming one is denied. That holds for `-C`, any `cd`, `pushd`, `popd`, or `chdir` word in the command, `--work-tree`, `--git-dir`, and a `GIT_DIR=`/`GIT_WORK_TREE=` prefix. The target directory follows real git's own `-C`/`--work-tree`/`--git-dir` priority, not a last-option-wins scan. A different owner-style grant, `GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT`, follows the same channel as `GH_WRITE_GUARD_ALLOW`. Reads and everything else pass through. Fourth, an **unbounded shell wait**, which the root spec's requirement 7 defines in full rather than this bullet redefining it. In outline: a loop that sleeps, carrying no bound the rule recognizes. It is the shape an agent writes when a brief tells it to poll. It is also the harm behind a third incident, where seven such loops outlived the subagents that wrote them and the run that dispatched those subagents. A shell started by a tool call runs in a session of its own, so the agent could not have reaped it even had it tried. A nested loop is judged on its own terms, and a command that forks anything out of a `timeout`'s reach, by backgrounding it or by `setsid`, is bounded by nothing. A heredoc body is data rather than a command line unless a shell is what it is fed to, so a document quoting the forbidden shape is written rather than denied. It fires even in autonomous / bypass-permissions sessions, which is how the first incident happened. See the root spec's requirements 6 and 7 for the exact rules. +- **`hooks/gh-write-guard.py`**: a PreToolUse hook that denies four classes of dangerous action. First, the GitHub **write** footguns behind the cross-repo comment incident: a state-changing `gh` call whose output is discarded, a GraphQL mutation passing a **literal** node id instead of a `$variable`, and a `gh` write whose explicit target is under an owner other than the checkout origin's. Sibling repositories under the same owner are allowed, since the harm this guards is reaching a stranger's repository rather than working across one maintainer's own fleet, and a different owner is allowed only when the maintainer names it in `GH_WRITE_GUARD_ALLOW` (an `owner/repo` list, where `owner/*` grants a whole owner). That variable is read from the environment the session was launched with, which is the one channel an agent cannot set for itself: a hook runs as its own process, so an inline `VAR=x cmd` prefix or an `export` inside a Bash call never reaches it. Second, a **git operation that bypasses a repository protection**. The branch-rule cases (a direct push, a force-push, or a branch delete to a branch that requires a pull request, a force-push where history is protected, a branch delete where deletion is blocked) are judged per refspec against the branch's **live** rules, so a code-style `develop` is denied while a config-style `develop` is allowed with no per-repo configuration. A push to a protected-default branch fails closed when its rules cannot be determined, either because the API is unreachable or because the checkout's origin cannot be resolved to query them. A push whose target the hook cannot read, such as one in text it does not parse, one carrying a `-c` config override, a git command an `include.path`/`includeIf` override may define as a push alias, or a git command that runs other git commands under one, is denied whatever branch it names. So is a git command holding a word the shell rewrites or drops before git reads it. Such a word is a `$`, backtick, or brace-expansion refspec or subcommand, or a comment among a push's words. The explicit-bypass flags are denied **unconditionally**, since the flag is itself the bypass and needs no branch query: `gh pr merge --admin` overrides the server-side merge gate, and `git commit`/`git push --no-verify` skips the local git hooks, as does a `git -c core.hooksPath=` override on either, which points git at another hooks directory, and an `include.path`/`includeIf` override on either, whose file may do the same. These are read in a command the hook parses, a wrapper shell's body among them, and not inside a quoted `eval` argument. Third, a **mutating git operation run directly against a primary checkout** (`checkout`/`switch`/`pull`/`reset`/`rebase`/`merge`/`cherry-pick`/`revert`/`restore`/`add`/`commit`/`rm`/`mv`/`apply`/`am`/`push`/`stash` (anything but `list`/`show`)/`clean -f`/`worktree remove -f`), the harm behind a second incident where an agent reused the maintainer's own primary checkout instead of a worktree despite having read the prose rule against it. "Primary" is decided by comparing `git rev-parse --git-dir` against `--git-common-dir`, not a `.git`-is-a-directory guess. `worktree add`/`list`/`prune`, an unforced `worktree remove`, `merge --ff-only`/`pull --ff-only`, and a `checkout `/`switch ` carrying no force flag (bundled or attached forms like `-qf`/`-Bname` recognized, not only an exact token) stay exempt even in a primary checkout, matching the documented base-clone cleanup step the `repo-worktree` skill already carries. A subcommand this rule does not recognize is resolved through a chain of git aliases, inline and persisted, before falling through, so a custom alias cannot hide a denied builtin behind an unrecognized name. A `!`-prefixed shell alias is denied outright rather than interpreted. A directory named with a `..` component is not resolved, so a mutating command naming one is denied. That holds for `-C`, any `cd`, `pushd`, `popd`, or `chdir` word in the command, `--work-tree`, `--git-dir`, and a `GIT_DIR=`/`GIT_WORK_TREE=` prefix. The target directory follows real git's own `-C`/`--work-tree`/`--git-dir` priority, not a last-option-wins scan. A different owner-style grant, `GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT`, follows the same channel as `GH_WRITE_GUARD_ALLOW`. Reads and everything else pass through. Fourth, an **unbounded shell wait**, which the root spec's requirement 7 defines in full rather than this bullet redefining it. In outline: a loop that sleeps, carrying no bound the rule recognizes. It is the shape an agent writes when a brief tells it to poll. It is also the harm behind a third incident, where seven such loops outlived the subagents that wrote them and the run that dispatched those subagents. A shell started by a tool call runs in a session of its own, so the agent could not have reaped it even had it tried. A nested loop is judged on its own terms, and a command that forks anything out of a `timeout`'s reach, by backgrounding it or by `setsid`, is bounded by nothing. A heredoc body is data rather than a command line unless a shell is what it is fed to, so a document quoting the forbidden shape is written rather than denied. It fires even in autonomous / bypass-permissions sessions, which is how the first incident happened. See the root spec's requirements 6 and 7 for the exact rules. - **`hooks/tool-containment.py`**: a shell prefix, registered as `CLAUDE_CODE_SHELL_PREFIX` in the `env` block of `settings.json`, that runs every Bash tool call inside a transient systemd user scope carrying `TasksMax`, `MemoryMax`, and `MemorySwapMax=0`, the last so a runaway at the memory ceiling is killed rather than spilled into the host's swap. A runaway fan-out then fails at the ceiling with `fork: Resource temporarily unavailable` instead of growing until the host has to be reset, the harm behind the probe-shim incident in the root spec's requirement 9. The guard above reads the command text, and that incident's recursion lived in a script file the command only named. The ceilings default to 8192 tasks and 25% of RAM. A `claude` session started from a tool call is nested in the session that ran it. Every scope of such a chain also joins one slice, `claude-tree-.slice`. That slice carries a total ceiling for the whole tree, 32768 tasks and 50% of RAM. The root's id reaches nested sessions as `AGENT_CONTAINMENT_ROOT`, which every contained command inherits. The tree's ceiling is a runtime drop-in put on when its slice is first created, and the sweep below reverts it. The maintainer raises any ceiling in the same `env` block. An inline `VAR=x` in a command cannot change one, since the prefix reads it before the command runs. The variables are `AGENT_CONTAINMENT_TASKS_MAX` and `AGENT_CONTAINMENT_MEMORY_MAX` for a scope, and `AGENT_CONTAINMENT_TREE_TASKS_MAX` and `AGENT_CONTAINMENT_TREE_MEMORY_MAX` for the tree. A tree ceiling set below a scope's binds first. Each scope is named `claude-tool---`, so the sweep below can stop one session's tool calls and nothing else. Its description records the pid of the agent that ran it. That record is how the sweep tells a scope whose session has ended from one whose session still runs. The installer sets the prefix only where it can contain, per the Linux bullet under "Scope and Limits", and never over a `CLAUDE_CODE_SHELL_PREFIX` value that names anything else. - **`hooks/stray-process-sweep.py`**: a SessionEnd hook in two phases. First, it stops every `claude-tool--*` scope this session left loaded and reports what each held, since the scope's name proves which session started it. It then stops every scope in this session's tree whose recorded agent is no longer running, and names each one's session. That is how a nested session's scopes end, since stopping the parent's scopes kills the nested `claude` before its own sweep runs. Each stop can end an agent nested deeper, so it judges again, up to four rounds. The root session then stops its tree's slice and reverts the ceiling put on it, once nothing is left in it. Second, it reports, and never ends, the process trees outside those scopes that outlive the session. The guard above stops a leak from being written. This finds the ones already running: the ones started before the guard was installed, and the shapes the guard deliberately does not reach. It reports every descendant of the agent process running outside the agent's own session. It names each subtree root's command and age, and hands over the `kill` line. This phase kills nothing itself. A long build backgrounded on purpose reads identically to a leaked wait from there. What is still wanted on the machine is the maintainer's call. A SessionEnd hook cannot block, and its JSON output is discarded, so stderr on exit 2 is the one channel that reaches the maintainer. The sweep uses that for a report rather than for a refusal. It registers under a group carrying no matcher, which fires on every exit reason. It also carries an explicit timeout, since the event's own budget is 1.5 seconds and only a declared per-hook timeout raises it. See the root spec's requirement 8. - **The whole of `CLAUDE.md`, rendered on every run**, loaded into every session on the machine (including ad-hoc work outside any project). It holds a header, two marker-delimited blocks of host-wide content, and the host-local file. Nothing else survives an install. Content outside the blocks was never written or checked, and it restated fleet rules in wording the fleet had since changed. @@ -21,7 +21,7 @@ Into `~/.claude/` (or `%USERPROFILE%\.claude\` on Windows): - A local file that cannot be read, is not a regular file, or is not UTF-8 stops the install before anything changes. So does one carrying this kit's own markers, since appending it would duplicate a block. - The `agent-safety` block carries restrictions alone, so nothing in it can widen a permission. `Repository Boundaries and Write Safety` mirrors the committed `GOVERNANCE.md` section of the same name, carrying as behavioral guidance its write rules and its rules that a task works in its own checkout and isolates into its own worktree before its first file edit. `Data in Agent-Authored Text` mirrors `GOVERNANCE.md` "Representative Data in Agent-Authored Text", so an agent illustrates with constructed data rather than data observed on the machine. `Authorization Scope and Memory Hygiene` bounds how a granted permission is recorded and read. Each mirrors a rule that otherwise reaches fleet repos only. - The `fleet-bootstrap` block carries `Fleet Governance Entry Point`, which names the template repository and routes by the state a repository is actually in. It is separate precisely because it enables rather than restricts, and the safety block's own text says nothing in it widens a permission, so merging the two would contradict that. It is host-wide rather than per repository because the repositories that most need it are the ones carrying no instruction set to point the way, and it mirrors the byte-locked `AGENTS.md` "Fleet Bootstrap" section that a conformant repository carries. -- **The permission rules this kit owns, merged into `settings.json`** beside the hook registration. Each is declared as a prefix and a rule, and a re-run drops every rule the prefix owns before adding the current one, so a rule whose spelling changes updates in place rather than accumulating beside the version it replaced. Ownership requires a rule-syntax delimiter after the prefix, since the prefix ends at the script name and a bare prefix test would also claim a longer path such as `pr_review.py-custom`, so a rule written by hand for a different script is never touched. These widen rather than restrict, which is why they are their own component for the same reason the `fleet-bootstrap` block is separate from the `agent-safety` one. Today the list holds one rule, for `scripts/pr_review.py`, which covers the review loop's `reply` and resolve and the `cleanup` delete of a merged pull request's head branch. Driving that loop by hand needs a raw GraphQL mutation carrying a node id, which is the shape that reached a stranger's repository, where the script queries the id itself and takes no argument an id fits in. The `:*` rule approves any arguments, so it approves a `cleanup` call whatever branch or pull request it names, and every check the delete makes lives inside the subcommand rather than in the rule. What the rule decides is which command runs without a prompt, and it matches the command text rather than the directory the command runs in, so it reaches a `scripts/pr_review.py` in any checkout that carries one. An absolute path would not narrow that, since the hub is reached as a checkout of the caller's own and its location differs per task, so pinning one path would name a checkout the next task does not use. What bounds it is the rule that an agent reaches the hub as a checkout of its own, fetched immediately before it is read, rather than a copy it happens to find on disk, which the `fleet-bootstrap` block beside this carries and [`GOVERNANCE.md`][governance] "Hub-Hosted Tooling" states in full. +- **The permission rules this kit owns, merged into `settings.json`** beside the hook registration. Each is declared as a prefix and a rule, and a re-run drops every rule the prefix owns before adding the current one, so a rule whose spelling changes updates in place rather than accumulating beside the version it replaced. Ownership requires a rule-syntax delimiter after the prefix, since the prefix ends at the script name and a bare prefix test would also claim a longer path such as `pr_review.py-custom`, so a rule written by hand for a different script is never touched. These widen rather than restrict, which is why they are their own component for the same reason the `fleet-bootstrap` block is separate from the `agent-safety` one. Today the list holds one rule, for `scripts/pr_review.py`, which covers the review loop's `reply` and resolve and the `cleanup` delete of a merged pull request's head branch. Driving that loop by hand needs a raw GraphQL mutation carrying a node id, which is the shape that reached a stranger's repository, where the script queries the id itself and takes no argument an id fits in. The `:*` rule approves any arguments, so it approves any `cleanup` call. Every check the delete makes therefore lives inside the subcommand rather than in the rule. What the rule decides is which command runs without a prompt, and it matches the command text rather than the directory the command runs in, so it reaches a `scripts/pr_review.py` in any checkout that carries one. An absolute path would not narrow that, since the hub is reached as a checkout of the caller's own and its location differs per task, so pinning one path would name a checkout the next task does not use. What bounds it is the rule that an agent reaches the hub as a checkout of its own, fetched immediately before it is read, rather than a copy it happens to find on disk, which the `fleet-bootstrap` block beside this carries and [`GOVERNANCE.md`][governance] "Hub-Hosted Tooling" states in full. See [`../README.md`][spec] "Requirements" for which of these each rule implements, and "Auditing an Implementation Against This Spec" for how to check this hook still satisfies them after a change. diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index e5f117189..bb1bef720 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -2229,7 +2229,7 @@ def _names_push(text): """True when `text` spells a git push as written, or once its quoting is removed. The shell joins `p''ush` into `push` before git reads it. The second reading matches `git` - without case, as `_is_git_exe` does, since a case-insensitive shell runs `GIT` as git. + without case, as `_is_git_exe` does, since a case-insensitive filesystem resolves `GIT` to git. """ return bool(_GIT_PUSH.search(text) or _GIT_PUSH_UNQUOTED.search(_unquoted(text))) @@ -2371,35 +2371,46 @@ def _unreadable_push(cmd): _GIT_DIR_VALUE_OPTS = frozenset({"-C", "--git-dir", "--work-tree"}) -_COMMIT_VALUE_OPTS = frozenset( - {"-m", "--message", "-F", "--file", "-C", "--reuse-message", "-c", "--reedit-message"} - | {"--author", "--date", "-t", "--template", "--cleanup", "--fixup", "--squash", "--trailer"} +_COMMIT_LONG_VALUE_OPTS = frozenset( + {"--message", "--file", "--reuse-message", "--reedit-message", "--author", "--date"} + | {"--template", "--cleanup", "--fixup", "--squash", "--trailer", "--pathspec-from-file"} ) +_COMMIT_VALUE_OPTS = _COMMIT_LONG_VALUE_OPTS | {"-m", "-F", "-C", "-c", "-t"} _COMMIT_VALUE_LETTERS = "mFCct" +_BRACE_EXPANSION = re.compile(r"\{[^{}'\"\s]*(?:,|\.\.)[^{}'\"\s]*\}") def _is_run_time_word(tok): - """True when the shell may rewrite a token at run time, a `$` or a backtick in it or a lone parenthesis run.""" - return "$" in tok or "`" in tok or (tok != "" and set(tok) <= {"(", ")"}) + """True when the shell may rewrite a token at run time. + + That is a `$` or a backtick in it, a brace expansion such as `{a,b}`, or a lone parenthesis run. + """ + return ( + "$" in tok + or "`" in tok + or (tok != "" and set(tok) <= {"(", ")"}) + or bool(_BRACE_EXPANSION.search(tok)) + ) def _region_end(toks, k): """The index of the separator after `toks[k]` that ends its command. - A parenthesis the command opens, as a `$(...)` in one of its words does, is read through to the - one closing it, separators inside included. A closing one it did not open ends the command, as - a subshell's does. + A parenthesis the command opens, as a `$(...)` or a `<(...)` in one of its words does, is read + through to the one closing it, separators inside included. A closing one it did not open ends + the command, as a subshell's does. """ n = len(toks) depth = 0 e = k + 1 while e < n: t = toks[e] - if _is_separator(t): - if depth == 0 and t[0] not in "()": + if _is_shell_op(t): + sep = _is_separator(t) + if sep and depth == 0 and t[0] not in "()": break depth += t.count("(") - t.count(")") - if depth < 0 or (depth == 0 and not set(t) <= {"(", ")"}): + if depth < 0 or (sep and depth == 0 and not set(t) <= {"(", ")"}): break e += 1 return e @@ -2430,9 +2441,15 @@ def _span_end(toks, i, end): def _is_commit_value_option(tok): - """True when a commit argument takes the next word as its value, as `-m` or a cluster such as `-am` does.""" + """True when a commit argument takes the next word as its value. + + That is `-m`, a cluster such as `-am`, or a long value option or an abbreviation of one, since + git reads `--mess` as `--message` and takes a following `--` as its value. + """ if tok in _COMMIT_VALUE_OPTS: return True + if tok.startswith("--") and len(tok) > 2 and "=" not in tok: + return any(o.startswith(tok) for o in _COMMIT_LONG_VALUE_OPTS) return ( len(tok) > 1 and tok[0] == "-" @@ -2455,10 +2472,11 @@ def _holds_attached_value(tok): def _expands_unquoted(raw): - """True when a token as written holds a `$` or a backtick outside double quotes. + """True when a token as written holds a `$`, a backtick, or a brace expansion outside double quotes. - Word splitting may split what such an expansion gives into more arguments. One inside a - `$(...)` that double quotes hold is still inside those quotes. + Word splitting may split what such an expansion gives into more arguments, and a brace + expansion gives several words itself. One inside a `$(...)` that double quotes hold is still + inside those quotes. """ stack = [] i = 0 @@ -2482,7 +2500,7 @@ def _expands_unquoted(raw): stack.append(ch) elif (top == "(" and ch == ")") or (top == "`" and ch == "`"): stack.pop() - elif ch in "$`" and '"' not in stack: + elif '"' not in stack and (ch in "$`" or (ch == "{" and _BRACE_EXPANSION.match(raw, i))): return True elif raw.startswith("$(", i): stack.append("(") @@ -2496,41 +2514,46 @@ def _expands_unquoted(raw): def _split_free(x, toks, mask, raw): """True when word splitting cannot split `toks[x]`, read from the quote-keeping token where known. - Where that token is unknown, a token opening with `$(` and holding more is read as quoted. The - operator lex and `_context_lex` each split an unquoted `$(` at its parenthesis, and the token - union `_shell_tokens` returns then holds a reading that denies it. + Where that token is unknown, a token holding an expansion is split-free only where it opens + with `$(` and ends with the `)` closing it. The operator lex and `_context_lex` each split an + unquoted `$(` at its parenthesis, and the token union `_shell_tokens` returns then holds a + reading that denies it. Any other spelling, such as `"a"$Y`, is read as one that may split. """ if raw is not None: return not _expands_unquoted(raw[x]) tok = toks[x] - return ( - bool(mask and mask[x]) - or not _expands_unquoted(tok) - or (len(tok) > 2 and tok.startswith("$(")) + return not _expands_unquoted(tok) or ( + len(tok) > 3 and tok.startswith("$(") and tok.endswith(")") ) def _rewritten_git_words(cmd): - """Name the form when a git push or commit holds a word the shell rewrites or drops, else None. + """Name the form when a git command holds a word the shell rewrites or drops, else None. - The guard reads each word as written, while the shell expands a `$` or a backtick word and drops - a comment before git reads any of them. Modeling either is open-ended, so the command fails closed. + The guard reads each word as written, while the shell expands a `$`, a backtick, or a brace + word and drops a comment before git reads any of them. Modeling either is open-ended, so the + command fails closed. - a run-time word where the subcommand goes, on any git command, since it may expand to `push` or to an option that moves the subcommand, as a backtick word ending the global options does + - a run-time word that word splitting may split among the options of a `git` with no + subcommand word, since `git -c $X` may expand to a commit under an override - a run-time word among a push's or a commit's global options, since `-c "$X"` may set `core.hooksPath` - a run-time word among a push's arguments, since it may name any refspec or remote - a run-time word among a commit's arguments, since it may expand to `--no-verify` + - a redirection target spanning several tokens on a push or a commit, since the push and + bypass readings stop at its parenthesis and every later word goes unread - a comment among a push's words, since the shell drops what the guard reads as a refspec or a flag there, which can turn a bare push into one naming another branch A comment on a commit only adds words the guard reads, which can only add a deny, so it is left alone. Four kinds of run-time word are left alone too. One is the value of a global option - naming a directory, unquoted too, since `-C $dir` is the common spelling, and what word - splitting makes of an unquoted one is not read. One is a commit option's value - whose expansions sit inside double quotes, since word splitting cannot then make it an option, - as in `-m "$(cat <<'EOF' ...)"`. One is a commit path after `--`. One is a redirection target. + naming a directory before a subcommand word, unquoted too, since `-C $dir` is the common + spelling, and what word splitting makes of an unquoted one is not read. One is a commit + option's value whose expansions sit inside double quotes, since word splitting cannot then make + it an option, as in `-m "$(cat <<'EOF' ...)"`. One is a commit path after `--`. One is a + redirection target held in one token, such as `2>"$LOG"`. Only a git word in a command's position counts, so a `grep git "$dir"` is no git command, and that test is `_runs_as_command`'s. A git word right after an unquoted `(` counts too, since a process substitution such as `<(git ...)` runs it whatever command reads the result. @@ -2544,13 +2567,20 @@ def _rewritten_git_words(cmd): continue end = _region_end(toks, k) j = min(_git_subcommand_index(toks, k), end) - if j >= end: + if j == k + 1 and j >= end: continue if not masked: mask, raw, masked = _quoted_mask(text, toks), _quote_kept_tokens(text, toks), True opened = k and toks[k - 1].endswith("(") and not (mask and mask[k - 1]) if not opened and not _runs_as_command(toks, k, mask): continue + if j >= end: + if any( + _is_run_time_word(toks[x]) and not _split_free(x, toks, mask, raw) + for x in range(k + 1, end) + ): + return "a run-time word among the options of a `git` with no subcommand word" + continue sub = toks[j] if _is_run_time_word(sub): return f"a run-time word, `{sub}`, where the subcommand of `git` goes" @@ -2569,15 +2599,19 @@ def _rewritten_git_words(cmd): if not opt.startswith(("--git-dir=", "--work-tree=")) and _is_run_time_word(opt): return f"a run-time word, `{opt}`, among the global options of `git {sub}`" x += 1 - x = j + 1 after_dashes = False - while x is not None and x < end: + while x < end: arg = toks[x] if _is_redir_op(arg): x += 1 if x < end and not _is_shell_op(toks[x]): - x = _span_end(toks, x, end) + if _span_end(toks, x, end) != x + 1: + return ( + f"a redirection target spanning several words among the words of " + f"`git {sub}`" + ) + x += 1 continue if sub == "commit": after_dashes = after_dashes or arg == "--" @@ -2597,8 +2631,6 @@ def _rewritten_git_words(cmd): if _is_run_time_word(arg): return f"a run-time word, `{arg}`, among the arguments of `git {sub}`" x += 1 - if x is None: - return f"a redirection target that does not close among the words of `git {sub}`" return None @@ -4455,7 +4487,7 @@ def classify( dec, reason = _check_bypass_flags(cmd) if dec == "deny": return dec, reason - # A word the shell rewrites or drops before git reads it hides what a push targets or what a commit skips, so it fails closed beside the readings below. + # A word the shell rewrites before git reads it can hide a push target or a commit bypass, so it fails closed. rewritten = _rewritten_git_words(cmd) if rewritten: return "deny", ( @@ -5986,7 +6018,7 @@ def classify( None, {"develop": _CODE_RULES}, "deny", - "an uppercase GIT reaches the push check, as a case-insensitive shell runs it as git", + "an uppercase GIT reaches the push check, as a case-insensitive filesystem resolves it to git", ), ( "git -c 'inc''lude.path=/example/config' p", @@ -6140,7 +6172,63 @@ def classify( None, {"feature/x": set()}, "allow", - "control: a run-time redirection target is not a refspec", + "control: a run-time word in a command the push pipes into is not a refspec", + ), + ( + "Y=' --no-veri'fy; git commit -m \"a\"$Y", + "feature/x", + {"feature/x": set()}, + "deny", + "an unquoted expansion after a quoted part of a commit message may split into more options", + ), + ( + "X='core.hooksPath=/dev/null commit -m x'; git -c $X", + "feature/x", + {"feature/x": set()}, + "deny", + "an unquoted option value on a git with no subcommand word may split into a commit", + ), + ( + "git commit --mess -- $(printf -- --no-veri%s fy)", + "feature/x", + {"feature/x": set()}, + "deny", + "an abbreviated value option takes a following -- as its value, so later words are options", + ), + ( + "git {push,origin} develop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a brace expansion where the subcommand goes may expand to push", + ), + ( + "git commit -m x {--no-verify,--quiet}", + "feature/x", + {"feature/x": set()}, + "deny", + "a brace expansion among a commit's arguments may expand to --no-verify", + ), + ( + "git push origin feature/x > $(printf a) develop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a redirection target spanning several words hides the push's later refspecs", + ), + ( + "git push origin feature/x < <(printf a) develop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a process substitution redirection hides the push's later refspecs too", + ), + ( + 'git push origin feature/x 2>"$LOG"', + None, + {"feature/x": set()}, + "allow", + "control: a run-time redirection target held in one word is not a refspec", ), ( "(git push origin feature/x)", From e0680b17c705ac7cf5bfac7dd60d88fe17aa37cd Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 13 Oct 2026 21:56:15 -0700 Subject: [PATCH 3/8] Read Quoted Braces and Any Git Command's Global Options in the Shell-Rewrite Deny A brace expansion counts wherever its braces and separator are unquoted, whatever quoted parts sit between them. On any git command, a run-time global option word that word splitting may split denies, and so does a brace expansion in a directory option's value. A redirection where the subcommand goes denies. The README claims now name only where the deny applies. Co-Authored-By: Claude Opus 5.5 --- host-setup/agent-safety/README.md | 12 +-- host-setup/agent-safety/claude/README.md | 2 +- .../agent-safety/claude/gh-write-guard.py | 86 +++++++++++++++++-- 3 files changed, 87 insertions(+), 13 deletions(-) diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md index 67d5e605a..4f55bfa40 100644 --- a/host-setup/agent-safety/README.md +++ b/host-setup/agent-safety/README.md @@ -80,11 +80,13 @@ text says, because the harm it covers was never in the text. `HEAD`/`@` refspec whose directory or branch may change before it runs. Such a push's local branch is checked under its whole name, such as `release/1.0`. It is also checked with its first segment dropped. A remote-tracking push target such as `origin/feature/x` is checked with its - remote dropped. A git command holding a word the shell rewrites or drops before git reads it - is denied too. That word is a `$`, backtick, or brace-expansion word. It is denied where any - git command's subcommand goes, and among a push's words or a commit's. A redirection target - spanning several words on either is denied too. So is a comment among a push's words. Some - words are read as they stand. They are a directory option's value, a commit option's + remote dropped. A word the shell rewrites or drops before git reads it is denied where it can + change what git runs. Such a word is a `$`, backtick, or brace-expansion word. It is denied + where any git command's subcommand goes, and among a push's words or a commit's. On any git + command it is denied among the global options where word splitting may split it. A + redirection where the subcommand goes, or a redirection target spanning several words on a + push or a commit, is denied too. So is a comment among a push's words. Some words are read as + they stand. They are a directory option's value short of a brace expansion, a commit option's double-quoted value, a commit path after `--`, and a redirection target held in one word. The push and include checks also read each command with its quotes and backslashes removed and `git` in any case, so a `p''ush` or an uppercase `GIT` reaches them. The diff --git a/host-setup/agent-safety/claude/README.md b/host-setup/agent-safety/claude/README.md index 95c2551ea..94dd84409 100644 --- a/host-setup/agent-safety/claude/README.md +++ b/host-setup/agent-safety/claude/README.md @@ -12,7 +12,7 @@ installation and operational detail. Codex and opencode have no equivalent hook Into `~/.claude/` (or `%USERPROFILE%\.claude\` on Windows): -- **`hooks/gh-write-guard.py`**: a PreToolUse hook that denies four classes of dangerous action. First, the GitHub **write** footguns behind the cross-repo comment incident: a state-changing `gh` call whose output is discarded, a GraphQL mutation passing a **literal** node id instead of a `$variable`, and a `gh` write whose explicit target is under an owner other than the checkout origin's. Sibling repositories under the same owner are allowed, since the harm this guards is reaching a stranger's repository rather than working across one maintainer's own fleet, and a different owner is allowed only when the maintainer names it in `GH_WRITE_GUARD_ALLOW` (an `owner/repo` list, where `owner/*` grants a whole owner). That variable is read from the environment the session was launched with, which is the one channel an agent cannot set for itself: a hook runs as its own process, so an inline `VAR=x cmd` prefix or an `export` inside a Bash call never reaches it. Second, a **git operation that bypasses a repository protection**. The branch-rule cases (a direct push, a force-push, or a branch delete to a branch that requires a pull request, a force-push where history is protected, a branch delete where deletion is blocked) are judged per refspec against the branch's **live** rules, so a code-style `develop` is denied while a config-style `develop` is allowed with no per-repo configuration. A push to a protected-default branch fails closed when its rules cannot be determined, either because the API is unreachable or because the checkout's origin cannot be resolved to query them. A push whose target the hook cannot read, such as one in text it does not parse, one carrying a `-c` config override, a git command an `include.path`/`includeIf` override may define as a push alias, or a git command that runs other git commands under one, is denied whatever branch it names. So is a git command holding a word the shell rewrites or drops before git reads it. Such a word is a `$`, backtick, or brace-expansion refspec or subcommand, or a comment among a push's words. The explicit-bypass flags are denied **unconditionally**, since the flag is itself the bypass and needs no branch query: `gh pr merge --admin` overrides the server-side merge gate, and `git commit`/`git push --no-verify` skips the local git hooks, as does a `git -c core.hooksPath=` override on either, which points git at another hooks directory, and an `include.path`/`includeIf` override on either, whose file may do the same. These are read in a command the hook parses, a wrapper shell's body among them, and not inside a quoted `eval` argument. Third, a **mutating git operation run directly against a primary checkout** (`checkout`/`switch`/`pull`/`reset`/`rebase`/`merge`/`cherry-pick`/`revert`/`restore`/`add`/`commit`/`rm`/`mv`/`apply`/`am`/`push`/`stash` (anything but `list`/`show`)/`clean -f`/`worktree remove -f`), the harm behind a second incident where an agent reused the maintainer's own primary checkout instead of a worktree despite having read the prose rule against it. "Primary" is decided by comparing `git rev-parse --git-dir` against `--git-common-dir`, not a `.git`-is-a-directory guess. `worktree add`/`list`/`prune`, an unforced `worktree remove`, `merge --ff-only`/`pull --ff-only`, and a `checkout `/`switch ` carrying no force flag (bundled or attached forms like `-qf`/`-Bname` recognized, not only an exact token) stay exempt even in a primary checkout, matching the documented base-clone cleanup step the `repo-worktree` skill already carries. A subcommand this rule does not recognize is resolved through a chain of git aliases, inline and persisted, before falling through, so a custom alias cannot hide a denied builtin behind an unrecognized name. A `!`-prefixed shell alias is denied outright rather than interpreted. A directory named with a `..` component is not resolved, so a mutating command naming one is denied. That holds for `-C`, any `cd`, `pushd`, `popd`, or `chdir` word in the command, `--work-tree`, `--git-dir`, and a `GIT_DIR=`/`GIT_WORK_TREE=` prefix. The target directory follows real git's own `-C`/`--work-tree`/`--git-dir` priority, not a last-option-wins scan. A different owner-style grant, `GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT`, follows the same channel as `GH_WRITE_GUARD_ALLOW`. Reads and everything else pass through. Fourth, an **unbounded shell wait**, which the root spec's requirement 7 defines in full rather than this bullet redefining it. In outline: a loop that sleeps, carrying no bound the rule recognizes. It is the shape an agent writes when a brief tells it to poll. It is also the harm behind a third incident, where seven such loops outlived the subagents that wrote them and the run that dispatched those subagents. A shell started by a tool call runs in a session of its own, so the agent could not have reaped it even had it tried. A nested loop is judged on its own terms, and a command that forks anything out of a `timeout`'s reach, by backgrounding it or by `setsid`, is bounded by nothing. A heredoc body is data rather than a command line unless a shell is what it is fed to, so a document quoting the forbidden shape is written rather than denied. It fires even in autonomous / bypass-permissions sessions, which is how the first incident happened. See the root spec's requirements 6 and 7 for the exact rules. +- **`hooks/gh-write-guard.py`**: a PreToolUse hook that denies four classes of dangerous action. First, the GitHub **write** footguns behind the cross-repo comment incident: a state-changing `gh` call whose output is discarded, a GraphQL mutation passing a **literal** node id instead of a `$variable`, and a `gh` write whose explicit target is under an owner other than the checkout origin's. Sibling repositories under the same owner are allowed, since the harm this guards is reaching a stranger's repository rather than working across one maintainer's own fleet, and a different owner is allowed only when the maintainer names it in `GH_WRITE_GUARD_ALLOW` (an `owner/repo` list, where `owner/*` grants a whole owner). That variable is read from the environment the session was launched with, which is the one channel an agent cannot set for itself: a hook runs as its own process, so an inline `VAR=x cmd` prefix or an `export` inside a Bash call never reaches it. Second, a **git operation that bypasses a repository protection**. The branch-rule cases (a direct push, a force-push, or a branch delete to a branch that requires a pull request, a force-push where history is protected, a branch delete where deletion is blocked) are judged per refspec against the branch's **live** rules, so a code-style `develop` is denied while a config-style `develop` is allowed with no per-repo configuration. A push to a protected-default branch fails closed when its rules cannot be determined, either because the API is unreachable or because the checkout's origin cannot be resolved to query them. A push whose target the hook cannot read, such as one in text it does not parse, one carrying a `-c` config override, a git command an `include.path`/`includeIf` override may define as a push alias, or a git command that runs other git commands under one, is denied whatever branch it names. So is a word the shell rewrites or drops before git reads it, where it can change what git runs. Such a word is a `$`, backtick, or brace-expansion refspec, subcommand, or commit argument, or a comment among a push's words. The explicit-bypass flags are denied **unconditionally**, since the flag is itself the bypass and needs no branch query: `gh pr merge --admin` overrides the server-side merge gate, and `git commit`/`git push --no-verify` skips the local git hooks, as does a `git -c core.hooksPath=` override on either, which points git at another hooks directory, and an `include.path`/`includeIf` override on either, whose file may do the same. These are read in a command the hook parses, a wrapper shell's body among them, and not inside a quoted `eval` argument. Third, a **mutating git operation run directly against a primary checkout** (`checkout`/`switch`/`pull`/`reset`/`rebase`/`merge`/`cherry-pick`/`revert`/`restore`/`add`/`commit`/`rm`/`mv`/`apply`/`am`/`push`/`stash` (anything but `list`/`show`)/`clean -f`/`worktree remove -f`), the harm behind a second incident where an agent reused the maintainer's own primary checkout instead of a worktree despite having read the prose rule against it. "Primary" is decided by comparing `git rev-parse --git-dir` against `--git-common-dir`, not a `.git`-is-a-directory guess. `worktree add`/`list`/`prune`, an unforced `worktree remove`, `merge --ff-only`/`pull --ff-only`, and a `checkout `/`switch ` carrying no force flag (bundled or attached forms like `-qf`/`-Bname` recognized, not only an exact token) stay exempt even in a primary checkout, matching the documented base-clone cleanup step the `repo-worktree` skill already carries. A subcommand this rule does not recognize is resolved through a chain of git aliases, inline and persisted, before falling through, so a custom alias cannot hide a denied builtin behind an unrecognized name. A `!`-prefixed shell alias is denied outright rather than interpreted. A directory named with a `..` component is not resolved, so a mutating command naming one is denied. That holds for `-C`, any `cd`, `pushd`, `popd`, or `chdir` word in the command, `--work-tree`, `--git-dir`, and a `GIT_DIR=`/`GIT_WORK_TREE=` prefix. The target directory follows real git's own `-C`/`--work-tree`/`--git-dir` priority, not a last-option-wins scan. A different owner-style grant, `GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT`, follows the same channel as `GH_WRITE_GUARD_ALLOW`. Reads and everything else pass through. Fourth, an **unbounded shell wait**, which the root spec's requirement 7 defines in full rather than this bullet redefining it. In outline: a loop that sleeps, carrying no bound the rule recognizes. It is the shape an agent writes when a brief tells it to poll. It is also the harm behind a third incident, where seven such loops outlived the subagents that wrote them and the run that dispatched those subagents. A shell started by a tool call runs in a session of its own, so the agent could not have reaped it even had it tried. A nested loop is judged on its own terms, and a command that forks anything out of a `timeout`'s reach, by backgrounding it or by `setsid`, is bounded by nothing. A heredoc body is data rather than a command line unless a shell is what it is fed to, so a document quoting the forbidden shape is written rather than denied. It fires even in autonomous / bypass-permissions sessions, which is how the first incident happened. See the root spec's requirements 6 and 7 for the exact rules. - **`hooks/tool-containment.py`**: a shell prefix, registered as `CLAUDE_CODE_SHELL_PREFIX` in the `env` block of `settings.json`, that runs every Bash tool call inside a transient systemd user scope carrying `TasksMax`, `MemoryMax`, and `MemorySwapMax=0`, the last so a runaway at the memory ceiling is killed rather than spilled into the host's swap. A runaway fan-out then fails at the ceiling with `fork: Resource temporarily unavailable` instead of growing until the host has to be reset, the harm behind the probe-shim incident in the root spec's requirement 9. The guard above reads the command text, and that incident's recursion lived in a script file the command only named. The ceilings default to 8192 tasks and 25% of RAM. A `claude` session started from a tool call is nested in the session that ran it. Every scope of such a chain also joins one slice, `claude-tree-.slice`. That slice carries a total ceiling for the whole tree, 32768 tasks and 50% of RAM. The root's id reaches nested sessions as `AGENT_CONTAINMENT_ROOT`, which every contained command inherits. The tree's ceiling is a runtime drop-in put on when its slice is first created, and the sweep below reverts it. The maintainer raises any ceiling in the same `env` block. An inline `VAR=x` in a command cannot change one, since the prefix reads it before the command runs. The variables are `AGENT_CONTAINMENT_TASKS_MAX` and `AGENT_CONTAINMENT_MEMORY_MAX` for a scope, and `AGENT_CONTAINMENT_TREE_TASKS_MAX` and `AGENT_CONTAINMENT_TREE_MEMORY_MAX` for the tree. A tree ceiling set below a scope's binds first. Each scope is named `claude-tool---`, so the sweep below can stop one session's tool calls and nothing else. Its description records the pid of the agent that ran it. That record is how the sweep tells a scope whose session has ended from one whose session still runs. The installer sets the prefix only where it can contain, per the Linux bullet under "Scope and Limits", and never over a `CLAUDE_CODE_SHELL_PREFIX` value that names anything else. - **`hooks/stray-process-sweep.py`**: a SessionEnd hook in two phases. First, it stops every `claude-tool--*` scope this session left loaded and reports what each held, since the scope's name proves which session started it. It then stops every scope in this session's tree whose recorded agent is no longer running, and names each one's session. That is how a nested session's scopes end, since stopping the parent's scopes kills the nested `claude` before its own sweep runs. Each stop can end an agent nested deeper, so it judges again, up to four rounds. The root session then stops its tree's slice and reverts the ceiling put on it, once nothing is left in it. Second, it reports, and never ends, the process trees outside those scopes that outlive the session. The guard above stops a leak from being written. This finds the ones already running: the ones started before the guard was installed, and the shapes the guard deliberately does not reach. It reports every descendant of the agent process running outside the agent's own session. It names each subtree root's command and age, and hands over the `kill` line. This phase kills nothing itself. A long build backgrounded on purpose reads identically to a leaked wait from there. What is still wanted on the machine is the maintainer's call. A SessionEnd hook cannot block, and its JSON output is discarded, so stderr on exit 2 is the one channel that reaches the maintainer. The sweep uses that for a report rather than for a refusal. It registers under a group carrying no matcher, which fires on every exit reason. It also carries an explicit timeout, since the event's own budget is 1.5 seconds and only a declared per-hook timeout raises it. See the root spec's requirement 8. - **The whole of `CLAUDE.md`, rendered on every run**, loaded into every session on the machine (including ad-hoc work outside any project). It holds a header, two marker-delimited blocks of host-wide content, and the host-local file. Nothing else survives an install. Content outside the blocks was never written or checked, and it restated fleet rules in wording the fleet had since changed. diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index bb1bef720..92e3e2e84 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -2471,18 +2471,20 @@ def _holds_attached_value(tok): return len(literal) >= 2 and literal[0] == "-" and literal[1] in _COMMIT_VALUE_LETTERS -def _expands_unquoted(raw): +def _expands_unquoted(raw, braces_only=False): """True when a token as written holds a `$`, a backtick, or a brace expansion outside double quotes. Word splitting may split what such an expansion gives into more arguments, and a brace expansion gives several words itself. One inside a `$(...)` that double quotes hold is still - inside those quotes. + inside those quotes. A brace expands where its `{`, its `,` or `..`, and its `}` are unquoted, + whatever quoted parts sit between them. With `braces_only`, only a brace expansion counts. """ stack = [] i = 0 while i < len(raw): ch = raw[i] top = stack[-1] if stack else None + unquoted = '"' not in stack if top == "'": if ch == "'": stack.pop() @@ -2500,7 +2502,14 @@ def _expands_unquoted(raw): stack.append(ch) elif (top == "(" and ch == ")") or (top == "`" and ch == "`"): stack.pop() - elif '"' not in stack and (ch in "$`" or (ch == "{" and _BRACE_EXPANSION.match(raw, i))): + elif top in ("{", "{,") and ch == "}": + if stack.pop() == "{," and unquoted: + return True + elif top == "{" and (ch == "," or raw.startswith("..", i)): + stack[-1] = "{," + elif ch == "{" and raw[i - 1 : i] != "$": + stack.append("{") + elif unquoted and not braces_only and ch in "$`": return True elif raw.startswith("$(", i): stack.append("(") @@ -2528,7 +2537,7 @@ def _split_free(x, toks, mask, raw): def _rewritten_git_words(cmd): - """Name the form when a git command holds a word the shell rewrites or drops, else None. + """Name the form when a git command holds a rewritten word where it can change what git runs, else None. The guard reads each word as written, while the shell expands a `$`, a backtick, or a brace word and drops a comment before git reads any of them. Modeling either is open-ended, so the @@ -2536,8 +2545,10 @@ def _rewritten_git_words(cmd): - a run-time word where the subcommand goes, on any git command, since it may expand to `push` or to an option that moves the subcommand, as a backtick word ending the global options does - - a run-time word that word splitting may split among the options of a `git` with no - subcommand word, since `git -c $X` may expand to a commit under an override + - a run-time word that word splitting may split among any git command's global options, or + before no subcommand word at all, since `git -c $X README.md` may expand to a commit under an + override, and a brace expansion in a directory option's value + - a redirection where the subcommand goes, since the readings below then see no subcommand - a run-time word among a push's or a commit's global options, since `-c "$X"` may set `core.hooksPath` - a run-time word among a push's arguments, since it may name any refspec or remote @@ -2550,7 +2561,8 @@ def _rewritten_git_words(cmd): A comment on a commit only adds words the guard reads, which can only add a deny, so it is left alone. Four kinds of run-time word are left alone too. One is the value of a global option naming a directory before a subcommand word, unquoted too, since `-C $dir` is the common - spelling, and what word splitting makes of an unquoted one is not read. One is a commit + spelling. What word splitting makes of an unquoted one is not read, and a brace expansion in + one is denied. One is a commit option's value whose expansions sit inside double quotes, since word splitting cannot then make it an option, as in `-m "$(cat <<'EOF' ...)"`. One is a commit path after `--`. One is a redirection target held in one token, such as `2>"$LOG"`. @@ -2584,6 +2596,24 @@ def _rewritten_git_words(cmd): sub = toks[j] if _is_run_time_word(sub): return f"a run-time word, `{sub}`, where the subcommand of `git` goes" + if _is_redir_op(sub): + return "a redirection where the subcommand of `git` goes" + for x in range(k + 1, j): + word = toks[x] + if not _is_run_time_word(word): + continue + if toks[x - 1] in _GIT_DIR_VALUE_OPTS or word.startswith( + ("--git-dir=", "--work-tree=") + ): + if ( + _expands_unquoted(raw[x], braces_only=True) + if raw is not None + else _BRACE_EXPANSION.search(word) + ): + return f"a brace expansion, `{word}`, among the global options of `git`" + continue + if not _split_free(x, toks, mask, raw): + return f"a run-time word, `{word}`, that may split among the global options of `git`" if sub not in ("push", "commit"): continue if sub == "push" and any( @@ -6230,6 +6260,48 @@ def classify( "allow", "control: a run-time redirection target held in one word is not a refspec", ), + ( + "git commit -m {x,'--no-veri'fy}", + "feature/x", + {"feature/x": set()}, + "deny", + "a brace expansion holding a quoted part in a commit value still gives more words", + ), + ( + "X='core.hooksPath=/dev/null commit -m x --'; git -c $X README.md", + "feature/x", + {"feature/x": set()}, + "deny", + "an unquoted config value may split into the subcommand, whatever literal word follows", + ), + ( + "git -c {core.hooksPath=/dev/null,commit} -m x", + "feature/x", + {"feature/x": set()}, + "deny", + "a brace expansion in a config value may give the subcommand", + ), + ( + "git -C {.,push} origin develop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a brace expansion in a directory value may give the subcommand", + ), + ( + "git -c core.hooksPath=/dev/null > a commit -m x", + "feature/x", + {"feature/x": set()}, + "deny", + "a redirection where the subcommand goes hides a commit under an override", + ), + ( + 'git -C "$W" -c user.name="$N" status', + "feature/x", + {"feature/x": set()}, + "allow", + "control: quoted run-time values before a read-only subcommand cannot split", + ), ( "(git push origin feature/x)", None, From ea6acc5866794569f673db8b865a51f9188ced28 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 13 Oct 2026 22:09:16 -0700 Subject: [PATCH 4/8] Read a Numbered Redirection Where the Subcommand Goes in the Write Guard A file-descriptor number before a redirection where git's subcommand goes, as in `git 2>/dev/null push`, now denies like an unnumbered one. A redirection after an option that ends git first, such as `--version`, stays allowed. Co-Authored-By: Claude Opus 5.5 --- host-setup/agent-safety/README.md | 5 ++-- .../agent-safety/claude/gh-write-guard.py | 30 +++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md index 4f55bfa40..c33ef24ff 100644 --- a/host-setup/agent-safety/README.md +++ b/host-setup/agent-safety/README.md @@ -84,8 +84,9 @@ text says, because the harm it covers was never in the text. change what git runs. Such a word is a `$`, backtick, or brace-expansion word. It is denied where any git command's subcommand goes, and among a push's words or a commit's. On any git command it is denied among the global options where word splitting may split it. A - redirection where the subcommand goes, or a redirection target spanning several words on a - push or a commit, is denied too. So is a comment among a push's words. Some words are read as + redirection where the subcommand goes, a numbered one such as `2>` included, is denied too, + short of one after an option such as `--version` that ends git first. So is a redirection + target spanning several words on a push or a commit. So is a comment among a push's words. Some words are read as they stand. They are a directory option's value short of a brace expansion, a commit option's double-quoted value, a commit path after `--`, and a redirection target held in one word. The push and include checks also read each command with its quotes and backslashes removed and diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index 92e3e2e84..6be584ae2 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -2377,6 +2377,7 @@ def _unreadable_push(cmd): ) _COMMIT_VALUE_OPTS = _COMMIT_LONG_VALUE_OPTS | {"-m", "-F", "-C", "-c", "-t"} _COMMIT_VALUE_LETTERS = "mFCct" +_GIT_TERMINAL_OPTS = frozenset({"--version", "-v", "--help", "-h", "--html-path", "--man-path"}) _BRACE_EXPANSION = re.compile(r"\{[^{}'\"\s]*(?:,|\.\.)[^{}'\"\s]*\}") @@ -2548,7 +2549,8 @@ def _rewritten_git_words(cmd): - a run-time word that word splitting may split among any git command's global options, or before no subcommand word at all, since `git -c $X README.md` may expand to a commit under an override, and a brace expansion in a directory option's value - - a redirection where the subcommand goes, since the readings below then see no subcommand + - a redirection where the subcommand goes, a file-descriptor number before it included, since + the readings below then see no subcommand, unless an option such as `--version` ends git first - a run-time word among a push's or a commit's global options, since `-c "$X"` may set `core.hooksPath` - a run-time word among a push's arguments, since it may name any refspec or remote @@ -2596,7 +2598,10 @@ def _rewritten_git_words(cmd): sub = toks[j] if _is_run_time_word(sub): return f"a run-time word, `{sub}`, where the subcommand of `git` goes" - if _is_redir_op(sub): + redirected = _is_redir_op(sub) or ( + sub.isdigit() and j + 1 < end and _is_redir_op(toks[j + 1]) + ) + if redirected and not any(toks[x] in _GIT_TERMINAL_OPTS for x in range(k + 1, j)): return "a redirection where the subcommand of `git` goes" for x in range(k + 1, j): word = toks[x] @@ -6302,6 +6307,27 @@ def classify( "allow", "control: quoted run-time values before a read-only subcommand cannot split", ), + ( + "git 2>/dev/null push origin develop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a numbered redirection where the subcommand goes hides a push", + ), + ( + "git -c core.hooksPath=/dev/null 2>/dev/null commit -m x", + "feature/x", + {"feature/x": set()}, + "deny", + "a numbered redirection where the subcommand goes hides a commit under an override", + ), + ( + "git --version >/dev/null 2>&1", + "feature/x", + {"feature/x": set()}, + "allow", + "control: a redirection after --version, which ends git before any subcommand", + ), ( "(git push origin feature/x)", None, From 6d6ddb6a97879d4d6b528c423d6f81b88b6c6c14 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 13 Oct 2026 22:20:46 -0700 Subject: [PATCH 5/8] Honor a Terminal Option Only as Git's First Word in the Write Guard A terminal option such as `--version` exempts a redirection where the subcommand goes only where it is git's first word, since another option may take it as its value. A `{name}` file descriptor before a redirection there denies like a number does. Co-Authored-By: Claude Opus 5.5 --- host-setup/agent-safety/README.md | 2 +- .../agent-safety/claude/gh-write-guard.py | 25 ++++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md index c33ef24ff..83f537a85 100644 --- a/host-setup/agent-safety/README.md +++ b/host-setup/agent-safety/README.md @@ -85,7 +85,7 @@ text says, because the harm it covers was never in the text. where any git command's subcommand goes, and among a push's words or a commit's. On any git command it is denied among the global options where word splitting may split it. A redirection where the subcommand goes, a numbered one such as `2>` included, is denied too, - short of one after an option such as `--version` that ends git first. So is a redirection + unless git's first word is an option such as `--version` that ends git there. So is a redirection target spanning several words on a push or a commit. So is a comment among a push's words. Some words are read as they stand. They are a directory option's value short of a brace expansion, a commit option's double-quoted value, a commit path after `--`, and a redirection target held in one word. The diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index 6be584ae2..dddcf929a 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -2549,8 +2549,9 @@ def _rewritten_git_words(cmd): - a run-time word that word splitting may split among any git command's global options, or before no subcommand word at all, since `git -c $X README.md` may expand to a commit under an override, and a brace expansion in a directory option's value - - a redirection where the subcommand goes, a file-descriptor number before it included, since - the readings below then see no subcommand, unless an option such as `--version` ends git first + - a redirection where the subcommand goes, a file-descriptor number or a `{name}` before it + included, since the readings below then see no subcommand, unless git's first word is an + option such as `--version` that ends git there - a run-time word among a push's or a commit's global options, since `-c "$X"` may set `core.hooksPath` - a run-time word among a push's arguments, since it may name any refspec or remote @@ -2599,9 +2600,11 @@ def _rewritten_git_words(cmd): if _is_run_time_word(sub): return f"a run-time word, `{sub}`, where the subcommand of `git` goes" redirected = _is_redir_op(sub) or ( - sub.isdigit() and j + 1 < end and _is_redir_op(toks[j + 1]) + (sub.isdigit() or _NAMED_FD_RE.match(sub)) + and j + 1 < end + and _is_redir_op(toks[j + 1]) ) - if redirected and not any(toks[x] in _GIT_TERMINAL_OPTS for x in range(k + 1, j)): + if redirected and toks[k + 1] not in _GIT_TERMINAL_OPTS: return "a redirection where the subcommand of `git` goes" for x in range(k + 1, j): word = toks[x] @@ -6328,6 +6331,20 @@ def classify( "allow", "control: a redirection after --version, which ends git before any subcommand", ), + ( + "git --namespace -h > a commit --no-verify -m x", + "feature/x", + {"feature/x": set()}, + "deny", + "a terminal option taken as another option's value does not end git", + ), + ( + "git {fd}>/dev/null push origin develop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a named file descriptor redirection where the subcommand goes hides a push", + ), ( "(git push origin feature/x)", None, From cf78e916d0c4897996a67534a53eaad269c99ac8 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 13 Oct 2026 22:32:38 -0700 Subject: [PATCH 6/8] Deny a Redirection Among Git's Global Options in the Write Guard A redirection right after a value-taking global option, as in `git -c > a core.hooksPath=/dev/null commit`, was read as that option's value, while the shell pairs the option with the word after the target. It now denies like one where the subcommand goes. Co-Authored-By: Claude Opus 5.5 --- host-setup/agent-safety/README.md | 9 ++++---- .../agent-safety/claude/gh-write-guard.py | 22 ++++++++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md index 83f537a85..4a594b047 100644 --- a/host-setup/agent-safety/README.md +++ b/host-setup/agent-safety/README.md @@ -84,10 +84,11 @@ text says, because the harm it covers was never in the text. change what git runs. Such a word is a `$`, backtick, or brace-expansion word. It is denied where any git command's subcommand goes, and among a push's words or a commit's. On any git command it is denied among the global options where word splitting may split it. A - redirection where the subcommand goes, a numbered one such as `2>` included, is denied too, - unless git's first word is an option such as `--version` that ends git there. So is a redirection - target spanning several words on a push or a commit. So is a comment among a push's words. Some words are read as - they stand. They are a directory option's value short of a brace expansion, a commit option's + redirection where the subcommand goes or among the global options is denied too, a numbered + one such as `2>` included. The exception is a git whose first word is an option such as + `--version` that ends git there. A redirection target spanning several words on a push or a + commit is denied, and so is a comment among a push's words. Some words are read as they + stand. They are a directory option's value short of a brace expansion, a commit option's double-quoted value, a commit path after `--`, and a redirection target held in one word. The push and include checks also read each command with its quotes and backslashes removed and `git` in any case, so a `p''ush` or an uppercase `GIT` reaches them. The diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index dddcf929a..2636f9d86 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -2549,9 +2549,10 @@ def _rewritten_git_words(cmd): - a run-time word that word splitting may split among any git command's global options, or before no subcommand word at all, since `git -c $X README.md` may expand to a commit under an override, and a brace expansion in a directory option's value - - a redirection where the subcommand goes, a file-descriptor number or a `{name}` before it - included, since the readings below then see no subcommand, unless git's first word is an - option such as `--version` that ends git there + - a redirection where the subcommand goes or among the global options, a file-descriptor number + or a `{name}` before it included, since the readings below then see no subcommand or pair an + option with the wrong value, unless git's first word is an option such as `--version` that + ends git there - a run-time word among a push's or a commit's global options, since `-c "$X"` may set `core.hooksPath` - a run-time word among a push's arguments, since it may name any refspec or remote @@ -2604,6 +2605,7 @@ def _rewritten_git_words(cmd): and j + 1 < end and _is_redir_op(toks[j + 1]) ) + redirected = redirected or any(_is_redir_op(toks[x]) for x in range(k + 1, j)) if redirected and toks[k + 1] not in _GIT_TERMINAL_OPTS: return "a redirection where the subcommand of `git` goes" for x in range(k + 1, j): @@ -6345,6 +6347,20 @@ def classify( "deny", "a named file descriptor redirection where the subcommand goes hides a push", ), + ( + "git -c > a core.hooksPath=/dev/null commit -m x", + "feature/x", + {"feature/x": set()}, + "deny", + "a redirection among the global options pairs an option with the word after its target", + ), + ( + "git -C > a . push origin develop", + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a redirection after -C leaves the push readable only to the shell", + ), ( "(git push origin feature/x)", None, From 2bf4508b46c6f327d2aa13fb13c3713d51922dee Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 13 Oct 2026 22:34:22 -0700 Subject: [PATCH 7/8] Name the Global Options in the Redirection Deny Reason Co-Authored-By: Claude Opus 5.5 --- host-setup/agent-safety/claude/gh-write-guard.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index 2636f9d86..5975f5204 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -2607,7 +2607,9 @@ def _rewritten_git_words(cmd): ) redirected = redirected or any(_is_redir_op(toks[x]) for x in range(k + 1, j)) if redirected and toks[k + 1] not in _GIT_TERMINAL_OPTS: - return "a redirection where the subcommand of `git` goes" + return ( + "a redirection where the subcommand of `git` goes or among its global options" + ) for x in range(k + 1, j): word = toks[x] if not _is_run_time_word(word): From a95015be75d554051e5396e55a5b6f76ef9e4c91 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 13 Oct 2026 22:45:05 -0700 Subject: [PATCH 8/8] Read a Quoted Operator as a Word in the Shell-Rewrite Scan The scan for where a git command ends now carries the quote mask, so a quoted parenthesis or separator no longer ends it before a later run-time word. Co-Authored-By: Claude Opus 5.5 --- .../agent-safety/claude/gh-write-guard.py | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/host-setup/agent-safety/claude/gh-write-guard.py b/host-setup/agent-safety/claude/gh-write-guard.py index 5975f5204..e0804701c 100755 --- a/host-setup/agent-safety/claude/gh-write-guard.py +++ b/host-setup/agent-safety/claude/gh-write-guard.py @@ -2394,19 +2394,20 @@ def _is_run_time_word(tok): ) -def _region_end(toks, k): +def _region_end(toks, k, quoted=None): """The index of the separator after `toks[k]` that ends its command. A parenthesis the command opens, as a `$(...)` or a `<(...)` in one of its words does, is read through to the one closing it, separators inside included. A closing one it did not open ends - the command, as a subshell's does. + the command, as a subshell's does. A token `quoted` marks as quoted is a word rather than an + operator, and with no mask every operator-shaped token is read as one. """ n = len(toks) depth = 0 e = k + 1 while e < n: t = toks[e] - if _is_shell_op(t): + if _is_shell_op(t) and not (quoted and quoted[e]): sep = _is_separator(t) if sep and depth == 0 and t[0] not in "()": break @@ -2581,12 +2582,12 @@ def _rewritten_git_words(cmd): for k, t in enumerate(toks): if not _is_git_exe(t): continue - end = _region_end(toks, k) + if not masked: + mask, raw, masked = _quoted_mask(text, toks), _quote_kept_tokens(text, toks), True + end = _region_end(toks, k, mask) j = min(_git_subcommand_index(toks, k), end) if j == k + 1 and j >= end: continue - if not masked: - mask, raw, masked = _quoted_mask(text, toks), _quote_kept_tokens(text, toks), True opened = k and toks[k - 1].endswith("(") and not (mask and mask[k - 1]) if not opened and not _runs_as_command(toks, k, mask): continue @@ -6363,6 +6364,20 @@ def classify( "deny", "a redirection after -C leaves the push readable only to the shell", ), + ( + 'git push origin feature/x ")" $(echo develop)', + "feature/x", + {"feature/x": set(), "develop": _CODE_RULES}, + "deny", + "a quoted parenthesis is a word, so the scan reaches the substitution after it", + ), + ( + 'git commit -m ")" -m ";"', + "feature/x", + {"feature/x": set()}, + "allow", + "control: quoted operator characters as commit message values are words", + ), ( "(git push origin feature/x)", None,