Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions host-setup/agent-safety/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,12 @@ hook or approval-gate API, not tied to Claude Code's `PreToolUse` JSON shape.
alone lets `-qf`/`-Bname` reach the ref-switch exemption below while still forcing the checkout
through, and would equally have let `switch -C <existing-branch>` through, confirmed live to
reset that branch to the current HEAD with no dirty-tree warning at all, since it is not a
working-tree overwrite. Allow `worktree add|list|prune`, a
plain `worktree remove` with no force flag, any read, `merge --ff-only`/`pull --ff-only` (git's
working-tree overwrite. A `worktree remove`'s own `-f` is bundled the same way, since `remove`
has no other short option `-f` could combine with: git requires `-f` given twice to remove a
locked worktree, and the bundled `-ff` spelling satisfies that exactly as `-f -f` does,
confirmed live to forcibly remove a locked worktree's uncommitted content. Allow
Comment thread
ptr727 marked this conversation as resolved.
`worktree add|list|prune`, a plain `worktree remove` with no force flag, any read, `merge
--ff-only`/`pull --ff-only` (git's
own semantics mean neither can discard anything), a bare `-` as a `checkout`/`switch` argument
(porcelain shorthand for the previous branch, which only those two subcommands themselves
understand, so it is exempt outright rather than checked), and a `checkout <ref>`/`switch <ref>`
Expand Down
75 changes: 52 additions & 23 deletions host-setup/agent-safety/claude/gh-write-guard.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
them.

Precision over recall for the write-footgun shapes (1-3) and the primary-checkout shape (6): they deny
the specific shapes that caused an incident, not everything unparseable - a false deny would break the
the specific shapes that caused an incident, not everything unparseable, since a false deny would break the
agent, and a miss still falls under the GOVERNANCE.md "Repository Boundaries and Write Safety" prose
rules. The branch-bypass rule (4) instead fails CLOSED on the protected-by-default branches, because the
harm there is a silent success under the maintainer's admin bypass. The denied shapes:
Expand All @@ -35,41 +35,41 @@
and the hand-run GraphQL form is then the documented fallback, not a footgun.
6. a mutating git operation (checkout/switch/pull/reset/rebase/merge/cherry-pick/revert/restore/stash
(anything but list/show)/clean -f/add/commit/rm/mv/apply/am/push/worktree remove -f) run
directly against a primary checkout, not a linked worktree - the harm behind a separate
incident, where an
agent reused the maintainer's own primary checkout instead of a worktree twice despite having read the
prose rule against it. A "primary checkout" is decided by comparing `git rev-parse --git-dir`
against `--git-common-dir`, never a `.git`-is-a-directory guess (a submodule's `.git` is a file
and is still primary). The target directory follows real git's own priority rather than a
last-option-wins scan: any `-C <dir>` options on the invocation compose sequentially onto a leading
`cd` (inside a `sh -c`/`bash -c` wrapper too) or the session's own cwd, then an explicit
`--work-tree`/`GIT_WORK_TREE=` value, when given, wins over that result regardless of `-C`;
`--git-dir`/`GIT_DIR=` alone never relocates that reported target, matching git's own fallback. A
leading `export GIT_WORK_TREE=x GIT_DIR=y &&` prefix is read the same way an inline `VAR=x git ...`
prefix already is, since a real shell export persists into the following command exactly as
effectively - confirmed live to discard a tracked local modification with no redirect at all on the
git invocation itself, a shape the inline-prefix scan alone cannot see.
directly against a primary checkout, not a linked worktree. This is the harm behind a
separate incident, where an agent reused the maintainer's own primary checkout instead of a
worktree twice despite having read the prose rule against it. A "primary checkout" is decided
by comparing `git rev-parse --git-dir` against `--git-common-dir`, never a `.git`-is-a-directory
guess (a submodule's `.git` is a file and is still primary). The target directory follows real
git's own priority rather than a last-option-wins scan: any `-C <dir>` options on the
invocation compose sequentially onto a leading `cd` (inside a `sh -c`/`bash -c` wrapper too) or
the session's own cwd, then an explicit `--work-tree`/`GIT_WORK_TREE=` value, when given, wins
over that result regardless of `-C`, and `--git-dir`/`GIT_DIR=` alone never relocates that
reported target, matching git's own fallback. A leading `export GIT_WORK_TREE=x GIT_DIR=y &&`
prefix is read the same way an inline `VAR=x git ...` prefix already is, since a real shell
export persists into the following command exactly as effectively, confirmed live to discard a
tracked local modification with no redirect at all on the git invocation itself, a shape the
inline-prefix scan alone cannot see.
Whether the invocation is primary-checkout at all is a separate question from the mutation target,
though: an explicit `--git-dir`/`GIT_DIR=` is resolved and tested for primary-checkout-ness
directly, regardless of `--work-tree`, since `--git-dir` names the repository actually mutated -
directly, regardless of `--work-tree`, since `--git-dir` names the repository actually mutated,
confirmed live that `--git-dir=<primary>/.git --work-tree=<empty-dir>` mutates `<primary>` even
though `<empty-dir>` resolves as no git repository at all, which testing the work-tree value alone
would fail open on. `~`/`$HOME` is expanded throughout and a relative value is joined against the
running result. Checkout/switch force flags (`-b`/`-B` for checkout, `-c`/`-C` for switch,
`-f`/`--force`/`--discard-changes`/`--orphan` for either) are recognized bundled or attached into a
short-option cluster (`-qf`, `-Bname`, `-Cother`), not only as an exact token. A subcommand this
rule does not recognize is resolved through a bounded chain of git aliases (inline `-c
alias.<name>=`, then the target's own persisted config) before falling through to allow; a
alias.<name>=`, then the target's own persisted config) before falling through to allow. A
`!`-prefixed shell alias is denied outright rather than interpreted. Exempt: `worktree
add/list/prune` and an unforced `worktree remove` (the
documented way to use a primary checkout at all), `merge --ff-only`/`pull --ff-only` (can never
discard anything), and a `checkout <ref>`/`switch <ref>` carrying no force-oriented flag whose
argument actually resolves as a ref, verified live (git's own ref-switch path refuses to carry a
local modification, but its pathspec-restore fallback for an argument that is not a ref, such as
`checkout .` or `checkout -- <path>`, carries no such check and is denied). A non-force flag such
as `--detach`/`-q` alongside the ref stays exempt too - it changes nothing about git's own
as `--detach`/`-q` alongside the ref stays exempt too, since it changes nothing about git's own
overwrite-refusal, verified live, so admitting it widens no actual safety hole, only the exemption's
literal shape - matching the documented base-clone cleanup step in the repo-worktree skill. Granted
literal shape, matching the documented base-clone cleanup step in the repo-worktree skill. Granted
only by GH_WRITE_GUARD_ALLOW_PRIMARY_CHECKOUT (a recognized falsy
value such as "0"/"false" reads as not granted, not as any-non-empty-string-is-truthy), the same
channel shape as GH_WRITE_GUARD_ALLOW.
Expand Down Expand Up @@ -345,9 +345,8 @@ def _shell_tokens(cmd):
try:
lex = shlex.shlex(cmd, posix=True, punctuation_chars=_PUNCTUATION_CHARS)
lex.whitespace_split = True
# `shlex.shlex`'s own default keeps `#` as a comment starter, unlike `shlex.split()` below (and the fallback branch, since it calls that same function), which explicitly clears it.
# Confirmed live: left at the default, a `#` anywhere in the command, even mid-word, silently drops everything after it through the next newline, fusing `git fetch origin # refresh\ngit reset --hard origin/main` into one `git fetch` invocation carrying the whole `reset --hard` as extra argv, hiding the second command from every tokenizer-based rule.
# A comment truncating visibility into a real subsequent command is a far worse failure mode for a security-relevant hook than the reverse (an ordinary bash `# comment` becoming literal trailing argv words instead of being dropped), so this is cleared unconditionally rather than attempting bash's own quoted-vs-unquoted, start-of-word-only comment semantics, which `shlex`'s single `commenters` string cannot express and unquoted-only detection over an already-tokenized stream cannot recover.
# `shlex.shlex`'s own default keeps `#` as a comment starter, unlike `shlex.split()`, which explicitly clears it, and confirmed live to otherwise fuse `git fetch origin # x\ngit reset --hard` into one invocation, hiding the second command from every tokenizer-based rule.
# Cleared unconditionally: a truncated command is a far worse failure than an ordinary `#` becoming literal trailing argv words instead.
lex.commenters = ""
lex.whitespace = lex.whitespace.replace(
"\n", ""
Expand Down Expand Up @@ -977,9 +976,14 @@ def _primary_checkout_verdict(sub, args, target_dir=None, ref_resolver=None):
if sub == "worktree":
# `add`/`list`/`prune` are always allowed, the documented way to use a primary checkout from an agent session.
# `remove` is allowed too unless forced: git itself already refuses to remove a worktree carrying uncommitted changes without --force, so only the forced form reproduces the harm this rule exists to catch.
# `-f` is bundled the same way checkout/switch's own force flags already are: git requires `-f` given twice to remove a locked worktree, and `-ff` satisfies that, confirmed live.
# Scanned before any `--`, the same cutoff `clean`'s own force scan already applies: confirmed live that `git worktree remove -- -f` reads `-f` as a worktree path argument (erroring since none is literally named that), not a force flag.
if not args or args[0] != "remove":
return None
return any(a in ("-f", "--force") for a in args[1:])
return any(
a in ("-f", "--force") or (a.startswith("-") and not a.startswith("--") and "f" in a)
for a in _args_before_double_dash(args[1:])
)
Comment thread
Copilot marked this conversation as resolved.
Comment thread
ptr727 marked this conversation as resolved.
if sub in ("checkout", "switch"):
# `--` unambiguously means every following argument is a pathspec, not a ref: `checkout -- <path>`/`checkout <ref> -- <path>` restores that path from the index unconditionally, with none of the "would overwrite a local modification" safety check a ref switch gets.
if "--" in args:
Expand Down Expand Up @@ -2879,6 +2883,31 @@ def classify(
"deny",
"a forced worktree remove reproduces the harm this rule guards against",
),
(
"git worktree remove -ff ../x",
"/primary",
{"/primary": True},
None,
"deny",
(
"-ff bundles force twice, exactly the -f -f git itself requires to remove a locked "
"worktree, confirmed live to forcibly remove one with uncommitted content, which an "
"exact-token check alone misses since remove has no other short option -f could "
"combine with"
),
Comment thread
ptr727 marked this conversation as resolved.
),
(
"git worktree remove -- -f",
"/primary",
{"/primary": True},
None,
"allow",
(
"-f after -- is a worktree path argument, not a force flag, confirmed live: git "
"reads it as a literal worktree name (erroring since none is named that) rather than "
"forcing anything, the same -- cutoff clean's own force scan already applies"
),
),
(
"git worktree remove ../x",
"/primary",
Expand Down