Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
d28a2ff
Add Repository Boundaries and Write Safety rules; fix the runbook foo…
ptr727 Jul 20, 2026
e0c1ecc
Add portable per-machine agent write-safety kit (#365)
ptr727 Jul 20, 2026
3c5fe71
Fix 2>&1 suppression detection, quote the launcher, house-style the k…
ptr727 Jul 20, 2026
20f9ffa
Tighten node-id detection, harden python selection, title-case + de-s…
ptr727 Jul 20, 2026
75b460d
Restore install.ps1 to CRLF
ptr727 Jul 20, 2026
7c427ae
Fix the 2>&1 wording in the runbook and a semicolon in install.py (Co…
ptr727 Jul 20, 2026
d4d224d
Address Copilot round 4 on the write-safety kit (#367)
ptr727 Jul 20, 2026
9325942
Use a reference-style link in the kit README (#367)
ptr727 Jul 20, 2026
8dfc5ff
Fix CLAUDE_HOME tilde expansion; record gh review-request API traps (…
ptr727 Jul 20, 2026
3fd3c5a
Actually block the || : force-success tail; align the pattern lists (…
ptr727 Jul 20, 2026
c38d9ed
Fail gracefully on corrupt settings.json; propagate exit code on Wind…
ptr727 Jul 20, 2026
0fab303
Align doc snippets with actual behavior (#367)
ptr727 Jul 20, 2026
d699ecf
Deny quoted -R cross-repo writes; title-case README H1; verify python…
ptr727 Jul 20, 2026
cf9b1e3
Do not false-deny a suppression token quoted inside a write body (#367)
ptr727 Jul 20, 2026
fe4f733
Handle escaped quotes when stripping; mark Verify shell explicitly (#…
ptr727 Jul 20, 2026
8353f17
Match the README to the installed CLAUDE.md heading (#367)
ptr727 Jul 20, 2026
6b377c3
Guarantee a single hook registration across multiple Bash groups (#367)
ptr727 Jul 20, 2026
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
6 changes: 6 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ end_of_line = lf
[spec/{validate,audit}.py]
end_of_line = lf

# The agent-safety kit's Python is shebang-executable tooling run by path (the PreToolUse hook and its
# installer), so pin LF for the same reason as the entry points above - a CRLF shebang breaks direct
# execution on a Unix host.
[host-setup/agent-safety/*.py]
end_of_line = lf

# uv regenerates uv.lock with LF on every platform, so pin it or an EOL check (editorconfig-checker/CI)
# reds on every `uv lock`/`uv sync` until the file is manually reconverted - same rationale as the
# shebang/Dockerfile pins (a tool owns the ending). A Python repo on the CRLF default carries this; a repo
Expand Down
5 changes: 4 additions & 1 deletion .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ catalog/snippets/husky/pre-commit text eol=lf

# Vanilla `.py` follows the CRLF default - Python's universal newlines accept CRLF, and it is
# commonly edited on Windows. Pin LF only for a `.py` executed directly via its shebang, by path -
# here the CI validation entry point; do not re-add a blanket `*.py text eol=lf`.
# here the CI validation entry point, the fleet-audit runner, and the agent-safety hook and its
# installer. Do not re-add a blanket `*.py text eol=lf`.
spec/validate.py text eol=lf
spec/audit.py text eol=lf
host-setup/agent-safety/gh-write-guard.py text eol=lf
host-setup/agent-safety/install.py text eol=lf

# uv regenerates uv.lock with LF on every platform; pin it so git enforces LF on checkout/renormalize and a
# CRLF-default repo does not fight the tool on every `uv lock`/`uv sync`. A repo with no lockfile is unaffected.
Expand Down
31 changes: 27 additions & 4 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ Known non-working request paths (don't rely on them - use the `requestReviews` m

- `POST /requested_reviewers` with `reviewers=[Copilot]` can return 200 but no-op.
- `copilot-pull-request-reviewer` as a requested reviewer slug returns 422.
- `requestReviews` with the reviewer's bot node id in **`userIds`** fails with `Could not resolve to User node` - the Copilot reviewer is a **Bot**, so its node id goes in **`botIds`** (as in the mutation above), never `userIds`.
- `suggestedActors(capabilities: [CAN_BE_ASSIGNED])` lists `copilot-swe-agent` (the coding agent), not `copilot-pull-request-reviewer` - do not source the reviewer's bot node id there. Read it from an existing review per step 1 above.
- There is no `removePullRequestFromReviewRequest` mutation, and removing the reviewer to force a fresh pass is unnecessary anyway - `requestReviews` with `union: true` re-fires the review on the current head.

### Verify Review Covered Current Head

Expand Down Expand Up @@ -119,6 +122,8 @@ If a review did not run on the current head, retry:

### Reply and Thread Resolution Workflow

Every id below is captured from a live query into a variable and passed from there - never hand-typed, guessed, or pasted as a `PRRT_...` literal. A node id resolves globally, so a fabricated or stale id does not fail, it writes to a real thread on an unrelated repository. This runbook implements [AGENTS.md "Repository Boundaries and Write Safety"](../AGENTS.md#repository-boundaries-and-write-safety): write only to this repo, capture every id from a live query, and never suppress a mutation's output.

List unresolved threads. Use `first: 100` with cursor-based pagination; if `hasNextPage` is true, re-run with `after: "<endCursor>"` to retrieve the next page:

```sh
Expand All @@ -142,20 +147,38 @@ gh api graphql -f query='
'
```

Reply on a thread, then resolve it:
Reply on a thread, then resolve it. Capture the target thread's id into `$TID` from the listing query above - filter to the thread being answered by its `path`, and guard for an empty result so a mutation never runs on a guessed id. When a file carries more than one unresolved thread, `path` alone is ambiguous and `head -n 1` would pick the wrong one, so narrow by first-comment body - the query already fetches `comments(first: 1)` for this - by adding `and (.comments.nodes[0].body | contains("<SNIPPET>"))` to the `select`:

```sh
TID=$(gh api graphql -f query='
{
repository(owner: "<owner>", name: "<repo>") {
pullRequest(number: <N>) {
reviewThreads(first: 100) {
nodes { id isResolved path comments(first: 1) { nodes { body } } }
}
}
}
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[]
| select(.isResolved == false and .path == "<PATH>")
| .id' | head -n 1)
[ -n "$TID" ] || { echo "no matching unresolved thread on <PATH> - do not guess an id" >&2; return 1 2>/dev/null || exit 1; }

# Show the mutation's output. Never append an output-discard or force-success tail
# (>/dev/null, 2>/dev/null, &>/dev/null, || true, || :, || echo) to a write.
gh api graphql -f query='
mutation($threadId: ID!, $body: String!) {
addPullRequestReviewThreadReply(input: { pullRequestReviewThreadId: $threadId, body: $body }) {
comment { id }
comment { id url }
}
}' -F threadId="PRRT_..." -F body="Fixed in <SHA>: <one-line summary>."
}' -F threadId="$TID" -F body="Fixed in <SHA>: <one-line summary>."

# Confirm isResolved: true in this response before treating the thread as closed - a write that
# appears to fail may have taken on the server.
gh api graphql -f query='
mutation($threadId: ID!) {
resolveReviewThread(input: { threadId: $threadId }) { thread { id isResolved } }
}' -F threadId="PRRT_..."
}' -F threadId="$TID"
```

Issue-level Copilot comments (those in `issues/<N>/comments`) have no resolution action - GitHub provides no API or UI to resolve them. Reply if the finding warrants it; no resolution step is needed or possible.
Expand Down
8 changes: 8 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ The specific rules in this file implement a few governing principles. Read these
- **Two version numbers, two jobs.** The 2-digit `major.minor` in `version.json` carries human meaning - the maintainer raises it only for a functional change (feature, behavior or API change, breaking change), at their discretion - while NBGV owns the patch position and always increments with git height, so every build is uniquely versioned with no edit. Human-facing docs name the 2-digit line; the toolchain guarantees monotonic builds. See "Release Model".
- **Contracts state what, not how, and favor reuse.** [`WORKFLOW.md`](./WORKFLOW.md) fixes required outcomes, not a required implementation - two repos may satisfy a guarantee with different YAML. Within that freedom, apply good engineering practice: minimize duplication and maximize reuse, which is why the pipeline splits a carried, generic orchestration layer from a repo-owned build layer.

## Repository Boundaries and Write Safety

A state-changing GitHub call is the highest-blast-radius thing an agent does here: it runs under the maintainer's identity, so one wrong target writes to another owner's repository as the maintainer - an outward-facing, hard-to-reverse act. These rules bound every write - a git push, an API mutation, a comment, a label, a merge - on any platform. Reads are unrestricted. The bounds below are on writes.

- **Write only to the current project's own repository.** Every state-changing call targets this project's `origin` and nothing else. A broad or logged-in identity is capability, not permission - a token that *can* reach another repository does not authorize writing to it. Writing to any other repository needs explicit, per-session human permission for that specific repository, and a "harmless test" write is still a write, so there is no probe exception. Reads from anywhere are fine.
- **Never fabricate, guess, or reuse an identifier passed to a write.** Every id a state-changing call consumes - a node id, a numeric id, a thread or comment id - is captured from a live query in the **same** session into a variable and passed from there. Do not hand-type an id, guess it, recall it from memory or an earlier session, or copy it from documentation or an example. Ids commonly resolve **globally**, so a wrong-but-valid id does not fail - it writes to the wrong target, in someone else's repository. If a query returns no id, stop rather than invent one to proceed.
- **A write is never a probe, and a write's output is never suppressed.** Never fire a state-changing call to see whether it works: decide it should happen, make it happen, and read the result. Never append output-discarding redirection or a force-success tail to a mutation (for example `>/dev/null`, `2>/dev/null`, `&>/dev/null`, `|| true`, `|| :`, `|| echo`) - the write's output is exactly what must be read. A write that appears to fail is **verified, not assumed harmless** - the operation may have succeeded on the server while the client reported an error - so confirm the actual state before retrying or moving on.

## Git and Commit Rules

- **Default to staging, not committing.** Stage changes with `git add` and leave `git commit` to the developer unless the developer has explicitly authorized the agent to commit for the current ask ("commit this", "open a PR", etc.). Authorization is scope-bound - it covers the commits needed for that specific task, not a blanket commit license for the rest of the session.
Expand Down
8 changes: 8 additions & 0 deletions host-setup/agent-safety/.markdownlint-cli2.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
// claude-md-safety.md is a fragment the installer appends into ~/.claude/CLAUDE.md (which already
// has its own H1), so it intentionally opens at H2. MD041 (first line must be a top-level heading)
// does not apply to an appended snippet. This nested config affects only this directory.
"config": {
"MD041": false
}
}
72 changes: 72 additions & 0 deletions host-setup/agent-safety/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Agent Write-Safety Kit

Per-machine, user-account-scoped guards against an agent making a mis-targeted GitHub **write** under the maintainer's identity. Deploy it as the **first thing on any new system** where Claude Code runs with the `gh` credentials logged in (WSL, Linux, macOS, Proxmox, Windows).

## What It Installs

Into `~/.claude/` (or `%USERPROFILE%\.claude\` on Windows):

- **`hooks/gh-write-guard.py`** - a PreToolUse hook that denies the three 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 outside the checkout's `origin`. Reads and everything else pass through. It fires even in autonomous / bypass-permissions sessions, which is how the incident happened.
- **A `## GitHub Write Safety (Any Project, Every Session)` section in `CLAUDE.md`** - the same three rules as behavioral guidance, loaded into every session on the machine (including ad-hoc work outside any project). It mirrors the committed `AGENTS.md` "Repository Boundaries and Write Safety" rules, which only reach fleet repos.

The hook is the mechanical backstop. The CLAUDE.md rules and the carried AGENTS.md rules are the behavioral layer. Prose alone is not enough - the incident happened under prose rules - so both ship.

## Install (Idempotent - Safe to Re-Run to Update)

```sh
# Linux / WSL / macOS / Proxmox
host-setup/agent-safety/install.sh
```

```powershell
# Windows
host-setup\agent-safety\install.ps1
```

Both are thin wrappers around `install.py`, so every OS runs one tested code path. The installer self-tests the hook before registering it, merges the settings.json entry without clobbering other keys, and updates the CLAUDE.md block in place (marker-delimited) rather than duplicating it.

**Restart Claude Code sessions on the machine afterward** so the new hook and CLAUDE.md load.

## Verify (POSIX Shell)

```sh
python3 ~/.claude/hooks/gh-write-guard.py --selftest # decision matrix: all cases pass
grep -c 'agent-safety v' ~/.claude/CLAUDE.md # expect 2 (start + end marker)
```

On Windows PowerShell:

```powershell
py -3 "$env:USERPROFILE\.claude\hooks\gh-write-guard.py" --selftest # all cases pass
(Select-String 'agent-safety v' "$env:USERPROFILE\.claude\CLAUDE.md").Count # expect 2
```

Live end-to-end (in any repo): attempt a discarded-output write and confirm the Bash tool is blocked:

```sh
gh api graphql -f query='mutation{noop}' -F t="PRRT_x" >/dev/null 2>&1 || true # blocked by the hook
```

## Manual settings.json Shape (for Reference)

The installer writes this. It is here so you can inspect or hand-place it:

```json
{
"hooks": {
"PreToolUse": [
{ "matcher": "Bash", "hooks": [ { "type": "command", "command": "\"python3\" \"<home>/.claude/hooks/gh-write-guard.py\"" } ] }
]
}
}
```

## Scope and Limits

- **Per-machine.** `~/.claude/` does not travel, so run the installer on each box. This is the rollout that [#365][issue-365] tracks.
- **Precision over recall.** The hook denies the specific dangerous shapes with high confidence rather than gating every write, so it never blocks legitimate work. A shape it does not catch still falls under the behavioral rules.
- **Opaque targets are unseen.** The hook cannot see the repository behind a GraphQL node id, which is exactly why rule 2 blocks a *literal* id at all - a captured `$variable` is trusted. Likewise, the cross-origin check only runs when an `origin` can be resolved and the write names an explicit `-R`/`repos/<owner>/<repo>` target. A write from a non-git directory, or one whose target is only a node id, is evaluated by rules 1 and 2 alone.
- **Not a credential control.** A fine-grained PAT limited to owned repositories is a separate, stronger structural guard (a hard `403` on any non-owned repo) and is left to per-machine credential setup, out of this kit.

<!-- Repo -->
[issue-365]: https://github.com/ptr727/ProjectTemplate/issues/365
9 changes: 9 additions & 0 deletions host-setup/agent-safety/claude-md-safety.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!-- agent-safety v1 start -->
## GitHub Write Safety (Any Project, Every Session)

A `gh` / GitHub API write runs under the logged-in identity, so a mis-targeted write acts publicly as that account on someone else's repository - outward-facing and hard to reverse. These rules bound every write (a git push, an API mutation, a comment, a label, a merge) in every session on this machine, including ad-hoc work outside any project. Reads are unrestricted. A committed repo's `AGENTS.md` "Repository Boundaries and Write Safety" states the same rules for its fleet, and the two are kept in sync deliberately, because this file also covers sessions that `AGENTS.md` never reaches. The `gh-write-guard` PreToolUse hook enforces the mechanical half.

- **Write only to the current project's own repository.** Every state-changing call targets this checkout's `origin` and nothing else. A broad or logged-in identity is capability, not permission. Another repository needs explicit, per-session human permission for that specific repository, and a "harmless test" write is still a write.
- **Never fabricate, guess, or reuse an identifier passed to a write.** Every id a write consumes (a node id, a numeric id, a thread or comment id) is captured from a live query in the same session into a variable and passed from there. Ids resolve globally, so a wrong-but-valid id does not fail - it writes to the wrong target in another repository. If a query returns no id, stop rather than invent one.
- **A write is never a probe, and a write's output is never suppressed.** Never fire a state-changing call to see whether it works, and never append an output-discarding or force-success tail (for example `>/dev/null`, `2>/dev/null`, `&>/dev/null`, `|| true`, `|| :`, `|| echo`) to a mutation. A write that appears to fail is verified, not assumed harmless - it may have succeeded on the server.
<!-- agent-safety v1 end -->
Loading