From 06e0e128cb647c419ee859e230680eb7c3869e00 Mon Sep 17 00:00:00 2001 From: Kodflow <133899878+kodflow@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:27:56 +0200 Subject: [PATCH 1/5] feat(gate): refuse an agent's configuration tracked in the tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `.claude/` reached a trunk and merged. Nothing in the gate looked at what a change leaves on disk: the attribution rules read messages and identities, the secret scan reads added lines, and a committed directory was invisible to all of them. scripts/agent-paths.txt is the path counterpart of patterns.txt β€” `.claude/`, `.cursor/`, `.codex/`, `.windsurf/`, `.aider.*`, `.roo/`, `.goose/`, `.amazonq/`, `.mcp.json` and the rest. It is a default where patterns-strict.txt is opt-in, and the asymmetry is the point: a commit MESSAGE naming `.claude/` is ordinary work, 25 of the template's last 60 legitimate subjects match it, while the string `.claude/settings.json` in the tree IS the artefact. Same characters, no prose to be fair to. Scope is the tree at the head, not the range. The directory that prompted this was merged long before the rule existed, and a check scoped to what a change adds would call every later pull request clean while it sat there. That scope is affordable because the remedy is cheap: `git rm -r --cached` and one commit, no history rewrite. It is the only refusal here that does not demand one. Editor configuration, `.devcontainer/` itself and markdown instructions are not matched. Precisely: nothing is matched BECAUSE it sits under `.devcontainer/` β€” an agent directory nested there is still one, or "put it under .devcontainer/" would be the way around every rule in the file. A repository that exists to distribute this configuration exempts the exact paths it ships with `agent_files_allow`. The verdict collapses hits to the shortest prefix that still matches rather than to the first dot-directory: the fleet's devcontainer ships its payload at `.devcontainer/images/.claude/`, and naming `.devcontainer/` as the thing to delete would point at 400 files the gate has no quarrel with. --- .gitignore | 2 + README.md | 56 ++++++++++++- action.yml | 23 +++++- scripts/agent-paths.txt | 105 ++++++++++++++++++++++++ scripts/post-commit.sh | 159 +++++++++++++++++++++++++++++++++++-- scripts/rewrite-history.sh | 11 ++- tests/run.sh | 91 +++++++++++++++++++++ 7 files changed, 431 insertions(+), 16 deletions(-) create mode 100644 .gitignore create mode 100644 scripts/agent-paths.txt diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..cfb921e --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +# The gate refuses a tracked agent configuration; this repo is not exempt. +.claude/ diff --git a/README.md b/README.md index 1edd09e..0ef05aa 100644 --- a/README.md +++ b/README.md @@ -16,13 +16,19 @@ on GitHub, against commits that already exist. There is nothing to skip. | **Identity** | **every commit reachable from the branch** (default) | a commit's author or committer is not one of the `authors` accounts. Opt-in: an empty list disables it | | **AI attribution** | **every commit reachable from the branch** (default) | any commit's message, author or committer carries an attribution: `Co-authored-by:` naming an assistant, a "Generated by/with …" banner, the πŸ€– footer, an `noreply@anthropic.com` identity, "AI-assisted", a `Plan:` footer | | **Format** | the commits this push/PR introduces (merges skipped) | a subject is not `type(scope)!: subject` with a conventional type | -| **Secrets** | lines this push/PR *adds*, outside test/fixture paths | a line looks like a credential (AWS key, GitHub token, private-key header, `password = "…"`, …) | +| **Secrets** | lines this push/PR *adds*, outside test/fixture paths | a line looks like a credential (AWS key, GitHub token, private-key header, a quoted `password` assignment, …) | +| **Agent artefacts** | **every path tracked in the tree at the head** | an AI agent's tooling configuration is committed: `.claude/`, `.cursor/`, `.cursorrules`, `.windsurf/`, `.codex/`, `.gemini/`, `.aider.*`, `.roo/`, `.cline/`, `.goose/`, `.amazonq/`, `.kiro/`, `.specstory/`, `.mcp.json`, … | Default attribution patterns match the **shape of an attribution**, not the mention of a vendor. Measured on `devcontainer-template`'s real history, a bare `.claude/` rule matches 25 of the last 60 legitimate commits. Keyword rules exist but are opt-in (`strict: true`). +That caution is about **prose**, and the artefact check is the other side of +it. A commit message naming `.claude/` is ordinary work; the string +`.claude/settings.json` in the tree *is* the thing. Same characters, no +ambiguity to be fair to β€” so paths are a default while keywords are not. + **Deliberately not here: lint, build, test.** Every repository's own CI already runs those on GitHub, so `--no-verify` never bypassed them. @@ -64,6 +70,44 @@ The gate reports and never rewrites; see [Rewriting history](#rewriting-history) Set `history: range` to only check the commits a push/PR introduces. +### Agent artefacts: the tree, and a cheap way out + +The artefact check reads the **tree at the head**, not the range. That is the +whole point of it: the `.claude/` that prompted the rule was merged into a +trunk long before the rule existed, and a check scoped to what a change *adds* +would call every later pull request clean while the directory sat there. The +gate stays red until the files are gone. + +It can afford that scope because gone is cheap here, and this is the one place +the gate refuses something without demanding a rewrite: + +```sh +git rm -r --cached .claude && echo '.claude/' >> .gitignore +git commit -m "chore: untrack the agent configuration" +``` + +The files stay on your machine; the repository stops carrying them. Nothing +walks the ancestry, so that single commit ends it β€” unlike an attribution, +which lives in a commit and needs `scripts/rewrite-history.sh`. + +**Not matched, by decision rather than by omission.** Editor configuration β€” +`.vscode/`, `.idea/`, `.zed/` β€” is untouched: an editor is not an agent, and a +repository's editor settings long predate all of this. Neither is +`.devcontainer/` itself, which humans and CI read too. Neither are markdown +instructions (`CLAUDE.md`, `AGENTS.md`, `GEMINI.md`): 21 of the fleet's 55 +repositories carry one, and whether that prose belongs in a repository is an +editorial call a gate is the wrong place to make. + +Read that claim precisely: nothing is matched *because* it sits under +`.devcontainer/`. An agent directory nested there is still an agent directory β€” +the fleet's own template ships one at `.devcontainer/images/.claude/` β€” and +exempting a parent would make "put it under `.devcontainer/`" the way around +every rule in the file. + +A repository whose purpose *is* to distribute this configuration exempts the +exact paths it ships with `agent_files_allow`, rather than by weakening the +list everyone else runs. + ## Install `.github/workflows/post-commit.yml` in the target repo (`stub/post-commit.yml` here): @@ -100,6 +144,8 @@ future fix. Don't pin it, don't copy the logic in. | `strict` | `false` | also apply `scripts/patterns-strict.txt` (vendor keywords, `.claude/` paths) | | `format` | `true` | conventional-commit subjects | | `secrets` | `true` | credential scan on added lines | +| `agent_files` | `true` | refuse an agent's tooling configuration tracked in the tree | +| `agent_files_allow` | *(empty)* | paths exempt from that check, space or comma separated. A bare entry exempts the whole subtree: `.claude` covers `.claude/settings.json` | | `token` | `github.token` | checkout token | ## Making it mandatory β€” and irremovable @@ -184,6 +230,7 @@ action.yml the action (composite): checkout β†’ resolve β†’ g scripts/post-commit.sh the gate scripts/patterns.txt default forbidden patterns (attribution-shaped) scripts/patterns-strict.txt opt-in keyword patterns +scripts/agent-paths.txt agent tooling paths (.claude/, .cursor/, …); editor config kept scripts/enforce.sh fleet: stub PR + ruleset, idempotent scripts/rewrite-history.sh history scrub (messages + identities), dry-run by default stub/post-commit.yml the file installed in each repo @@ -195,6 +242,7 @@ tests/run.sh behaviour tests against real throwaway repos ## Adding a rule -Edit `scripts/patterns.txt`, add a case to `tests/run.sh`, open a PR β€” this -repo gates itself with the version under review. Once on `main`, the rule is -live everywhere on the next run. +Edit `scripts/patterns.txt` for a rule about what a commit *says*, or +`scripts/agent-paths.txt` for one about what it *tracks*. Add a case to +`tests/run.sh`, open a PR β€” this repo gates itself with the version under +review. Once on `main`, the rule is live everywhere on the next run. diff --git a/action.yml b/action.yml index b3d8d5f..6d721bc 100644 --- a/action.yml +++ b/action.yml @@ -5,8 +5,9 @@ name: post-commit description: >- Mandatory merge gate: every commit authored by an allowed account, no AI - attribution anywhere in history, conventional commit subjects, no - credentials added. Logic lives in kodflow/post-commit β€” + attribution anywhere in history, no agent configuration tracked in the tree, + conventional commit subjects, no credentials added. Logic lives in + kodflow/post-commit β€” callers pin @main so a fix there is live everywhere on the next run. branding: icon: shield @@ -35,6 +36,22 @@ inputs: secrets: description: Scan the range's added lines for credential shapes. default: 'true' + agent_files: + description: >- + Refuse an AI agent's tooling configuration tracked in the repository β€” + .claude/, .cursor/, .windsurf/, .aider.*, .codex/, … (full list: + scripts/agent-paths.txt). The tree at the head is what is read, not the + range, so the check stays red until the files are gone; one `git rm + --cached` clears it, no history rewrite. Editor configuration (.vscode/, + .idea/), .devcontainer/ and markdown instructions (CLAUDE.md, AGENTS.md) + are never matched. + default: 'true' + agent_files_allow: + description: >- + Space/comma separated paths exempt from the agent-artefact check, for a + repository whose purpose is to distribute that configuration. A bare + entry exempts the whole subtree: `.claude` covers `.claude/settings.json`. + default: '' token: description: Token used to check out the calling repository. default: ${{ github.token }} @@ -137,6 +154,8 @@ runs: PC_HISTORY: ${{ inputs.history }} PC_FORMAT: ${{ inputs.format }} PC_SECRETS: ${{ inputs.secrets }} + PC_AGENT_FILES: ${{ inputs.agent_files }} + PC_AGENT_ALLOW: ${{ inputs.agent_files_allow }} PC_REPORT: ${{ runner.temp }}/post-commit-report.md run: | rc=0 diff --git a/scripts/agent-paths.txt b/scripts/agent-paths.txt new file mode 100644 index 0000000..4219491 --- /dev/null +++ b/scripts/agent-paths.txt @@ -0,0 +1,105 @@ +# Agent artefact paths β€” one ERE per line, matched case-insensitively against +# every path tracked at the scanned head. Blank lines and #-comments ignored. +# +# Single source of truth for the fleet: a rule added here is live in every +# repo pinned to kodflow/post-commit@main on its next run. +# +# DESIGN β€” a tracked PATH, not a mention. `.claude/` had to stay opt-in in +# patterns.txt for a precise reason: a commit MESSAGE naming it is ordinary +# work ("fix: update .claude/scripts/…"), and 25 of devcontainer-template's +# last 60 legitimate subjects match. A tracked path is the opposite case. The +# string `.claude/settings.json` in the tree IS the artefact; there is nothing +# to misread, no prose to be fair to. Same characters, no false-positive +# problem β€” so this file is a default while patterns-strict.txt is not. +# +# The remedy differs too, and that is why the scope can afford to be the whole +# tree. An attribution lives in a commit and only a history rewrite removes it. +# A tracked file is removed by removing it: +# +# git rm -r --cached .claude && echo '.claude/' >> .gitignore && commit +# +# The check reads the tree at the head, never the ancestry, so that one commit +# clears it for good. +# +# SCOPE β€” an agent's tooling configuration. Deliberately NOT matched, and by +# decision rather than by omission: +# +# Β· .vscode/ .idea/ .zed/ .fleet/ .helix/ β€” editor configuration. An editor +# is not an agent, and a repository's editor settings long predate all of +# this. Whatever else changes here, these stay out. +# Β· .devcontainer/ β€” the environment definition. Humans and CI read it too. +# Note the exact claim: nothing is matched BECAUSE it sits under +# .devcontainer/. An agent directory nested there is still an agent +# directory β€” the fleet's own template ships one at +# .devcontainer/images/.claude/ β€” and exempting a parent would make +# "put it under .devcontainer/" the way around every rule in this file. +# Β· CLAUDE.md AGENTS.md GEMINI.md .github/copilot-instructions.md β€” markdown +# instructions read as project documentation, and 21 fleet repositories +# carry one. Whether that prose belongs in a repository is an editorial +# call; a gate is the wrong place to make it. +# +# A repository whose purpose IS to distribute this configuration β€” +# devcontainer-template β€” exempts itself with the `agent_files_allow` input +# rather than by weakening the list here. + +# --- Claude Code ------------------------------------------------------------ +(^|/)\.claude/ +(^|/)\.claude\.json$ +(^|/)\.claudeignore$ +# The project-level MCP server list: agent tooling by definition. Bare +# `mcp.json` is NOT matched β€” it is the ordinary config file name of every +# repository that implements an MCP server, several of which are ours. +(^|/)\.mcp\.json$ +# --- Cursor ----------------------------------------------------------------- +(^|/)\.cursor/ +(^|/)\.cursorrules$ +(^|/)\.cursorignore$ +(^|/)\.cursorindexingignore$ +# --- OpenAI Codex ----------------------------------------------------------- +(^|/)\.codex/ +# --- GitHub Copilot --------------------------------------------------------- +(^|/)\.copilot/ +# --- Gemini CLI / Code Assist ----------------------------------------------- +(^|/)\.gemini/ +(^|/)\.aiexclude$ +# --- Windsurf / Codeium ----------------------------------------------------- +(^|/)\.windsurf/ +(^|/)\.windsurfrules$ +(^|/)\.codeium/ +(^|/)\.codeiumignore$ +# --- aider ------------------------------------------------------------------ +# One pattern for the lot: aider prefixes every dropping it leaves behind β€” +# .aider.chat.history.md, .aider.input.history, .aider.tags.cache.v3/, +# .aider.conf.yml β€” with the same string. +(^|/)\.aider[.-] +(^|/)\.aiderignore$ +# --- Cline / Roo Code / Kilo Code ------------------------------------------- +(^|/)\.cline/ +# .clinerules is a file in older versions and a directory in newer ones. +(^|/)\.clinerules(/|$) +(^|/)\.roo/ +(^|/)\.roomodes$ +(^|/)\.kilocode/ +# --- Continue --------------------------------------------------------------- +(^|/)\.continue/ +# --- Goose ------------------------------------------------------------------ +(^|/)\.goose/ +(^|/)\.goosehints$ +# --- opencode / Crush / Trae / Junie / Augment ------------------------------ +(^|/)\.opencode/ +(^|/)\.crush/ +(^|/)\.trae/ +(^|/)\.junie/ +(^|/)\.augment/ +(^|/)\.augmentignore$ +# --- Amazon Q Developer / Kiro ---------------------------------------------- +(^|/)\.amazonq/ +(^|/)\.kiro/ +# --- Qodo / SpecStory / Devin / Tabnine / Plandex --------------------------- +(^|/)\.qodo/ +(^|/)\.specstory/ +(^|/)\.devin/ +(^|/)\.tabnine/ +(^|/)\.plandex/ +# --- JetBrains AI ----------------------------------------------------------- +(^|/)\.aiignore$ diff --git a/scripts/post-commit.sh b/scripts/post-commit.sh index 5b822cc..2b1766e 100755 --- a/scripts/post-commit.sh +++ b/scripts/post-commit.sh @@ -26,6 +26,13 @@ # 4. format β€” conventional-commit subject on the range's non-merge # commits (project convention, see devcontainer-template). # 5. secrets β€” no credential-shaped ADDED lines in the range's diff. +# 6. artefacts β€” no AI agent's tooling configuration TRACKED in the tree at +# the head (.claude/, .cursor/, .aider.*, …). The tree and +# not the range: the directory this rule exists to remove +# was merged long before the rule existed, and a range check +# would call every later pull request clean while it sat +# there. Editor configuration is untouched β€” an editor is +# not an agent. # # Deliberately NOT here: lint/build/test. Every repo's own CI already runs # those server-side, so --no-verify never bypassed them in the first place. @@ -47,6 +54,8 @@ SECRETS="${PC_SECRETS:-true}" HISTORY="${PC_HISTORY:-full}" FORMAT="${PC_FORMAT:-true}" AUTHORS="${PC_AUTHORS:-}" +AGENT_FILES="${PC_AGENT_FILES:-true}" +AGENT_ALLOW="${PC_AGENT_ALLOW:-}" MAX_REPORT="${PC_MAX_REPORT:-50}" SUMMARY="${GITHUB_STEP_SUMMARY:-/dev/null}" # Second sink for the same markdown. The job summary is only read by @@ -68,12 +77,14 @@ fi # Comments and blank lines stripped. Case-insensitivity is applied once, by # the runner, instead of being baked into each pattern. PATTERNS=() -load_patterns() { +AGENT_PATTERNS=() +load_patterns() { # load_patterns [] local f="$1" line + local -n dest="${2:-PATTERNS}" [ -r "$f" ] || { echo "::error::pattern file not readable: $f" >&2; exit 2; } while IFS= read -r line || [ -n "$line" ]; do case "$line" in ''|'#'*) continue ;; esac - PATTERNS+=("$line") + dest+=("$line") done < "$f" } load_patterns "$SCRIPT_DIR/patterns.txt" @@ -82,6 +93,17 @@ if [ "${#PATTERNS[@]}" -eq 0 ]; then echo "::error::no patterns loaded β€” refusing to report a false clean" >&2 exit 2 fi +# Path patterns are a separate list, matched against tracked paths rather than +# against message text β€” the two never share a pattern, so they never share a +# file either. Empty is treated as an error for the same reason as above: a +# check that silently matches nothing reports a clean it never verified. +if [ "$AGENT_FILES" = "true" ]; then + load_patterns "$SCRIPT_DIR/agent-paths.txt" AGENT_PATTERNS + if [ "${#AGENT_PATTERNS[@]}" -eq 0 ]; then + echo "::error::no agent path patterns loaded β€” refusing to report a false clean" >&2 + exit 2 + fi +fi # --- Allowed identities ----------------------------------------------------- # PC_AUTHORS is a space/comma separated list of GitHub logins. Each expands to @@ -106,9 +128,10 @@ if [ -n "$AUTHORS" ]; then fi ATTR_FILE="$(mktemp)"; FMT_FILE="$(mktemp)"; SEC_FILE="$(mktemp)"; IDENT_FILE="$(mktemp)" -TRAIL_FILE="$(mktemp)"; REPORT_BODY="$(mktemp)" -trap 'rm -f "$ATTR_FILE" "$FMT_FILE" "$SEC_FILE" "$IDENT_FILE" "$TRAIL_FILE" "$REPORT_BODY"' EXIT -ATTR_N=0; ATTR_SCANNED=0; FMT_N=0; FMT_SCANNED=0; SEC_N=0; IDENT_N=0; TRAIL_N=0 +TRAIL_FILE="$(mktemp)"; AGENT_FILE="$(mktemp)"; REPORT_BODY="$(mktemp)" +trap 'rm -f "$ATTR_FILE" "$FMT_FILE" "$SEC_FILE" "$IDENT_FILE" "$TRAIL_FILE" "$AGENT_FILE" "$REPORT_BODY"' EXIT +ATTR_N=0; ATTR_SCANNED=0; FMT_N=0; FMT_SCANNED=0; SEC_N=0; IDENT_N=0; TRAIL_N=0; AGENT_N=0 +AGENT_ROOTS=""; AGENT_ROOT_N=0 # Identity trailers, whatever the case. An address here is as permanent as the # author field, so the same allow-list applies to both. Accounts whose display @@ -242,6 +265,85 @@ if [ "$SECRETS" = "true" ] && [ -n "$RANGE" ]; then | grep -E '^\+' | grep -Ev '^\+\+\+' | grep -iE -- "$SECRET_RE" | head -40) fi +# --- 4. Agent artefacts (the tree at the head) ------------------------------- +# Scope is the tree, not the range, and that is the whole point. The `.claude/` +# that prompted this rule was merged into a trunk months before the rule +# existed; a range check sees only what a change adds, so every later pull +# request would have been called clean while the directory sat there. Reading +# the tracked paths means the gate stays red until it is actually gone. +# +# Which it can afford to be, because gone is cheap here. A tainted commit +# message needs rewrite-history.sh and new SHAs for every descendant; a tracked +# file needs one `git rm -r --cached` and a commit. The check never looks at +# the ancestry, so that commit ends it. +# +# --branches is the whole-repository audit scope (push, manual runs, +# rewrite-history.sh) and is not a tree-ish. There, the checked-out head is the +# tree to judge β€” on push that is exactly the commit that was pushed. +TREE_REV="$HEAD_REV"; [ "$TREE_REV" = "--branches" ] && TREE_REV=HEAD +if [ "$AGENT_FILES" = "true" ]; then + # One combined alternation rather than a grep per pattern: a large + # repository has tens of thousands of tracked paths, and forty passes over + # them is forty times the work for the same answer. Nothing is lost by not + # recording which alternative fired β€” unlike a message match, the path is + # its own evidence. + AGENT_RE="" + for pattern in "${AGENT_PATTERNS[@]}"; do AGENT_RE="${AGENT_RE}|(${pattern})"; done + AGENT_RE="${AGENT_RE#|}" + TREE="$(git ls-tree -r --name-only "$TREE_REV" 2>/dev/null)" || { + echo "::error::git ls-tree failed for '$TREE_REV' β€” no such tree?" >&2 + exit 2 + } + while IFS= read -r p; do + [ -z "$p" ] && continue + ALLOWED="" + for allow in ${AGENT_ALLOW//,/ }; do + # A bare entry exempts the whole subtree, so `.claude` and + # `.claude/*` both mean what whoever wrote the input expects. `*` + # crosses `/` inside [[ == ]], so no globstar is involved. + # shellcheck disable=SC2053 + if [[ "$p" == $allow || "$p" == $allow/* ]]; then ALLOWED=y; break; fi + done + [ -n "$ALLOWED" ] && continue + AGENT_N=$((AGENT_N + 1)) + printf '%s\n' "$p" >> "$AGENT_FILE" + done < <(printf '%s\n' "$TREE" | grep -iE -- "$AGENT_RE") + # Every hit is recorded, uncapped, because the report does not show them + # one by one. A `.claude/` holding two hundred files is ONE thing to + # delete, not two hundred lines of verdict, so each path collapses to its + # artefact root and the roots are counted. MAX_REPORT then caps roots + # rather than files, which is what a reader was going to act on anyway. + # The per-file GitHub annotations below stay per-file: those anchor on the + # path and are read by a machine. + # + # The root is the SHORTEST PREFIX THAT STILL MATCHES, not the first + # dot-directory along the path, and the difference is not cosmetic. The + # fleet's devcontainer ships its agent configuration at + # `.devcontainer/images/.claude/…`; collapsing on the first dot-directory + # would name `.devcontainer/` as the thing to delete β€” a directory this + # gate promises never to touch, holding 400 files it has no quarrel with. + # A verdict that names the wrong directory is worse than none. + # + # tolower() on the candidate is how the case-insensitivity of the grep + # above is carried into awk, which has no equivalent flag. A pattern + # written with an uppercase letter would simply not collapse β€” the path is + # then reported whole, which is accurate, just longer. + if [ "$AGENT_N" -gt 0 ]; then + AGENT_ROOTS="$(awk -F/ -v re="$AGENT_RE" '{ + r = ""; root = $0 + for (i = 1; i <= NF; i++) { + r = (r == "" ? $i : r "/" $i) + cand = (i < NF ? r "/" : r) + if (tolower(cand) ~ re) { root = cand; break } + } + if (!(root in n)) order[++k] = root + n[root]++ + } + END { for (j = 1; j <= k; j++) print n[order[j]] "\t" order[j] }' "$AGENT_FILE")" + AGENT_ROOT_N="$(printf '%s\n' "$AGENT_ROOTS" | wc -l)" + fi +fi + # --- Report ----------------------------------------------------------------- esc() { printf '%s' "$1" | sed 's/|/\\|/g; s/`/ΚΌ/g'; } { @@ -253,9 +355,11 @@ esc() { printf '%s' "$1" | sed 's/|/\\|/g; s/`/ΚΌ/g'; } echo "Attribution: **$ATTR_SCANNED** commit(s) in \`$RANGE\`." fi [ -n "$RANGE" ] && echo "Format: **$FMT_SCANNED** non-merge commit(s) in \`$RANGE\`. Secrets: added lines in \`$RANGE\`." + [ "$AGENT_FILES" = "true" ] && echo "Agent artefacts: paths tracked at \`${TREE_REV:0:12}\`." echo "" - if [ "$ATTR_N" -eq 0 ] && [ "$FMT_N" -eq 0 ] && [ "$SEC_N" -eq 0 ] && [ "$IDENT_N" -eq 0 ] && [ "$TRAIL_N" -eq 0 ]; then - echo "βœ… Clean β€” allowed identities, no AI attribution, conventional subjects, no credentials added." + if [ "$ATTR_N" -eq 0 ] && [ "$FMT_N" -eq 0 ] && [ "$SEC_N" -eq 0 ] && [ "$IDENT_N" -eq 0 ] \ + && [ "$TRAIL_N" -eq 0 ] && [ "$AGENT_N" -eq 0 ]; then + echo "βœ… Clean β€” allowed identities, no AI attribution, conventional subjects, no credentials added, no agent artefacts tracked." fi if [ "$TRAIL_N" -gt 0 ]; then echo "### ❌ Foreign identity in a trailer β€” $TRAIL_N commit(s)" @@ -325,6 +429,32 @@ esc() { printf '%s' "$1" | sed 's/|/\\|/g; s/`/ΚΌ/g'; } while IFS= read -r l; do printf -- '- `%s…`\n' "$(printf '%s' "$l" | cut -c1-40 | tr -d '`')"; done < "$SEC_FILE" echo "" fi + if [ "$AGENT_N" -gt 0 ]; then + echo "### ❌ Agent artefacts tracked β€” $AGENT_N file(s)" + echo "" + echo "An AI agent's tooling configuration does not belong in a repository." + echo "This is the same rejection policy the attribution rules apply to commit" + echo "messages, applied to what a change leaves on disk." + echo "" + while IFS=$'\t' read -r cnt root; do + [ -z "$root" ] && continue + if [ "$cnt" -gt 1 ]; then printf -- '- `%s` β€” %s file(s)\n' "$(esc "$root")" "$cnt" + else printf -- '- `%s`\n' "$(esc "$root")"; fi + done < <(printf '%s\n' "$AGENT_ROOTS" | head -n "$MAX_REPORT") + [ "$AGENT_ROOT_N" -gt "$MAX_REPORT" ] && echo "" && echo "_… and $((AGENT_ROOT_N - MAX_REPORT)) more._" + echo "" + echo "> Untrack them and commit β€” the file stays on your machine:" + echo "> \`git rm -r --cached \` then add it to \`.gitignore\`." + echo "> **No history rewrite is needed**: this check reads the tree at the head," + echo "> not the ancestry, so one commit clears it." + echo ">" + echo "> Editor configuration (\`.vscode/\`, \`.idea/\`), \`.devcontainer/\` itself and" + echo "> markdown instructions (\`CLAUDE.md\`, \`AGENTS.md\`) are never matched β€” but an" + echo "> agent directory nested inside one of them still is." + echo "> A repository that exists to distribute this configuration exempts the" + echo "> exact paths it ships with the \`agent_files_allow\` input." + echo "" + fi } > "$REPORT_BODY" cat "$REPORT_BODY" >> "$SUMMARY" [ -n "$REPORT" ] && cp "$REPORT_BODY" "$REPORT" @@ -361,8 +491,21 @@ if [ "$SEC_N" -gt 0 ]; then RC=1 echo "::error::$SEC_N added line(s) look like credentials" fi +if [ "$AGENT_N" -gt 0 ]; then + RC=1 + # file= makes GitHub anchor the annotation on the offending path itself, + # which is also the instruction: this is the file to remove. + while IFS= read -r a; do + echo "::error file=${a}::agent artefact tracked here β€” remove it (git rm -r --cached)" + done < <(head -n "$MAX_REPORT" "$AGENT_FILE") + echo "::error::$AGENT_N agent artefact file(s) tracked at ${TREE_REV:0:12} β€” no history rewrite needed, one commit removes them" +fi if [ "$RC" -eq 0 ]; then - echo "βœ… post-commit: $ATTR_SCANNED commit(s) attribution-free${IDENT_RE:+ and correctly attributed}, $FMT_SCANNED subject(s) conventional, no credentials added" + # PC_AGENT_FILES is "true" or "false" β€” both non-empty, so the claim is + # built from whether the check ran, not from the flag's truthiness. + AGENT_CLAIM="" + [ "$AGENT_FILES" = "true" ] && AGENT_CLAIM=", no agent artefacts tracked" + echo "βœ… post-commit: $ATTR_SCANNED commit(s) attribution-free${IDENT_RE:+ and correctly attributed}, $FMT_SCANNED subject(s) conventional, no credentials added${AGENT_CLAIM}" fi exit "$RC" diff --git a/scripts/rewrite-history.sh b/scripts/rewrite-history.sh index fd34d7c..5a494ee 100755 --- a/scripts/rewrite-history.sh +++ b/scripts/rewrite-history.sh @@ -76,7 +76,14 @@ git clone --mirror -q "https://github.com/$REPO.git" "$MIRROR" || exit 1 BEFORE_COUNT="$(git -C "$MIRROR" rev-list --branches --tags --count)" # The gate exits 1 on a tainted history β€” expected here β€” and `pipefail` # would turn that into a pipeline failure, so capture first, parse after. -BEFORE_OUT="$(cd "$MIRROR" && GITHUB_STEP_SUMMARY=/dev/null PC_MAX_REPORT=0 PC_AUTHORS="$AUTHORS" bash "$SCRIPT_DIR/post-commit.sh" --branches 2>&1 || true)" +# +# PC_AGENT_FILES=false on both gate calls, before and after: this script +# rewrites messages and identities and never touches the tree, so a tracked +# `.claude/` would show up in the "after" verdict as a failure the rewrite was +# never meant to fix β€” and the --execute path refuses to push a history the +# gate rejects, so a correct rewrite would be blocked by an unrelated rule. +# Removing those files is one ordinary commit; it does not belong here. +BEFORE_OUT="$(cd "$MIRROR" && GITHUB_STEP_SUMMARY=/dev/null PC_MAX_REPORT=0 PC_AGENT_FILES=false PC_AUTHORS="$AUTHORS" bash "$SCRIPT_DIR/post-commit.sh" --branches 2>&1 || true)" BEFORE_TAINT="$(printf '%s' "$BEFORE_OUT" | grep -oE '^::error::[0-9]+ tainted' | grep -oE '[0-9]+' || true)" : "${BEFORE_TAINT:=0}" # Identity violations are counted separately: a repository can be free of AI @@ -247,7 +254,7 @@ AFTER_COUNT="$(git -C "$MIRROR" rev-list --branches --tags --count)" CHANGED="$(awk 'NR>1 && $1!=$2' "$MIRROR/filter-repo/commit-map" 2>/dev/null | wc -l | tr -d ' ')" # --- verify with the gate itself -------------------------------------------- -GATE_OUT="$(cd "$MIRROR" && GITHUB_STEP_SUMMARY=/dev/null PC_MAX_REPORT=20 PC_AUTHORS="$AUTHORS" bash "$SCRIPT_DIR/post-commit.sh" --branches 2>&1)" +GATE_OUT="$(cd "$MIRROR" && GITHUB_STEP_SUMMARY=/dev/null PC_MAX_REPORT=20 PC_AGENT_FILES=false PC_AUTHORS="$AUTHORS" bash "$SCRIPT_DIR/post-commit.sh" --branches 2>&1)" GATE_RC=$? RESIDUAL_KW="$(git -C "$MIRROR" log --branches --format='%h %s%n%b' | grep -iE 'claude|anthropic|copilot|chatgpt|openai|gemini|\bllm\b' | head -20)" OPEN_PRS="$(gh pr list --repo "$REPO" --state open --json number,title --jq '.[] | "#\(.number) \(.title)"' 2>/dev/null)" diff --git a/tests/run.sh b/tests/run.sh index 4b6189c..9497fe6 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -343,6 +343,97 @@ d=$(mkrepo); mkdir -p "$d/tests"; echo 'AKIAIOSFODNN7EXAMPLE' > "$d/tests/fixtur git -C "$d" add -A; git -C "$d" commit -qm "chore: config" check "a real path still fails even with a fixture alongside" 1 "$d" +echo "== agent artefacts ==" +# The tree, not the range. Every case below commits with a conventional +# subject and an allowed identity so that exit 1 can only mean this check. +d=$(mkrepo); mkdir -p "$d/.claude"; echo '{}' > "$d/.claude/settings.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: add agent config" +check "a tracked .claude/ is refused" 1 "$d" + +d=$(mkrepo); mkdir -p "$d/.vscode"; echo '{}' > "$d/.vscode/settings.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: editor settings" +check ".vscode/ is editor config, not an agent" 0 "$d" + +d=$(mkrepo); mkdir -p "$d/.idea" "$d/.devcontainer" +echo '' > "$d/.idea/workspace.xml"; echo '{}' > "$d/.devcontainer/devcontainer.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: editor and environment" +check ".idea/ and .devcontainer/ are kept" 0 "$d" + +# The scope decision of #21: markdown instructions read as project +# documentation and 21 fleet repos carry one, so they are deliberately out. +d=$(mkrepo); echo '# instructions' > "$d/CLAUDE.md"; echo '# agents' > "$d/AGENTS.md" +git -C "$d" add -A; git -C "$d" commit -qm "docs: project instructions" +check "CLAUDE.md and AGENTS.md are tolerated as documentation" 0 "$d" + +d=$(mkrepo); echo 'be nice' > "$d/.cursorrules" +git -C "$d" add -A; git -C "$d" commit -qm "chore: cursor rules" +check "a dotfile agent config is refused too" 1 "$d" + +d=$(mkrepo); echo 'chat' > "$d/.aider.chat.history.md" +git -C "$d" add -A; git -C "$d" commit -qm "chore: leftovers" +check "aider drops its history with a known prefix" 1 "$d" + +d=$(mkrepo); mkdir -p "$d/packages/app/.claude"; echo '{}' > "$d/packages/app/.claude/x.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: nested agent config" +check "a nested .claude/ is found, not just the root one" 1 "$d" + +# The whole point of reading the tree: the artefact arrives in an ancestor and +# the range that adds it is long merged. A range-scoped check would call this +# clean, which is exactly how the directory that prompted the rule survived. +d=$(mkrepo); mkdir -p "$d/.claude"; echo '{}' > "$d/.claude/settings.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: agent config" +commit_msg "$d" "feat: unrelated later work" +check "an artefact merged earlier still fails a later clean change" 1 "$d" PC_HISTORY=range + +# …and removing it is enough. No history rewrite, unlike an attribution. +d=$(mkrepo); mkdir -p "$d/.claude"; echo '{}' > "$d/.claude/settings.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: agent config" +git -C "$d" rm -qr .claude; git -C "$d" commit -qm "chore: drop the agent config" +check "removing it in one commit clears the check" 0 "$d" + +d=$(mkrepo); mkdir -p "$d/.claude"; echo '{}' > "$d/.claude/settings.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: agent config" +check "the check is switchable" 0 "$d" PC_AGENT_FILES=false + +# A repository whose purpose is to distribute this config exempts its paths. +d=$(mkrepo); mkdir -p "$d/.claude"; echo '{}' > "$d/.claude/settings.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: agent config" +check "a bare allow entry exempts the whole subtree" 0 "$d" PC_AGENT_ALLOW=.claude + +d=$(mkrepo); mkdir -p "$d/.claude" "$d/.cursor" +echo '{}' > "$d/.claude/settings.json"; echo '{}' > "$d/.cursor/rules.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: two agents" +check "an allow entry exempts only what it names" 1 "$d" PC_AGENT_ALLOW=".claude/*" + +# The verdict names the artefact root, and getting that root wrong is worse +# than not collapsing at all: the fleet's devcontainer ships its agent config +# at .devcontainer/images/.claude/, and naming `.devcontainer/` as the thing to +# delete would point at 400 files the gate has no quarrel with. +d=$(mkrepo); mkdir -p "$d/.devcontainer/images/.claude" +echo '{}' > "$d/.devcontainer/images/.claude/settings.json" +echo '{}' > "$d/.devcontainer/devcontainer.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: devcontainer with agent config" +r="$(mktemp)" +( cd "$d" && env GITHUB_STEP_SUMMARY=/dev/null PC_REPORT="$r" bash "$GATE" HEAD HEAD~1..HEAD ) >/dev/null 2>&1 +if grep -qF '`.devcontainer/images/.claude/`' "$r" && ! grep -qF '`.devcontainer/` β€”' "$r"; then + PASS=$((PASS+1)); printf ' ok %s\n' "the reported root is the artefact, not the .devcontainer around it" +else + FAIL=$((FAIL+1)); printf ' FAIL %s\n%s\n' "the reported root is the artefact, not the .devcontainer around it" "$(sed 's/^/ /' "$r")" +fi +rm -rf "$d" "$r" + +# The whole-repository scope (push, manual runs) is not a tree-ish; the check +# has to fall back to the checked-out head rather than fail the job. +d=$(mkrepo); mkdir -p "$d/.claude"; echo '{}' > "$d/.claude/settings.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: agent config" +out="$(cd "$d" && env GITHUB_STEP_SUMMARY=/dev/null bash "$GATE" --branches 2>&1)"; rc=$? +if [ "$rc" -eq 1 ] && printf '%s' "$out" | grep -q 'agent artefact'; then + PASS=$((PASS+1)); printf ' ok %s\n' "--branches scans the checked-out tree" +else + FAIL=$((FAIL+1)); printf ' FAIL %s (want exit 1 and an artefact error, got %s)\n' "--branches scans the checked-out tree" "$rc" +fi +rm -rf "$d" + echo "== usage errors (expect 2) ==" d=$(mkrepo) out="$(cd "$d" && bash "$GATE" 2>&1)"; rc=$? From 8a90697219b9bfb70b7dc09d15f256541fff25f7 Mon Sep 17 00:00:00 2001 From: Kodflow <133899878+kodflow@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:36:47 +0200 Subject: [PATCH 2/5] fix(gate): read tracked paths NUL-delimited and stop globbing the allow list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ways the artefact check could be told a repository was clean when it was not. A path holding a non-ASCII or control character is C-quoted by `git ls-tree --name-only`: `.claude/naΓ―ve.md` arrives as `".claude/na\303\257ve.md"`. The quote lands exactly where `(^|/)` and `$` need a path boundary, so every pattern in agent-paths.txt stops matching β€” measured, not theorised: a repository whose `.claude/` files all carried an accent was reported clean, and naming files that way is a one-line evasion of the whole rule. Now `-z`, NUL to the end: through the grep, the allow filter, the hit file, the roots and the annotations. A bash string cannot hold a NUL, so the tree no longer travels through a variable and the revision is verified with rev-parse instead of through git's exit status. `for allow in ${AGENT_ALLOW//,/ }` glob-expanded each entry against the working tree before using it as a pattern. Filename globbing skips leading dots, so `.claude/*` exempted `.claude/settings.json` and left `.claude/.mcp.json` next to it unexempted β€” an exemption that silently depended on what happened to be on disk. `read -a` splits on the documented separators and expands nothing. The root of a hit is now computed with parameter expansion in the same loop rather than in awk. `read -a` stops at a newline, which is one of the characters this fix exists to handle, and an awk regex passed with -v is subject to escape processing that could turn `\.` into a bare `.` β€” which would report `aclaude/` as the root of `aclaude/.claude/f`, the wrong-directory bug by another road. Annotations encode `%`, CR, LF, `:` and `,` in the `file=` property, and the report renders each root through `%q`: a no-op for an ordinary path and the only honest rendering of one carrying a newline. Six tests: the three evasions above, the hidden-file exemption, the lookalike directory, and a guard that both of rewrite-history.sh's gate calls still disable this check. --- scripts/post-commit.sh | 138 +++++++++++++++++++++++++---------------- tests/run.sh | 57 +++++++++++++++++ 2 files changed, 142 insertions(+), 53 deletions(-) diff --git a/scripts/post-commit.sh b/scripts/post-commit.sh index 2b1766e..947ee16 100755 --- a/scripts/post-commit.sh +++ b/scripts/post-commit.sh @@ -55,7 +55,14 @@ HISTORY="${PC_HISTORY:-full}" FORMAT="${PC_FORMAT:-true}" AUTHORS="${PC_AUTHORS:-}" AGENT_FILES="${PC_AGENT_FILES:-true}" -AGENT_ALLOW="${PC_AGENT_ALLOW:-}" +# read -a, not an unquoted expansion. `for x in ${VAR//,/ }` word-splits AND +# glob-expands, so an entry like `.claude/*` would be expanded against the +# working tree before it is ever used as a pattern β€” and bash's filename +# globbing skips leading dots, so `.claude/.mcp.json` would come out NOT +# exempted while `.claude/settings.json` would. read does the splitting +# without the expansion, leaving the entry to be matched as a pattern by +# [[ == ]], where `*` does cross both `/` and a leading dot. +IFS=', ' read -r -a AGENT_ALLOW <<< "${PC_AGENT_ALLOW:-}" MAX_REPORT="${PC_MAX_REPORT:-50}" SUMMARY="${GITHUB_STEP_SUMMARY:-/dev/null}" # Second sink for the same markdown. The job summary is only read by @@ -131,7 +138,7 @@ ATTR_FILE="$(mktemp)"; FMT_FILE="$(mktemp)"; SEC_FILE="$(mktemp)"; IDENT_FILE="$ TRAIL_FILE="$(mktemp)"; AGENT_FILE="$(mktemp)"; REPORT_BODY="$(mktemp)" trap 'rm -f "$ATTR_FILE" "$FMT_FILE" "$SEC_FILE" "$IDENT_FILE" "$TRAIL_FILE" "$AGENT_FILE" "$REPORT_BODY"' EXIT ATTR_N=0; ATTR_SCANNED=0; FMT_N=0; FMT_SCANNED=0; SEC_N=0; IDENT_N=0; TRAIL_N=0; AGENT_N=0 -AGENT_ROOTS=""; AGENT_ROOT_N=0 +AGENT_ROOT_N=0 # Identity trailers, whatever the case. An address here is as permanent as the # author field, so the same allow-list applies to both. Accounts whose display @@ -281,23 +288,35 @@ fi # rewrite-history.sh) and is not a tree-ish. There, the checked-out head is the # tree to judge β€” on push that is exactly the commit that was pushed. TREE_REV="$HEAD_REV"; [ "$TREE_REV" = "--branches" ] && TREE_REV=HEAD +declare -A AGENT_ROOT_COUNT=() +AGENT_ROOT_ORDER=() if [ "$AGENT_FILES" = "true" ]; then # One combined alternation rather than a grep per pattern: a large # repository has tens of thousands of tracked paths, and forty passes over - # them is forty times the work for the same answer. Nothing is lost by not - # recording which alternative fired β€” unlike a message match, the path is - # its own evidence. + # them is forty times the work for the same answer. AGENT_RE="" for pattern in "${AGENT_PATTERNS[@]}"; do AGENT_RE="${AGENT_RE}|(${pattern})"; done AGENT_RE="${AGENT_RE#|}" - TREE="$(git ls-tree -r --name-only "$TREE_REV" 2>/dev/null)" || { - echo "::error::git ls-tree failed for '$TREE_REV' β€” no such tree?" >&2 + + # -z, and NUL all the way through. Without it git C-quotes any path holding + # a non-ASCII or control character β€” `.claude/naΓ―ve.md` is emitted as + # `".claude/na\303\257ve.md"` β€” and the quote it adds sits exactly where + # `(^|/)` and `$` need a path boundary, so every pattern in agent-paths.txt + # stops matching. Measured before the fix: a repository whose `.claude/` + # files all carried an accent was reported clean. Naming files that way is + # a one-line evasion of the whole rule. + # + # The paths cannot pass through a shell variable on the way, because a bash + # string cannot hold a NUL. So the pipeline runs straight into the loop and + # the revision is verified separately rather than through git's exit status. + git rev-parse -q --verify "${TREE_REV}^{tree}" >/dev/null 2>&1 || { + echo "::error::no tree at '$TREE_REV' β€” shallow checkout? (needs fetch-depth: 0)" >&2 exit 2 } - while IFS= read -r p; do + while IFS= read -r -d '' p; do [ -z "$p" ] && continue ALLOWED="" - for allow in ${AGENT_ALLOW//,/ }; do + for allow in ${AGENT_ALLOW[@]+"${AGENT_ALLOW[@]}"}; do # A bare entry exempts the whole subtree, so `.claude` and # `.claude/*` both mean what whoever wrote the input expects. `*` # crosses `/` inside [[ == ]], so no globstar is involved. @@ -306,46 +325,49 @@ if [ "$AGENT_FILES" = "true" ]; then done [ -n "$ALLOWED" ] && continue AGENT_N=$((AGENT_N + 1)) - printf '%s\n' "$p" >> "$AGENT_FILE" - done < <(printf '%s\n' "$TREE" | grep -iE -- "$AGENT_RE") - # Every hit is recorded, uncapped, because the report does not show them - # one by one. A `.claude/` holding two hundred files is ONE thing to - # delete, not two hundred lines of verdict, so each path collapses to its - # artefact root and the roots are counted. MAX_REPORT then caps roots - # rather than files, which is what a reader was going to act on anyway. - # The per-file GitHub annotations below stay per-file: those anchor on the - # path and are read by a machine. - # - # The root is the SHORTEST PREFIX THAT STILL MATCHES, not the first - # dot-directory along the path, and the difference is not cosmetic. The - # fleet's devcontainer ships its agent configuration at - # `.devcontainer/images/.claude/…`; collapsing on the first dot-directory - # would name `.devcontainer/` as the thing to delete β€” a directory this - # gate promises never to touch, holding 400 files it has no quarrel with. - # A verdict that names the wrong directory is worse than none. - # - # tolower() on the candidate is how the case-insensitivity of the grep - # above is carried into awk, which has no equivalent flag. A pattern - # written with an uppercase letter would simply not collapse β€” the path is - # then reported whole, which is accurate, just longer. - if [ "$AGENT_N" -gt 0 ]; then - AGENT_ROOTS="$(awk -F/ -v re="$AGENT_RE" '{ - r = ""; root = $0 - for (i = 1; i <= NF; i++) { - r = (r == "" ? $i : r "/" $i) - cand = (i < NF ? r "/" : r) - if (tolower(cand) ~ re) { root = cand; break } - } - if (!(root in n)) order[++k] = root - n[root]++ - } - END { for (j = 1; j <= k; j++) print n[order[j]] "\t" order[j] }' "$AGENT_FILE")" - AGENT_ROOT_N="$(printf '%s\n' "$AGENT_ROOTS" | wc -l)" - fi + printf '%s\0' "$p" >> "$AGENT_FILE" + + # The artefact root: the SHORTEST PREFIX THAT STILL MATCHES. Not the + # first dot-directory along the path, and the difference is not + # cosmetic β€” the fleet's devcontainer ships its agent configuration at + # `.devcontainer/images/.claude/…`, so collapsing on the first would + # name `.devcontainer/` as the thing to delete: a directory this gate + # promises never to touch, holding 400 files it has no quarrel with. A + # verdict that names the wrong directory is worse than none. + # + # Split with parameter expansion rather than `read -a`: read stops at a + # newline, and a newline is one of the characters a path may contain + # and this loop exists to handle. Lowercasing the candidate is how the + # grep's -i is carried into [[ =~ ]]; a pattern written with an + # uppercase letter simply would not collapse, leaving the whole path + # reported, which is accurate and merely longer. + cand=""; rest="$p"; root="$p" + while [ -n "$rest" ]; do + seg="${rest%%/*}" + cand="${cand}${seg}" + if [ "$seg" = "$rest" ]; then rest=""; else cand="${cand}/"; rest="${rest#*/}"; fi + if [[ "${cand,,}" =~ $AGENT_RE ]]; then root="$cand"; break; fi + done + if [ -z "${AGENT_ROOT_COUNT[$root]+set}" ]; then + AGENT_ROOT_ORDER+=("$root"); AGENT_ROOT_COUNT["$root"]=0 + fi + AGENT_ROOT_COUNT["$root"]=$(( AGENT_ROOT_COUNT["$root"] + 1 )) + done < <(git ls-tree -r -z --name-only "$TREE_REV" | grep -z -iE -- "$AGENT_RE") + AGENT_ROOT_N="${#AGENT_ROOT_ORDER[@]}" fi # --- Report ----------------------------------------------------------------- esc() { printf '%s' "$1" | sed 's/|/\\|/g; s/`/ΚΌ/g'; } +# GitHub workflow-command property values are comma-separated and newline- +# terminated, so a path carrying either would truncate or split the annotation +# it is supposed to point at. Pure parameter expansion: a path is arbitrary +# bytes and piping it through sed would lose the newline this exists to encode. +esc_prop() { + local v="$1" + v="${v//%/%25}"; v="${v//$'\r'/%0D}"; v="${v//$'\n'/%0A}" + v="${v//:/%3A}"; v="${v//,/%2C}" + printf '%s' "$v" +} { echo "## post-commit" echo "" @@ -436,11 +458,19 @@ esc() { printf '%s' "$1" | sed 's/|/\\|/g; s/`/ΚΌ/g'; } echo "This is the same rejection policy the attribution rules apply to commit" echo "messages, applied to what a change leaves on disk." echo "" - while IFS=$'\t' read -r cnt root; do - [ -z "$root" ] && continue - if [ "$cnt" -gt 1 ]; then printf -- '- `%s` β€” %s file(s)\n' "$(esc "$root")" "$cnt" - else printf -- '- `%s`\n' "$(esc "$root")"; fi - done < <(printf '%s\n' "$AGENT_ROOTS" | head -n "$MAX_REPORT") + shown=0 + for root in ${AGENT_ROOT_ORDER[@]+"${AGENT_ROOT_ORDER[@]}"}; do + shown=$((shown + 1)); [ "$shown" -gt "$MAX_REPORT" ] && break + # %q on the way out: a no-op for an ordinary path, and the only + # honest rendering of one holding a newline or a control character + # β€” which would otherwise break the list it is printed into. + disp="$(printf '%q' "$root")" + if [ "${AGENT_ROOT_COUNT[$root]}" -gt 1 ]; then + printf -- '- `%s` β€” %s file(s)\n' "$(esc "$disp")" "${AGENT_ROOT_COUNT[$root]}" + else + printf -- '- `%s`\n' "$(esc "$disp")" + fi + done [ "$AGENT_ROOT_N" -gt "$MAX_REPORT" ] && echo "" && echo "_… and $((AGENT_ROOT_N - MAX_REPORT)) more._" echo "" echo "> Untrack them and commit β€” the file stays on your machine:" @@ -495,9 +525,11 @@ if [ "$AGENT_N" -gt 0 ]; then RC=1 # file= makes GitHub anchor the annotation on the offending path itself, # which is also the instruction: this is the file to remove. - while IFS= read -r a; do - echo "::error file=${a}::agent artefact tracked here β€” remove it (git rm -r --cached)" - done < <(head -n "$MAX_REPORT" "$AGENT_FILE") + shown=0 + while IFS= read -r -d '' a; do + shown=$((shown + 1)); [ "$shown" -gt "$MAX_REPORT" ] && break + echo "::error file=$(esc_prop "$a")::agent artefact tracked here β€” remove it (git rm -r --cached)" + done < "$AGENT_FILE" echo "::error::$AGENT_N agent artefact file(s) tracked at ${TREE_REV:0:12} β€” no history rewrite needed, one commit removes them" fi diff --git a/tests/run.sh b/tests/run.sh index 9497fe6..187c2ef 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -405,6 +405,47 @@ echo '{}' > "$d/.claude/settings.json"; echo '{}' > "$d/.cursor/rules.json" git -C "$d" add -A; git -C "$d" commit -qm "chore: two agents" check "an allow entry exempts only what it names" 1 "$d" PC_AGENT_ALLOW=".claude/*" +# git C-quotes a path holding a non-ASCII or control character unless asked for +# NUL-delimited output, and the quote it adds lands exactly where `(^|/)` and +# `$` need a path boundary. Before the fix these three were reported clean β€” +# naming every file in `.claude/` with an accent evaded the whole rule. +d=$(mkrepo); mkdir -p "$d/.claude"; echo '{}' > "$d/.claude/naΓ―ve.md" +git -C "$d" add -A; git -C "$d" commit -qm "chore: accented name inside the artefact" +check "a non-ASCII file name does not evade the scan" 1 "$d" + +d=$(mkrepo); mkdir -p "$d/dΓ©jΓ "; echo '{}' > "$d/dΓ©jΓ /.mcp.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: anchored artefact below a non-ASCII parent" +check "an end-anchored pattern still matches below a non-ASCII parent" 1 "$d" + +d=$(mkrepo); mkdir -p "$d/.claude"; echo '{}' > "$d/.claude/$(printf 'two\nlines').md" +git -C "$d" add -A; git -C "$d" commit -qm "chore: newline in the file name" +check "a newline in the file name does not split a record" 1 "$d" + +# `.claude/*` must exempt what is under `.claude/`, hidden files included. An +# unquoted expansion would glob it against the working tree first, and bash's +# filename globbing skips leading dots β€” so this passed for settings.json and +# quietly failed for .mcp.json next to it. +d=$(mkrepo); mkdir -p "$d/.claude" +echo '{}' > "$d/.claude/settings.json"; echo '{}' > "$d/.claude/.mcp.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: agent config with a hidden file" +check "a glob allow entry covers hidden files too" 0 "$d" PC_AGENT_ALLOW=".claude/*" + +# The root is found with the pattern list, so the pattern list has to reach awk +# intact. On an awk that strips the escape from a -v value, `\.` becomes a bare +# `.`, `aclaude/` matches `(^|/).claude/`, and the verdict names an ordinary +# directory as the thing to delete. +d=$(mkrepo); mkdir -p "$d/aclaude/.claude" +echo '{}' > "$d/aclaude/.claude/settings.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: agent config below a lookalike" +r="$(mktemp)" +( cd "$d" && env GITHUB_STEP_SUMMARY=/dev/null PC_REPORT="$r" bash "$GATE" HEAD HEAD~1..HEAD ) >/dev/null 2>&1 +if grep -qF '`aclaude/.claude/`' "$r" && ! grep -qF '`aclaude/`' "$r"; then + PASS=$((PASS+1)); printf ' ok %s\n' "a directory whose name merely ends in the pattern is not the root" +else + FAIL=$((FAIL+1)); printf ' FAIL %s\n%s\n' "a directory whose name merely ends in the pattern is not the root" "$(sed 's/^/ /' "$r")" +fi +rm -rf "$d" "$r" + # The verdict names the artefact root, and getting that root wrong is worse # than not collapsing at all: the fleet's devcontainer ships its agent config # at .devcontainer/images/.claude/, and naming `.devcontainer/` as the thing to @@ -434,6 +475,22 @@ else fi rm -rf "$d" +# rewrite-history.sh keeps the artefact check off on both of its gate calls: it +# rewrites messages and identities and never touches the tree, so a tracked +# `.claude/` would make --execute refuse to push a correct rewrite. Running the +# script itself needs a network mirror and git-filter-repo; what is guarded +# here is the pairing that can silently come undone. +# Counted on the invoking lines themselves, not on the file: the comment that +# explains the pairing names the variable too, and matching that would make the +# guard pass on the explanation alone. +calls="$(grep -c 'post-commit.sh" --branches' "$ROOT/scripts/rewrite-history.sh")" +guarded="$(grep 'post-commit.sh" --branches' "$ROOT/scripts/rewrite-history.sh" | grep -c 'PC_AGENT_FILES=false')" +if [ "$calls" -gt 0 ] && [ "$guarded" -eq "$calls" ]; then + PASS=$((PASS+1)); printf ' ok %s\n' "every rewrite-history gate call disables the artefact check" +else + FAIL=$((FAIL+1)); printf ' FAIL %s (%s call(s), %s guarded)\n' "every rewrite-history gate call disables the artefact check" "$calls" "$guarded" +fi + echo "== usage errors (expect 2) ==" d=$(mkrepo) out="$(cd "$d" && bash "$GATE" 2>&1)"; rc=$? From 354ad061abf762124ffcb916a3353b7ff4c04744 Mon Sep 17 00:00:00 2001 From: Kodflow <133899878+kodflow@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:39:37 +0200 Subject: [PATCH 3/5] test(gate): pin that a tracked path is never evaluated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A path comes from the repository being judged, and the scan uses it as an associative array subscript, a [[ ]] operand and an annotation property. `declare -A` is what keeps the subscript a string instead of an arithmetic expression; without it `c];touch PWNED;x[/.mcp.json` would run. Nothing to fix today β€” this fails loudly if the -A is ever dropped or the array turned into an indexed one. --- tests/run.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/run.sh b/tests/run.sh index 187c2ef..c86ca82 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -421,6 +421,22 @@ d=$(mkrepo); mkdir -p "$d/.claude"; echo '{}' > "$d/.claude/$(printf 'two\nlines git -C "$d" add -A; git -C "$d" commit -qm "chore: newline in the file name" check "a newline in the file name does not split a record" 1 "$d" +# A tracked path is attacker-controlled input and ends up as an associative +# array subscript, a [[ ]] operand and an annotation. `declare -A` is what makes +# the subscript a string rather than an arithmetic expression; this is here so +# that dropping the -A, or reaching for an indexed array, fails loudly. +d=$(mkrepo); mkdir -p "$d/a\$(touch PWNED).d" "$d/c];touch PWNED3;x[.d" +printf 'x' > "$d/a\$(touch PWNED).d/.mcp.json" +printf 'x' > "$d/c];touch PWNED3;x[.d/.mcp.json" +git -C "$d" add -A; git -C "$d" commit -qm "chore: hostile parent names" +( cd "$d" && env GITHUB_STEP_SUMMARY=/dev/null bash "$GATE" HEAD HEAD~1..HEAD ) >/dev/null 2>&1 +if [ ! -e "$d/PWNED" ] && [ ! -e "$d/PWNED3" ]; then + PASS=$((PASS+1)); printf ' ok %s\n' "a hostile file name is data, never evaluated" +else + FAIL=$((FAIL+1)); printf ' FAIL %s β€” the scan executed part of a path\n' "a hostile file name is data, never evaluated" +fi +rm -rf "$d" + # `.claude/*` must exempt what is under `.claude/`, hidden files included. An # unquoted expansion would glob it against the working tree first, and bash's # filename globbing skips leading dots β€” so this passed for settings.json and From 3b79cc90ebc0fb20fbd4b7bc28f0db2556afd9c9 Mon Sep 17 00:00:00 2001 From: Kodflow <133899878+kodflow@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:41:53 +0200 Subject: [PATCH 4/5] test(gate): require both rewrite-history gate calls, not just one The guard asserted that every gate call in rewrite-history.sh disables the artefact check, which a single remaining call satisfies. Both are load-bearing: the before scan feeds the tainted and identity counters the report prints, and the after scan is what --execute consults before force pushing. Losing either is a silent regression the old assertion waved through. --- tests/run.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/run.sh b/tests/run.sh index c86ca82..181e6a0 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -501,10 +501,13 @@ rm -rf "$d" # guard pass on the explanation alone. calls="$(grep -c 'post-commit.sh" --branches' "$ROOT/scripts/rewrite-history.sh")" guarded="$(grep 'post-commit.sh" --branches' "$ROOT/scripts/rewrite-history.sh" | grep -c 'PC_AGENT_FILES=false')" -if [ "$calls" -gt 0 ] && [ "$guarded" -eq "$calls" ]; then +# Two, not "at least one": the before scan feeds the tainted/identity counters +# the report prints, and the after scan is what --execute consults before force +# pushing. Losing either is a silent regression that `-gt 0` would wave through. +if [ "$calls" -eq 2 ] && [ "$guarded" -eq 2 ]; then PASS=$((PASS+1)); printf ' ok %s\n' "every rewrite-history gate call disables the artefact check" else - FAIL=$((FAIL+1)); printf ' FAIL %s (%s call(s), %s guarded)\n' "every rewrite-history gate call disables the artefact check" "$calls" "$guarded" + FAIL=$((FAIL+1)); printf ' FAIL %s (want 2 call(s) all guarded, got %s call(s), %s guarded)\n' "every rewrite-history gate call disables the artefact check" "$calls" "$guarded" fi echo "== usage errors (expect 2) ==" From 47467dcc4b8afafa75913ca054676dd23731a47e Mon Sep 17 00:00:00 2001 From: Kodflow <133899878+kodflow@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:45:56 +0200 Subject: [PATCH 5/5] test(gate): assert the hostile paths are actually refused, not just inert The injection guard checked only that no part of a path was executed. That is equally true of a scan which matched nothing, so a regression in the matching would have left the test green while proving only that a clean run is harmless. The exit status is now part of the assertion. --- tests/run.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/run.sh b/tests/run.sh index 181e6a0..9af3da7 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -430,8 +430,14 @@ printf 'x' > "$d/a\$(touch PWNED).d/.mcp.json" printf 'x' > "$d/c];touch PWNED3;x[.d/.mcp.json" git -C "$d" add -A; git -C "$d" commit -qm "chore: hostile parent names" ( cd "$d" && env GITHUB_STEP_SUMMARY=/dev/null bash "$GATE" HEAD HEAD~1..HEAD ) >/dev/null 2>&1 -if [ ! -e "$d/PWNED" ] && [ ! -e "$d/PWNED3" ]; then +rc=$? +# The exit status belongs in the assertion: "nothing was executed" is also true +# of a scan that matched nothing, so without it a regression in the matching +# would leave this test green while proving only that a clean run is harmless. +if [ "$rc" -eq 1 ] && [ ! -e "$d/PWNED" ] && [ ! -e "$d/PWNED3" ]; then PASS=$((PASS+1)); printf ' ok %s\n' "a hostile file name is data, never evaluated" +elif [ "$rc" -ne 1 ]; then + FAIL=$((FAIL+1)); printf ' FAIL %s β€” the gate did not refuse them (exit %s)\n' "a hostile file name is data, never evaluated" "$rc" else FAIL=$((FAIL+1)); printf ' FAIL %s β€” the scan executed part of a path\n' "a hostile file name is data, never evaluated" fi