diff --git a/.editorconfig b/.editorconfig index 2038d6fa..464d489c 100644 --- a/.editorconfig +++ b/.editorconfig @@ -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 diff --git a/.gitattributes b/.gitattributes index 126849d2..b69124f1 100644 --- a/.gitattributes +++ b/.gitattributes @@ -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. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index ec55c76b..fd0af4be 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -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 @@ -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: ""` to retrieve the next page: ```sh @@ -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(""))` to the `select`: ```sh +TID=$(gh api graphql -f query=' +{ + repository(owner: "", name: "") { + pullRequest(number: ) { + reviewThreads(first: 100) { + nodes { id isResolved path comments(first: 1) { nodes { body } } } + } + } + } +}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] + | select(.isResolved == false and .path == "") + | .id' | head -n 1) +[ -n "$TID" ] || { echo "no matching unresolved thread on - 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 : ." +}' -F threadId="$TID" -F body="Fixed in : ." +# 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//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. diff --git a/AGENTS.md b/AGENTS.md index 253b97e9..4b9d980c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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. diff --git a/host-setup/agent-safety/.markdownlint-cli2.jsonc b/host-setup/agent-safety/.markdownlint-cli2.jsonc new file mode 100644 index 00000000..8eb5a70a --- /dev/null +++ b/host-setup/agent-safety/.markdownlint-cli2.jsonc @@ -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 + } +} diff --git a/host-setup/agent-safety/README.md b/host-setup/agent-safety/README.md new file mode 100644 index 00000000..ee1062b4 --- /dev/null +++ b/host-setup/agent-safety/README.md @@ -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\" \"/.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//` 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. + + +[issue-365]: https://github.com/ptr727/ProjectTemplate/issues/365 diff --git a/host-setup/agent-safety/claude-md-safety.md b/host-setup/agent-safety/claude-md-safety.md new file mode 100644 index 00000000..38dc8a49 --- /dev/null +++ b/host-setup/agent-safety/claude-md-safety.md @@ -0,0 +1,9 @@ + +## 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. + diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py new file mode 100644 index 00000000..5ce846e1 --- /dev/null +++ b/host-setup/agent-safety/gh-write-guard.py @@ -0,0 +1,224 @@ +#!/usr/bin/env python3 +"""PreToolUse guard: deny the GitHub-write footguns behind the cross-repo comment incident. + +Registered as a Claude Code PreToolUse hook on the Bash tool. It reads the tool-input JSON on stdin, +classifies the command, and DENIES (with a reason shown to the agent) when a command is a GitHub *write* +matching a known-dangerous pattern. Reads and everything that is not a clear write pass through. + +Precision over recall by design: it denies the specific shapes that caused the incident, not everything +it cannot parse. A false deny would break the agent, while a missed case still falls under the AGENTS.md +"Repository Boundaries and Write Safety" prose rules. The three denied shapes: + + 1. a state-changing gh call whose output is discarded or forced to success + (>/dev/null, 2>/dev/null, &>/dev/null, || true, || :, || echo) + 2. a GraphQL mutation passing a literal GitHub node id (PRRT_/PR_/BOT_/...) instead of a $variable + 3. a gh write with an explicit -R/--repo/repos// target outside the checkout's origin + +Run `gh-write-guard.py --selftest` to verify the decision matrix without Claude Code. +""" +import json +import os +import re +import subprocess +import sys + +# --- What counts as a GitHub write ------------------------------------------------------------------- +# gh subcommands that mutate. `gh api` is handled separately (it needs field/method inspection). +_GH_WRITE_SUB = re.compile( + r"""\bgh\s+(?: + pr\s+(?:create|comment|close|merge|edit|review|reopen|ready|lock|unlock) + | issue\s+(?:create|comment|close|edit|reopen|delete|lock|unlock|pin|unpin|transfer) + | release\s+(?:create|edit|delete|upload) + | repo\s+(?:create|delete|edit|rename|archive) + | (?:label|secret|variable|ruleset)\s+(?:create|delete|edit|set) + | gist\s+(?:create|edit|delete) + )\b""", + re.X, +) +_GH_API = re.compile(r"\bgh\s+api\b") +_EXPLICIT_WRITE_METHOD = re.compile(r"(?:--method|-X)\s+(?:POST|PUT|PATCH|DELETE)\b", re.I) +# gh api with a field flag defaults to POST even without -X, so it is a write. +_API_FIELD_FLAG = re.compile(r"(?:^|\s)(?:-f|-F|--field|--raw-field|--input)\b") +_GRAPHQL = re.compile(r"\bgh\s+api\b.*\bgraphql\b", re.S) +_MUTATION = re.compile(r"\bmutation\b") +_GIT_PUSH = re.compile(r"\bgit\s+push\b") + +# --- Risk-pattern detectors -------------------------------------------------------------------------- +# Output-discard / force-success tails. Bare `2>&1` is NOT here: it merges stderr into stdout, leaving +# the output visible, so it is not suppression (and denying it would break `... 2>&1 | tee log`). +_SUPPRESS = re.compile(r">\s*/dev/null|&>\s*/dev/null|2>\s*/dev/null|\|\|\s*(?:true\b|echo\b|:)") +# A quoted argument value ("..." or '...'). Stripped before the suppression scan so a --body/--title +# that merely mentions `|| true` or `>/dev/null` as text is not mistaken for a real command tail. Real +# suppression tails are unquoted shell operators, so stripping quotes never hides an actual footgun. The +# double-quoted form allows `\"` escapes so an embedded quote does not end the span early; shell single +# quotes take no escapes, so their form is literal. +_QUOTED_SPAN = re.compile(r'"(?:\\.|[^"\\])*"' r"|'[^']*'") +# A GitHub global node id literal: an UPPERCASE prefix (PR_, PRRT_, IC_, BOT_, ...) + a long base64url +# body, or a legacy MD... base64 id. The uppercase prefix plus a >=12-char body keeps it from matching +# an ordinary underscored word in a reply body (e.g. body="fixed_the_thing_now", lowercase prefix). +_NODE_ID_LITERAL = re.compile(r'^(?:[A-Z]{1,5}_[A-Za-z0-9_\-]{12,}|MD[A-Za-z0-9]{12,})$') +# -F/-f name=VALUE, capturing the value - handles "quoted" and bare +_FIELD_ASSIGN = re.compile(r"""(?:-F|-f|--field|--raw-field)\s+[A-Za-z_][\w]*=(?P'[^']*'|"[^"]*"|\S+)""") +_EXPLICIT_REPO = re.compile(r"(?:-R|--repo)\s+(?P['\"]?)(?P[^\s'\"]+)(?P=q)") +_API_REPO_PATH = re.compile(r"\bgh\s+api\b[^\n|]*?\brepos/(?P[A-Za-z0-9_.\-]+)/(?P[A-Za-z0-9_.\-]+)") + + +def _is_gh_write(cmd): + if _GH_WRITE_SUB.search(cmd) or _GIT_PUSH.search(cmd): + return True + if _GH_API.search(cmd): + if _EXPLICIT_WRITE_METHOD.search(cmd): + return True + if _GRAPHQL.search(cmd) and _MUTATION.search(cmd): + return True + if _API_FIELD_FLAG.search(cmd) and not _GRAPHQL.search(cmd): + return True # gh api -f k=v => POST + return False + + +def _origin_owner_repo(cwd): + try: + url = subprocess.run( + ["git", "-C", cwd or ".", "remote", "get-url", "origin"], + capture_output=True, text=True, timeout=5, + ).stdout.strip() + except Exception: + return None + m = re.search(r"[:/]([A-Za-z0-9_.\-]+)/([A-Za-z0-9_.\-]+?)(?:\.git)?/?$", url) + return (m.group(1).lower(), m.group(2).lower()) if m else None + + +def classify(cmd, cwd=None, origin=None): + """Return (decision, reason). decision is 'allow' or 'deny'. + + origin, when given, is a (owner, repo) tuple used instead of resolving from cwd - the self-test + passes it for a deterministic, offline run. + """ + if not _is_gh_write(cmd): + return "allow", "" + + # 1. suppressed output on a write - scan with quoted argument values removed so a --body/--title + # that only mentions a suppression token as text does not false-deny a legitimate write. + if _SUPPRESS.search(_QUOTED_SPAN.sub("", cmd)): + return "deny", ( + "This is a GitHub write with its output discarded or forced to success " + "(>/dev/null, 2>/dev/null, &>/dev/null, || true, || :, || echo). " + "A write's result is exactly what must be read: a mutation can succeed on the server " + "while the client reports an error. Run it without the output-discarding tail and read " + "the response. See AGENTS.md 'Repository Boundaries and Write Safety'." + ) + + # 2. literal node id in a mutation + if _GRAPHQL.search(cmd) and _MUTATION.search(cmd): + for m in _FIELD_ASSIGN.finditer(cmd): + val = m.group("v").strip("'\"") + if val.startswith("$") or val.startswith("${"): + continue + if _NODE_ID_LITERAL.match(val): + return "deny", ( + f"This mutation passes a literal GitHub node id ({val[:16]}...) instead of a " + "variable captured from a live query. Node ids resolve globally, so a fabricated " + "or stale id writes to a real object in another repository. Capture the id from a " + "query in this session into a variable and pass -F ...=\"$VAR\". See AGENTS.md " + "'Repository Boundaries and Write Safety'." + ) + + # 3. explicit target outside origin + if origin is None: + origin = _origin_owner_repo(cwd) + targets = [] + mr = _EXPLICIT_REPO.search(cmd) + if mr and "/" in mr.group("r") and "<" not in mr.group("r"): + o, r = mr.group("r").split("/", 1) + targets.append((o.lower(), r.lower())) + for m in _API_REPO_PATH.finditer(cmd): + if "<" not in m.group("owner"): + targets.append((m.group("owner").lower(), m.group("repo").lower())) + # Only runs when origin resolves (a git checkout): with no project context there is nothing to + # compare an explicit target against, so this check is skipped and rules 1-2 still apply. A node-id + # target is invisible here regardless - that is what rule 2 guards. + if origin: + for t in targets: + if t != origin: + return "deny", ( + f"This write targets {t[0]}/{t[1]}, which is not this checkout's origin " + f"({origin[0]}/{origin[1]}). Write only to the current project's own repository. " + "Another repository needs explicit per-session permission. See AGENTS.md " + "'Repository Boundaries and Write Safety'." + ) + + return "allow", "" + + +# --- Self-test --------------------------------------------------------------------------------------- +_CASES = [ + # (command, expected_decision, label) + ("gh api graphql -f query='mutation($t:ID!){addPullRequestReviewThreadReply(input:{pullRequestReviewThreadId:$t,body:\"x\"}){comment{id}}}' -F t=\"PRRT_kwDODvuuzM6SFvx0\" >/dev/null 2>&1 || true", "deny", "the incident: suppressed + literal id"), + ("gh api graphql -f query='mutation($t:ID!){resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"PRRT_kwDOabc123def\"", "deny", "literal node id in a mutation"), + ("gh api graphql -f query='mutation($t:ID!){resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"$TID\"", "allow", "mutation with captured $TID"), + ("gh issue comment 5 -R mankatcheung/job-finder --body \"hi\"", "deny", "cross-origin explicit -R"), + ("gh issue comment 5 -R \"mankatcheung/job-finder\" --body \"hi\"", "deny", "cross-origin quoted -R"), + ("gh pr create --title x --body y >/dev/null 2>&1", "deny", "suppressed gh pr create"), + ("gh api repos/ptr727/PlexCleaner/issues/1/comments -f body=\"ok\"", "allow", "gh api POST to origin"), + ("gh api graphql -f query='{repository(owner:\"o\",name:\"r\"){pullRequest(number:1){reviewThreads(first:100){nodes{id}}}}}'", "allow", "graphql READ query"), + ("gh pr view 5 --json reviews", "allow", "gh pr view (read)"), + ("return 1 2>/dev/null || exit 1", "allow", "shell guard, not a gh write"), + ("git push origin develop", "allow", "normal push (no suppression, no cross-repo)"), + ("git commit -m 'x' && git push >/dev/null 2>&1", "deny", "push with discarded output"), + ("gh issue comment 5 --body x 2>&1 | tee out.log", "allow", "bare 2>&1 piped to tee is not suppression"), + ("gh pr comment 5 --body ok 2>&1", "allow", "bare 2>&1 leaves output visible"), + ("gh api repos/ptr727/PlexCleaner/issues/1/comments -f body=x 2>/dev/null", "deny", "stderr discarded on a write"), + ("gh issue comment 5 --body \"run make || true to skip errors\"", "allow", "|| true inside a quoted body is not a tail"), + ("gh pr comment 5 --body \"pipe noisy output to >/dev/null\"", "allow", ">/dev/null inside a quoted body is not a redirect"), + ("gh issue comment 5 --body \"see notes\" >/dev/null", "deny", "real redirect after a quoted body still denies"), + ("gh issue comment 5 --body \"he said \\\"pipe to >/dev/null\\\" today\"", "allow", "escaped quotes in a body do not end the span early"), + ("gh pr close 5 || :", "deny", "force-success no-op tail on a write"), + ("gh pr comment 5 --body x || echo done", "deny", "force-success echo tail on a write"), + ("gh api graphql -f query='mutation{addPullRequestReviewThreadReply(input:{pullRequestReviewThreadId:$t,body:$b}){comment{id}}}' -F t=\"$TID\" -F b=\"fixed_the_underscore_bug_here\"", "allow", "underscored reply body is not a node id"), + ("gh api graphql -f query='mutation{resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"TODO_fixit\"", "allow", "short all-caps token is not a node id"), +] + + +def _selftest(): + # Deterministic offline run: pin origin to ptr727/PlexCleaner (the incident repo) so the + # cross-origin case resolves without touching a real checkout. + origin = ("ptr727", "plexcleaner") + ok = True + for cmd, want, label in _CASES: + got, _ = classify(cmd, origin=origin) + mark = "ok " if got == want else "FAIL" + if got != want: + ok = False + print(f" {mark} [{got:5}] want={want:5} {label}") + print("SELFTEST PASS" if ok else "SELFTEST FAIL") + return 0 if ok else 1 + + +# --- Hook entrypoint (PreToolUse) -------------------------------------------------------------------- +def _main(): + try: + data = json.load(sys.stdin) + except Exception: + sys.exit(0) # not our event shape - do not interfere + if data.get("tool_name") != "Bash": + sys.exit(0) + cmd = (data.get("tool_input") or {}).get("command", "") + cwd = data.get("cwd") or os.getcwd() + decision, reason = classify(cmd, cwd) + if decision == "deny": + # Documented PreToolUse deny contract (confirm field names against current docs before shipping). + print(json.dumps({ + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": reason, + } + })) + sys.exit(0) + sys.exit(0) + + +if __name__ == "__main__": + if "--selftest" in sys.argv: + sys.exit(_selftest()) + _main() diff --git a/host-setup/agent-safety/install.ps1 b/host-setup/agent-safety/install.ps1 new file mode 100644 index 00000000..2274fea6 --- /dev/null +++ b/host-setup/agent-safety/install.ps1 @@ -0,0 +1,30 @@ +# Thin wrapper: run the cross-platform installer with a Python 3 (Windows). +# All logic lives in install.py so every OS runs one tested code path. Idempotent, safe to re-run. +# .\install.ps1 +# $env:CLAUDE_HOME = "C:\path"; .\install.ps1 # override the target (testing) +$ErrorActionPreference = "Stop" +$here = Split-Path -Parent $MyInvocation.MyCommand.Path +$script = Join-Path $here "install.py" + +# Prefer launchers that are unambiguously Python 3. install.py and the hook use Python 3 syntax, so a +# bare `python` (Python 2 on some systems) is the last resort. +if (Get-Command "py" -ErrorAction SilentlyContinue) { + & py -3 $script @args +} elseif (Get-Command "python3" -ErrorAction SilentlyContinue) { + & python3 $script @args +} elseif (Get-Command "python" -ErrorAction SilentlyContinue) { + # Verify a bare `python` is Python 3 before handing it Python 3 syntax - it is Python 2 on some setups, + # which would fail to parse install.py. py -3 and python3 above are Python 3 by construction. + & python -c "import sys; sys.exit(0 if sys.version_info[0] == 3 else 1)" 2>$null + if ($LASTEXITCODE -ne 0) { + Write-Error "Found python on PATH but it is not Python 3 (tried py -3, python3, python). Install Python 3." + exit 1 + } + & python $script @args +} else { + Write-Error "Python 3 is required and was not found on PATH (tried py -3, python3, python)." + exit 1 +} + +# Propagate the installer's exit code - a native command's non-zero exit does not stop the script. +exit $LASTEXITCODE diff --git a/host-setup/agent-safety/install.py b/host-setup/agent-safety/install.py new file mode 100644 index 00000000..05896972 --- /dev/null +++ b/host-setup/agent-safety/install.py @@ -0,0 +1,108 @@ +#!/usr/bin/env python3 +"""Install the agent write-safety kit for the current user account. Cross-platform, idempotent. + +Deploys the PreToolUse hook, registers it in the user settings.json, adds the safety rules to the user +CLAUDE.md (marker-delimited so re-runs update in place), and self-tests the hook before registering it. +The bash and PowerShell wrappers both call this, so every OS runs one tested code path. + +Usage: python3 install.py (installs to ~/.claude) + CLAUDE_HOME=/x python3 install.py (override target, for testing) +""" +import json +import os +import pathlib +import re +import shutil +import subprocess +import sys + +HERE = pathlib.Path(__file__).resolve().parent + + +def hook_launcher(): + """A python invocation for the settings.json command. Prefer a bare `python3` (portable and + unambiguously Python 3), else this interpreter's absolute path (guaranteed the Python 3 running the + installer). Never a bare `python`, which is Python 2 on some systems and would fail the hook's + Python 3 syntax.""" + if shutil.which("python3"): + return "python3" + return sys.executable + + +def main(): + if sys.version_info < (3, 7): + sys.stderr.write("This installer and the hook require Python 3.7+. Run it with python3.\n") + return 1 + # expanduser so a CLAUDE_HOME set to a `~/...` form resolves to the home dir, not a literal `~` dir. + claude_home_env = os.environ.get("CLAUDE_HOME") + claude_home = pathlib.Path(claude_home_env).expanduser() if claude_home_env else pathlib.Path.home() / ".claude" + hooks_dir = claude_home / "hooks" + hook_dst = hooks_dir / "gh-write-guard.py" + settings = claude_home / "settings.json" + claude_md = claude_home / "CLAUDE.md" + + print(f"Installing agent write-safety kit into: {claude_home}") + hooks_dir.mkdir(parents=True, exist_ok=True) + + # 1. Deploy the hook and self-test it BEFORE wiring anything up. + shutil.copyfile(HERE / "gh-write-guard.py", hook_dst) + try: + os.chmod(hook_dst, 0o755) + except OSError: + pass + print(f" hook -> {hook_dst}") + r = subprocess.run([sys.executable, str(hook_dst), "--selftest"], capture_output=True, text=True) + if r.returncode != 0: + sys.stderr.write("Hook self-test FAILED; aborting before registration.\n" + r.stdout + r.stderr) + return 1 + print(" hook self-test: PASS") + + # 2. Register our hook command in settings.json so exactly one PreToolUse/Bash group carries it. + launcher = hook_launcher() + # Quote the launcher too: the sys.executable fallback can contain spaces (e.g. C:\Program Files\...). + hook_cmd = f'"{launcher}" "{hook_dst}"' + data = {} + if settings.exists() and settings.read_text(encoding="utf-8").strip(): + try: + data = json.loads(settings.read_text(encoding="utf-8")) + except json.JSONDecodeError as e: + sys.stderr.write( + f"{settings} exists but is not valid JSON ({e}). Fix or remove it, then re-run.\n" + ) + return 1 + pre = data.setdefault("hooks", {}).setdefault("PreToolUse", []) + # Strip our hook from every existing group first, so a re-run never leaves a duplicate behind even + # when settings.json already has more than one Bash group. Then register it in a single Bash group. + for g in pre: + hooks_list = g.get("hooks") + if isinstance(hooks_list, list): + hooks_list[:] = [h for h in hooks_list if "gh-write-guard" not in str(h.get("command", ""))] + group = next((g for g in pre if g.get("matcher") == "Bash"), None) + if group is None: + group = {"matcher": "Bash", "hooks": []} + pre.append(group) + group.setdefault("hooks", []).append({"type": "command", "command": hook_cmd}) + settings.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + print(f" settings -> {settings} (PreToolUse/Bash hook registered)") + + # 3. CLAUDE.md: replace the agent-safety marker block if present, else append it. + snippet = (HERE / "claude-md-safety.md").read_text(encoding="utf-8").strip() + existing = claude_md.read_text(encoding="utf-8") if claude_md.exists() else "" + block_re = re.compile(r".*?", re.S) + if block_re.search(existing): + updated, action = block_re.sub(lambda _: snippet, existing), "updated" + else: + sep = "" if existing == "" or existing.endswith("\n\n") else ("\n" if existing.endswith("\n") else "\n\n") + updated, action = existing + sep + snippet + "\n", "appended" + claude_md.write_text(updated, encoding="utf-8") + print(f" CLAUDE.md -> {claude_md} (safety block {action})") + + print("\nDone. Verify:") + print(f" {launcher} \"{hook_dst}\" --selftest") + print(f" grep -c 'agent-safety v' \"{claude_md}\" # expect 2") + print("Restart Claude Code sessions on this machine so the hook and CLAUDE.md load.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/host-setup/agent-safety/install.sh b/host-setup/agent-safety/install.sh new file mode 100755 index 00000000..d52c88d0 --- /dev/null +++ b/host-setup/agent-safety/install.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +# Thin wrapper: run the cross-platform installer with a Python 3 (Linux / WSL / macOS / Proxmox). +# All logic lives in install.py so every OS runs one tested code path. Idempotent, safe to re-run. +# ./install.sh installs to ~/.claude +# CLAUDE_HOME=/x ./install.sh overrides the target (testing) +set -Eeuo pipefail +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +# Pick the first candidate that is actually Python 3 - install.py and the hook use Python 3 syntax, so a +# bare `python` that is Python 2 must be rejected, not handed the script (it would fail on import). +py="" +for c in python3 python; do + if command -v "$c" >/dev/null 2>&1 && "$c" -c 'import sys; raise SystemExit(0 if sys.version_info[0] == 3 else 1)' 2>/dev/null; then + py="$c"; break + fi +done +[ -n "$py" ] || { echo "Python 3 is required and was not found on PATH (tried python3, python)." >&2; exit 1; } + +exec "$py" "$here/install.py" "$@" diff --git a/spec/files.json b/spec/files.json index b622105d..fb9760c3 100644 --- a/spec/files.json +++ b/spec/files.json @@ -2,7 +2,7 @@ "$schema": "./files.schema.json", "note": "The standardization baseline: files and sections a fleet repo is expected to carry, and their intent authority. The audit checks presence (letter) and equivalence (intent); a section for an absent language or target is N/A.", "baseline": [ - { "path": "AGENTS.md", "sections": ["Git and Commit Rules", "Branching Model", "Release Model", "Pull Request Title and Commit Message Conventions", "Documentation Style Conventions", "PR Review Etiquette", "Workflow YAML Conventions"], "intentRef": "AGENTS.md", "appliesTo": "*" }, + { "path": "AGENTS.md", "sections": ["Repository Boundaries and Write Safety", "Git and Commit Rules", "Branching Model", "Release Model", "Pull Request Title and Commit Message Conventions", "Documentation Style Conventions", "PR Review Etiquette", "Workflow YAML Conventions"], "intentRef": "AGENTS.md", "appliesTo": "*" }, { "path": "CODESTYLE.md", "whole": true, "placeholders": ["InternalsVisibleTo project names"], "intentRef": "CODESTYLE.md", "appliesTo": "*" }, { "path": "WORKFLOW.md", "whole": true, "intentRef": "WORKFLOW.md", "appliesTo": "*" }, { "path": "README.md", "appliesTo": "*" },