diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md index c9b5fb61..4a594b04 100644 --- a/host-setup/agent-safety/README.md +++ b/host-setup/agent-safety/README.md @@ -80,7 +80,18 @@ 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 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 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 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 bd8464d6..94dd8440 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 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. @@ -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 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 75b499b0..e0804701 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 filesystem resolves `GIT` to 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,320 @@ 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_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" +_GIT_TERMINAL_OPTS = frozenset({"--version", "-v", "--help", "-h", "--html-path", "--man-path"}) +_BRACE_EXPANSION = re.compile(r"\{[^{}'\"\s]*(?:,|\.\.)[^{}'\"\s]*\}") + + +def _is_run_time_word(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, 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. 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) and not (quoted and quoted[e]): + sep = _is_separator(t) + if sep and depth == 0 and t[0] not in "()": + break + depth += t.count("(") - t.count(")") + if depth < 0 or (sep and 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. + + 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] == "-" + 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, 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. 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() + 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 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("(") + 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 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 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 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 + 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 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 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 + - 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 before a subcommand word, unquoted too, since `-C $dir` is the common + 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"`. + 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 + 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 + 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" + redirected = _is_redir_op(sub) or ( + (sub.isdigit() or _NAMED_FD_RE.match(sub)) + 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 or among its global options" + ) + 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( + 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 < end: + arg = toks[x] + if _is_redir_op(arg): + x += 1 + if x < end and not _is_shell_op(toks[x]): + 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 == "--" + 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 + return None + + def _push_invocations(cmd): """Every `git push` as (c_dirs, work_tree, leading_cd, cd_unreliable, args), wrappers included. @@ -4193,10 +4530,19 @@ def classify( dec, reason = _check_bypass_flags(cmd) if dec == "deny": return dec, reason + # 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", ( + 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 +6042,349 @@ 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 filesystem resolves it to 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 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 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 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 --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 -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 ")" $(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, + {"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 2802cf11..7299bc2d 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:*)"), ]