From 12bf55c14cd7a8f87b2a94d1912be75cfd4183fc Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Sep 2026 11:25:19 -0700 Subject: [PATCH 1/3] Explain clean merges and admin pushes in workflow-audit Most nightly issues were the admin's own commits, re-reported on every rebase and merge from main. Two new classifiers: - is_clean_merge: a two-parent merge whose window paths equal `git merge-tree --write-tree` of its parents. - is_admin_first_push: the earliest ref update in the repository activity log containing the commit is a push by an admin, and no author/committer field names a bot. The pusher is GitHub's record, which TEND_BOT_TOKEN cannot forge; PR merges never count as an introduction. Unexplained entries now name who first pushed them. A week's replay explains 32 of 45 window commits; the rest are dormouse-bot pushes. Also reword a comment line that shellcheck parsed as a directive, which had silently disabled shellcheck for the whole audit script. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/workflow-audit.yaml | 124 ++++++++++++++++++++--- docs/specs/security-audit.md | 2 +- docs/specs/security-ci.md | 9 +- docs/specs/security-ci.rationale.md | 2 + scripts/spec-word-budgets.json | 2 +- scripts/workflow-audit.test.mjs | 140 +++++++++++++++++++++++++- 6 files changed, 256 insertions(+), 23 deletions(-) diff --git a/.github/workflows/workflow-audit.yaml b/.github/workflows/workflow-audit.yaml index 6664d593a..e4ae723b4 100644 --- a/.github/workflows/workflow-audit.yaml +++ b/.github/workflows/workflow-audit.yaml @@ -13,8 +13,8 @@ name: workflow-audit # successful run's API timestamp, so a failed run leaves the lower bound # unchanged rather than skipping commits. # -# Reports the *unexplained*. Two routine sources are classified and -# skipped on independently checked provenance and content (see the +# Reports the *unexplained*. Four routine sources are classified and +# skipped on independently checked provenance or content (see the # classifier comments for the trust boundary): # # - Renovate pin bumps — a valid GitHub-signed commit authored by @@ -30,8 +30,16 @@ name: workflow-audit # .config/tend.yaml — which the commit must leave untouched, or # "reproducible" is true by construction. The config being in the # window is what closes the same trick split across two commits. +# - clean merges — a merge commit whose window paths are exactly what +# `git merge-tree` produces from its parents, so it introduced nothing +# the parents (each audited on its own) did not already carry. +# - admin pushes — the earliest server-recorded push that made the +# commit reachable was by a repository admin, and neither its author +# nor committer claims to be a bot. The pusher is GitHub's record of +# the authenticated credential, which `TEND_BOT_TOKEN` cannot forge; +# the commit's own author fields are only a further refusal. # -# Both classifiers fail open: any error, ambiguity, or unparseable input +# Every classifier fails open: any error, ambiguity, or unparseable input # reports the commit. A silent run is the healthy steady state and keeps # the 48-hour liveness check in docs/specs/security-ci.md green — that check keys on a # successful *run*, not on an issue existing. @@ -89,11 +97,11 @@ jobs: # what each one is told to check, and the rule that keeps the # orchestrator from ending its turn. A bot that edits those changes # what gets audited without touching a single workflow file, which is - # exactly the persistence this job exists to catch. Neither + # exactly the persistence this job exists to catch. Neither content # classifier can explain such a commit (a Renovate bump touches only # `uses:` refs; a tend regen reproduces from `uvx tend init`, which - # does not generate this directory), so anything landing here is - # reported on its own content, which is the intent. + # does not generate this directory); only a clean merge or an admin's + # own push, neither of which a bot can produce alone, is explained. # # .vscode/ is in the window because `tasks.json` can carry # `"runOn": "folderOpen"`, which executes when a maintainer opens the @@ -117,11 +125,14 @@ jobs: # passes the same-commit guard in is_tend_regen, and reproduces # byte-for-byte against a config nothing ever looked at. Widening the # window makes the config edit an auditable commit reported on its own - # content. Both classifiers refuse any commit that touches the config, - # so nothing in the widened window can be swallowed by an arm that - # doesn't inspect it — that pairing is the invariant, not either half. + # content. Both content classifiers refuse any commit that touches the + # config, so nothing in the widened window can be swallowed by an arm + # that doesn't inspect it — that pairing is the invariant, not either + # half. The clean-merge and admin-push arms vouch for the whole commit + # rather than a slice of it, so they need no refusal. # ONE definition of the window. Every consumer below — the commit - # list, `own_changes`, and both classifiers' refusals — must use this + # list, `own_changes`, `is_clean_merge`, and both content classifiers' + # refusals — must use this # same set, or a path that is in the window for one and out of it for # another goes silently unreported: `git log` matches the commit, # `own_changes` returns nothing for it, and the empty-list `continue` @@ -132,7 +143,7 @@ jobs: # unquoted scalar is correct here only because this step runs bash, # and silently matches nothing under a shell that does not word-split. # `"${WINDOW[@]}"` means the same thing everywhere and needs no - # shellcheck exemption. + # exemption from shellcheck. WINDOW=(.github/workflows/ .config/tend.yaml .github/audit/ .vscode/) # DERIVED, never hand-maintained: element 0 is the workflows tree, and # this is everything else — the half both classifiers must refuse @@ -277,10 +288,85 @@ jobs: return $rc } + # A merge that `git merge-tree` reproduces on every window path. Its + # parents are audited on their own, so a merge adding nothing beyond + # their mechanical combination has nothing left to explain. A conflict + # outside the window is irrelevant; one inside it leaves markers in + # the reproduced tree, so its hand resolution is reported. Only a + # two-parent merge qualifies. + is_clean_merge() { + local sha="$1" parents out rc + read -r -a parents <<< "$(git rev-list --parents -n1 "$sha" | cut -d' ' -f2-)" + [ "${#parents[@]}" -eq 2 ] || return 1 + # Exit 1 is "conflicted", with the tree still on the first line; + # anything higher is an error. + out=$(git merge-tree --write-tree "${parents[0]}" "${parents[1]}" 2>/dev/null) && rc=0 || rc=$? + [ "$rc" -le 1 ] || return 1 + git diff --quiet "${out%%$'\n'*}" "$sha" -- "${WINDOW[@]}" + } + + # Repository admin per the collaborator API. Admin is the role the + # `Merge access` and `Tag operations` rulesets exempt, so anyone this + # accepts can already push to `main` directly. + is_admin() { + local login="$1" permission + [[ "$login" =~ ^[A-Za-z0-9-]+$ ]] || return 1 + permission=$(gh api "repos/$GITHUB_REPOSITORY/collaborators/$login/permission" \ + --jq '.permission' 2>/dev/null) || return 1 + [ "$permission" = admin ] + } + + # The earliest ref update in the repository activity log (`$ACTIVITY`, + # server-set timestamps, oldest first) whose range contains the commit, + # as `\t`; nothing if no retained update does. + first_introduction() { + local sha="$1" type actor before after + while IFS=$'\t' read -r _ type actor before after; do + git merge-base --is-ancestor "$sha" "$after" 2>/dev/null || continue + if [[ ! "$before" =~ ^0+$ ]] && git merge-base --is-ancestor "$sha" "$before" 2>/dev/null; then + continue + fi + printf '%s\t%s\n' "$type" "$actor" + return + done < "$ACTIVITY" + } + + # First pushed by an admin: the commit's first introduction is a push, + # force-push, or branch creation by an admin. A PR merge as the first + # introduction means the real push is missing from the log, so it is + # reported. So is a commit whose author or committer claims a bot, even + # if an admin re-pushed it: a rebased bot commit is still bot content. + is_admin_first_push() { + local sha="$1" type actor + if git show -s --format='%ae%n%ce%n%an%n%cn' "$sha" | grep -qiE '(\[bot\]|-bot)(@|$)'; then + return 1 + fi + IFS=$'\t' read -r type actor < <(first_introduction "$sha") || return 1 + case "$type" in + push|force_push|branch_creation) is_admin "$actor" ;; + *) return 1 ;; + esac + } + REPORT=$(mktemp) SKIPPED=$(mktemp) COUNT=0 + # The last quarter of ref updates (the API's longest `time_period` + # short of a year), oldest first by GitHub's own timestamp. Any + # failure empties the file, which `is_admin_first_push` refuses. + ACTIVITY=$(mktemp) + if ! gh api --paginate \ + "repos/$GITHUB_REPOSITORY/activity?per_page=100&time_period=quarter" \ + --jq '.[] | select(.activity_type != "branch_deletion") + | [.timestamp, .activity_type, (.actor.login // ""), (.before // ""), (.after // "")] | @tsv' \ + | sort > "$ACTIVITY.unsorted"; then + echo "Activity log unavailable; the admin-push classifier explains nothing this run." \ + | tee -a "$GITHUB_STEP_SUMMARY" >&2 + : > "$ACTIVITY.unsorted" + fi + mv "$ACTIVITY.unsorted" "$ACTIVITY" + # What this commit itself changed under .github/workflows/. # # For a merge, `git show --name-only` reports nothing, which would @@ -326,8 +412,17 @@ jobs: echo "- \`${sha:0:7}\` — $SUBJECT (reproduces from the tend generator)" >> "$SKIPPED" continue fi + if is_clean_merge "$sha"; then + echo "- \`${sha:0:7}\` — $SUBJECT (clean merge: reproduces from its parents)" >> "$SKIPPED" + continue + fi + if is_admin_first_push "$sha"; then + echo "- \`${sha:0:7}\` — $SUBJECT (first pushed by an admin)" >> "$SKIPPED" + continue + fi COUNT=$((COUNT + 1)) + PUSHER=$(first_introduction "$sha" | awk -F'\t' '{ print $2 " (" $1 ")" }') AUTHOR=$(git show -s --format='%an <%ae>' "$sha") DATE=$(git show -s --format='%ci' "$sha") REFS=$(git branch -a --contains "$sha" 2>/dev/null \ @@ -337,9 +432,11 @@ jobs: echo "### \`${sha:0:7}\` — $SUBJECT" echo "" echo "- **Author:** $AUTHOR (self-declared; not proof of origin)" + echo "- **First pushed by:** ${PUSHER:-not in the activity log}" echo "- **Date:** $DATE" echo "- **Refs:** ${REFS:-none — unreferenced commit}" echo "- **Files:**" + # shellcheck disable=SC2016 # `$` is sed's end-of-line anchor echo "$FILES" | sed 's|^| - `|; s|$|`|' echo "- [View diff](https://github.com/$GITHUB_REPOSITORY/commit/$sha)" echo "" @@ -369,8 +466,9 @@ jobs: { echo "$COUNT unexplained commit(s) in the audit window (\`${WINDOW[*]}\`) since \`$SINCE\`." echo "" - echo "Routine Renovate pin bumps and reproducible tend regenerations are" - echo "classified and omitted — see the run summary for what was skipped." + echo "Renovate pin bumps, reproducible tend regenerations, clean merges, and" + echo "commits first pushed by an admin are classified and omitted — see the" + echo "run summary for what was skipped." echo "Everything below needs a human to account for it." echo "" cat "$REPORT" diff --git a/docs/specs/security-audit.md b/docs/specs/security-audit.md index 996b04dc1..a91c48fcc 100644 --- a/docs/specs/security-audit.md +++ b/docs/specs/security-audit.md @@ -44,7 +44,7 @@ - **FAIL IF** `application-security` or `hosted` does not run on a stronger model than the mechanical domains, in **both** `.github/workflows/security-audit.yaml`'s `claude_args` — its `--model` sets the floor and its `--agents` raises those two domains — and `scripts/security-audit-local.sh` (rationale). - **FAIL IF** `.github/audit/` is missing a prompt file the workflow names, or `scripts/security-audit-local.sh` stops running the audit from those same files (rationale). - **FAIL IF** the union of the subagents' qualitative scopes does not cover every top-level path in the repository (rationale). -- **FAIL IF** `.github/audit/` or `.vscode/` is outside **any** consumer of `.github/workflows/workflow-audit.yaml`'s diff window — the commit list, `own_changes`, and both classifiers' refusals, whose half is *derived* from the single `WINDOW` array (`"${WINDOW[@]:1}"`). Widening one consumer without the others is the failure. The security specs are deliberately *not* watched there (rationale). +- **FAIL IF** `.github/audit/` or `.vscode/` is outside **any** consumer of `.github/workflows/workflow-audit.yaml`'s diff window — the commit list, `own_changes`, `is_clean_merge`, and both content classifiers' refusals, whose half is *derived* from the single `WINDOW` array (`"${WINDOW[@]:1}"`). Widening one consumer without the others is the failure. The security specs are deliberately *not* watched there (rationale). Source of truth: the `**Scope` and `## Qualitative pass` sections of each domain prompt in `.github/audit/`; `claude_args` in `.github/workflows/security-audit.yaml`; `run_domain` in `scripts/security-audit-local.sh`. diff --git a/docs/specs/security-ci.md b/docs/specs/security-ci.md index 6744139c9..013525c58 100644 --- a/docs/specs/security-ci.md +++ b/docs/specs/security-ci.md @@ -42,12 +42,15 @@ This repository runs the [tend](https://github.com/max-sixty/tend) agent harness **Upstream compromise.** Every generated workflow references tend's action as `max-sixty/tend/claude@` — a **tag**, not a commit SHA, and mutable by whoever owns that repository, so upstream can change what our workflows execute with no commit landing here and `workflow-audit.yaml` seeing a byte-identical file. **A real residual, accepted** (rationale). **The version pin bounds *deliberate* upgrades, not a hostile upstream**; `uvx tend@latest` runs only at install and during nightly regen, so a compromise of that path affects the next re-run, not the in-flight workflows. **A second publisher now sits in the same position**: `tend-mention`'s and `tend-notifications`' jobs run `astral-sh/setup-uv@`, whose `uv` then interprets a `run:` step holding `TEND_BOT_TOKEN` — a broader trust than tend's, **accepted on the same generated-file grounds** (rationale). -**Audit visibility.** `.github/workflows/workflow-audit.yaml` walks nightly every commit touching `.github/workflows/`, `.config/tend.yaml`, `.github/audit/`, or `.vscode/` since its previous successful run — **across all branches, not just `main`**, so a workflow pushed to a feature branch is seen even though it never opens a PR. **This enumeration and the job's `WINDOW` must name the same paths** (rationale). It reports the *unexplained*, classifying out two routine sources on independently checked provenance and content: +**Audit visibility.** `.github/workflows/workflow-audit.yaml` walks nightly every commit touching `.github/workflows/`, `.config/tend.yaml`, `.github/audit/`, or `.vscode/` since its previous successful run — **across all branches, not just `main`**, so a workflow pushed to a feature branch is seen even though it never opens a PR. **This enumeration and the job's `WINDOW` must name the same paths** (rationale). It reports the *unexplained*, classifying out four routine sources on independently checked provenance or content: - A **Renovate pin bump** — a valid GitHub-signed commit with `author.login == "renovate[bot]"` and `committer.login == "web-flow"`, associated only with Renovate-authored PRs, changing nothing but the ref of an already-referenced action (rationale). Residual: the ref Renovate selected inside that action's own repo, the trust every Renovate bump already rests on. - A **tend regeneration** — byte-for-byte reproducible from `uvx tend@ init` at the version in the files' own header, not touching `.config/tend.yaml` in the same commit (rationale). +- A **clean merge** — a two-parent merge whose window paths equal `git merge-tree --write-tree` of its parents; a conflict outside the window does not disqualify it. +- An **admin push** — the earliest ref update in the repository activity log (`GET /repos/{repo}/activity`, last quarter, server-set timestamps) whose `before..after` range contains the commit is a `push`, `force_push`, or `branch_creation` by an actor whose collaborator permission is `admin`, and no author or committer field names a bot (rationale). Residual: a bot commit first pushed more than a quarter ago, invisible to the log, then pushed by an admin under a forged human author. -- **Identity is not evidence here at all**: `TEND_BOT_TOKEN` is precisely the credential in question. +- **Never treat a commit's own author or committer as evidence**: `TEND_BOT_TOKEN` is precisely the credential in question. The pusher recorded by GitHub is evidence; the commit's self-declared identity only ever refuses. +- **Must report a commit whose earliest retained introduction is a PR merge**; that is the admin reviewing, not pushing. - **Must report classifier errors and ambiguity as unexplained commits.** - **FAIL IF** tend regeneration materializes anything except regular `.config/tend.yaml` and workflow YAML blobs from the audited commit. `scripts/workflow-audit.test.mjs` pins this boundary (rationale). - **Must still report commits already merged to `main`**; review is not proof (rationale). @@ -80,7 +83,7 @@ This repository runs the [tend](https://github.com/max-sixty/tend) agent harness - **FAIL IF** any job in an agent-managed workflow has **effective** `GITHUB_TOKEN` permissions beyond `contents: write`, `pull-requests: write`, `issues: write`, `id-token: write`, `actions: read`, or any `read` permission. Effective, not declared: apply job permissions over workflow permissions over the repository default; omitted scopes in an explicit block become `none` (rationale). - **FAIL IF** `default_workflow_permissions` for this repository is not `read`, or `can_approve_pull_request_reviews` is not `false` (`gh api repos/diffplug/dormouse/actions/permissions/workflow`) — the backstop for every permission bullet in this spec (rationale). -Source of truth: `packageRules` in `.github/renovate.json`; `WINDOW` and `is_tend_regen` in `.github/workflows/workflow-audit.yaml`. +Source of truth: `packageRules` in `.github/renovate.json`; `WINDOW`, `is_tend_regen`, `is_clean_merge`, and `is_admin_first_push` in `.github/workflows/workflow-audit.yaml`; `scripts/workflow-audit.test.mjs`. ## Hosted Deployments diff --git a/docs/specs/security-ci.rationale.md b/docs/specs/security-ci.rationale.md index e014bd84b..e2ad669dc 100644 --- a/docs/specs/security-ci.rationale.md +++ b/docs/specs/security-ci.rationale.md @@ -38,6 +38,8 @@ **Why regeneration materializes only its inputs.** In [tend 0.2.0's generator](https://github.com/max-sixty/tend/blob/0.2.0/generator/src/tend/cli.py), `init` writes workflows and `.github/actionlint.yaml` with `Path.write_text`, following symlinks. The former full worktree let an audited commit redirect those writes outside the checkout. Materializing only regular config/workflow blobs also excludes attacker-controlled ignore rules that could hide unexpected generated files from `git status`. The regression tests exercise both failures against the shipped classifier. +**Why the admin-push classifier keys on the pusher (2026-09-23).** GitHub records each ref update's authenticated actor in the repository activity log, which `TEND_BOT_TOKEN` cannot set to an admin. Before it, the six open audit issues held 24 commits, 15 of them the admin's own work re-reported on every rebase; a week's replay against the live log explained 32 of 45 window commits, leaving only dormouse-bot pushes and two admin squash merges of bot PRs. The earliest event is the one that counts because a later admin push (a rebase, a merge of `main` into a bot branch) re-contains bot content without authoring it; a PR merge as the earliest event means the actual push fell outside the log. The bot-name refusal covers the rebase case, where the admin push is genuinely the earliest record of a new SHA. + **Why merged commits are still reported.** Review is not proof — the social-engineering path ends in an admin merge. **Why the lower bound stays server-set.** It prevents the pusher from choosing the lower bound, but `--since` still compares attacker-controlled committer dates. Closing backdating and ephemeral-branch evasions would require server-observed pushes, force-pushes, and deletions with timestamps and before/after SHAs, rather than only the current commit graph. diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 555b27d60..397511b17 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -19,7 +19,7 @@ "docs/specs/remote-api.md": 4700, "docs/specs/remote-security-model.md": 4750, "docs/specs/security-audit.md": 1950, - "docs/specs/security-ci.md": 2700, + "docs/specs/security-ci.md": 2850, "docs/specs/security-hosted.md": 600, "docs/specs/security-local.md": 3150, "docs/specs/security-remote.md": 5750, diff --git a/scripts/workflow-audit.test.mjs b/scripts/workflow-audit.test.mjs index cb34c354e..0a533c9e3 100644 --- a/scripts/workflow-audit.test.mjs +++ b/scripts/workflow-audit.test.mjs @@ -43,6 +43,8 @@ function fixture(t) { writeFileSync(target, value); }; const commit = () => { git('add', '-A'); git('commit', '-qm', 'fixture'); return git('rev-parse', 'HEAD'); }; + const activity = join(dir, 'activity.tsv'); + writeFileSync(activity, ''); write('.config/tend.yaml', 'bot_name: test\n'); write('.github/workflows/tend-review.yaml', original); commit(); @@ -53,12 +55,20 @@ printf '%s' '${generated}' > .github/workflows/tend-review.yaml printf 'generated actionlint' > .github/actionlint.yaml if [[ -n "\${EXTRA_WORKFLOW:-}" ]]; then printf 'unexpected' > .github/workflows/tend-extra.yaml; fi `, { mode: 0o755 }); - const classify = (sha, extraEnv = {}) => spawnSync('bash', ['-c', `set -euo pipefail -WINDOW_NON_WORKFLOW=(.config/tend.yaml .github/audit/ .vscode/) + // Answers only the collaborator-permission call; ADMINS lists the admins. + writeFileSync(join(bin, 'gh'), `#!/bin/bash +set -euo pipefail +[[ "$1" = api && "$2" =~ ^repos/test/repo/collaborators/([^/]+)/permission$ ]] || exit 1 +login="\${BASH_REMATCH[1]}" +if [[ " \${ADMINS:-} " = *" $login "* ]]; then echo admin; else echo write; fi +`, { mode: 0o755 }); + const classify = (sha, extraEnv = {}, fn = 'is_tend_regen') => spawnSync('bash', ['-c', `set -euo pipefail +WINDOW=(.github/workflows/ .config/tend.yaml .github/audit/ .vscode/) +WINDOW_NON_WORKFLOW=("\${WINDOW[@]:1}") ${classifier} -is_tend_regen "$1" -`, 'classifier', sha], { cwd: repo, env: { ...env, ...extraEnv }, encoding: 'utf8' }); - return { dir, repo, write, commit, classify }; +${fn} "$1" +`, 'classifier', sha], { cwd: repo, env: { ...env, GITHUB_REPOSITORY: 'test/repo', ACTIVITY: activity, ...extraEnv }, encoding: 'utf8' }); + return { dir, repo, git, write, commit, classify, activity }; } test('accepts exact regeneration and reports changed output', t => { @@ -103,3 +113,123 @@ test('reports a config change even when its workflows reproduce', t => { f.write('.github/workflows/tend-review.yaml', generated); assert.equal(f.classify(f.commit()).status, 1); }); + +const ZERO = '0'.repeat(40); + +function mergeFixture(t) { + const f = fixture(t); + const base = f.git('rev-parse', 'HEAD'); + f.git('switch', '-qc', 'side'); + f.write('.github/audit/side.md', 'side\n'); + f.commit(); + f.git('switch', '-q', '-'); + f.write('.github/audit/main.md', 'main\n'); + f.commit(); + return { ...f, base }; +} + +test('explains a merge that reproduces from its parents', t => { + const f = mergeFixture(t); + f.git('merge', '-q', '--no-edit', 'side'); + assert.equal(f.classify(f.git('rev-parse', 'HEAD'), {}, 'is_clean_merge').status, 0); +}); + +test('reports a merge that edits a window path beyond its parents', t => { + const f = mergeFixture(t); + f.git('merge', '-q', '--no-commit', 'side'); + f.write('.github/audit/main.md', 'evil\n'); + f.git('add', '-A'); + f.git('commit', '-qm', 'merge'); + assert.equal(f.classify(f.git('rev-parse', 'HEAD'), {}, 'is_clean_merge').status, 1); +}); + +test('explains a merge whose only conflict is outside the window', t => { + const f = mergeFixture(t); + f.git('switch', '-q', 'side'); + f.write('package.json', 'side\n'); + f.commit(); + f.git('switch', '-q', '-'); + f.write('package.json', 'main\n'); + f.commit(); + spawnSync('git', ['merge', '-q', '--no-edit', 'side'], { cwd: f.repo }); + f.write('package.json', 'resolved\n'); + f.git('add', '-A'); + f.git('commit', '-qm', 'merge'); + assert.equal(f.classify(f.git('rev-parse', 'HEAD'), {}, 'is_clean_merge').status, 0); +}); + +test('reports a hand-resolved conflict inside the window', t => { + const f = mergeFixture(t); + f.git('switch', '-q', 'side'); + f.write('.github/audit/main.md', 'side\n'); + f.commit(); + f.git('switch', '-q', '-'); + spawnSync('git', ['merge', '-q', '--no-edit', 'side'], { cwd: f.repo }); + f.write('.github/audit/main.md', 'main\n'); + f.git('add', '-A'); + f.git('commit', '-qm', 'merge'); + assert.equal(f.classify(f.git('rev-parse', 'HEAD'), {}, 'is_clean_merge').status, 1); +}); + +test('reports a single-parent commit as not a clean merge', t => { + const f = mergeFixture(t); + assert.equal(f.classify(f.git('rev-parse', 'HEAD'), {}, 'is_clean_merge').status, 1); +}); + +function pushFixture(t, authorEnv = {}) { + const f = fixture(t); + const before = f.git('rev-parse', 'HEAD'); + f.write('.github/audit/change.md', 'change\n'); + f.git('add', '-A'); + execFileSync('git', ['commit', '-qm', 'change'], { cwd: f.repo, env: { ...process.env, ...authorEnv } }); + const sha = f.git('rev-parse', 'HEAD'); + const events = rows => writeFileSync(f.activity, rows.map(r => r.join('\t')).join('\n') + '\n'); + const run = () => { + const result = f.classify(sha, { ADMINS: 'admin-user' }, 'is_admin_first_push'); + assert.ok(result.status < 2, result.stderr); + return result.status; + }; + return { ...f, before, sha, events, run }; +} + +test('explains a commit first pushed by an admin', t => { + const f = pushFixture(t); + f.events([['2026-01-01T00:00:00Z', 'push', 'admin-user', f.before, f.sha]]); + assert.equal(f.run(), 0); +}); + +test('reports a commit a non-admin pushed before an admin did', t => { + const f = pushFixture(t); + f.events([ + ['2026-01-01T00:00:00Z', 'branch_creation', 'bot-user', ZERO, f.sha], + ['2026-01-02T00:00:00Z', 'push', 'admin-user', f.before, f.sha], + ]); + assert.equal(f.run(), 1); +}); + +test('skips updates that already contained the commit', t => { + const f = pushFixture(t); + f.events([ + ['2026-01-01T00:00:00Z', 'push', 'bot-user', f.before, f.before], + ['2026-01-02T00:00:00Z', 'force_push', 'bot-user', f.sha, f.sha], + ['2026-01-03T00:00:00Z', 'push', 'admin-user', f.before, f.sha], + ]); + assert.equal(f.run(), 0); +}); + +test('reports a commit whose first retained introduction is a PR merge', t => { + const f = pushFixture(t); + f.events([['2026-01-01T00:00:00Z', 'pr_merge', 'admin-user', f.before, f.sha]]); + assert.equal(f.run(), 1); +}); + +test('reports a bot-authored commit even when an admin pushed it', t => { + const f = pushFixture(t, { GIT_AUTHOR_NAME: 'dormouse-bot', GIT_AUTHOR_EMAIL: '1+dormouse-bot@users.noreply.github.com' }); + f.events([['2026-01-01T00:00:00Z', 'push', 'admin-user', f.before, f.sha]]); + assert.equal(f.run(), 1); +}); + +test('reports a commit missing from the activity log', t => { + const f = pushFixture(t); + assert.equal(f.run(), 1); +}); From 98eb8ab9ad61424b230bb0a00e108c8d8b1faa4e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Sep 2026 12:06:51 -0700 Subject: [PATCH 2/3] Refuse admin rebases of bot commits and keep null activity fields aligned Review on #769: - A force-push by an admin now explains a commit only if every window commit it replaced was admin-introduced too; a forged human author on a bot commit could otherwise ride an admin rebase. GitHub serves replaced tips by SHA, so missing ones are fetched; an unfetchable tip ends the walk unexplained unless an admin made that update. - Null activity fields become `-`, not empty: tab is IFS whitespace, so an empty column shifted every later one left. Lookups now index the activity log once (tips pinned as refs/audit/*, one for-each-ref --contains per commit, one admin query per actor); the week's replay runs in ~60s with the same 13 unexplained commits. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/workflow-audit.yaml | 109 ++++++++++++++++++++------ docs/specs/security-ci.md | 2 +- docs/specs/security-ci.rationale.md | 2 +- scripts/spec-word-budgets.json | 2 +- scripts/workflow-audit.test.mjs | 67 ++++++++++++++++ 5 files changed, 155 insertions(+), 27 deletions(-) diff --git a/.github/workflows/workflow-audit.yaml b/.github/workflows/workflow-audit.yaml index e4ae723b4..acaffaf52 100644 --- a/.github/workflows/workflow-audit.yaml +++ b/.github/workflows/workflow-audit.yaml @@ -316,56 +316,117 @@ jobs: [ "$permission" = admin ] } - # The earliest ref update in the repository activity log (`$ACTIVITY`, - # server-set timestamps, oldest first) whose range contains the commit, - # as `\t`; nothing if no retained update does. - first_introduction() { - local sha="$1" type actor before after + # Index `$ACTIVITY` once so each lookup is one `for-each-ref`: every + # resolvable tip is pinned as `refs/audit/{after,before}/`, a tip + # missing from the clone is fetched by SHA first (a force-push leaves + # the tip it replaced on no branch), and each actor is asked about + # admin once. `$ROWS` holds `\t\t\t`, + # `state` being `ok`, or `unresolved` when a tip cannot be fetched. + prepare_activity() { + local row=0 type actor before after state missing + ROWS=$(mktemp) + ADMIN_LOGINS=" " + missing=$(cut -f4,5 "$ACTIVITY" | tr '\t' '\n' | grep -E '^[0-9a-f]{40}$' | grep -vE '^0+$' | sort -u \ + | git cat-file --batch-check='%(objectname) %(objecttype)' | awk '$2 == "missing" { print $1 }') || true + if [ -n "$missing" ]; then + printf '%s\n' "$missing" | xargs -n 100 git fetch -q origin 2>/dev/null || true + fi + while IFS= read -r actor; do + if is_admin "$actor"; then ADMIN_LOGINS="$ADMIN_LOGINS$actor "; fi + done < <(cut -f3 "$ACTIVITY" | sort -u) while IFS=$'\t' read -r _ type actor before after; do - git merge-base --is-ancestor "$sha" "$after" 2>/dev/null || continue - if [[ ! "$before" =~ ^0+$ ]] && git merge-base --is-ancestor "$sha" "$before" 2>/dev/null; then - continue + row=$((row + 1)) + state=ok + if git cat-file -e "$after^{commit}" 2>/dev/null; then + echo "update refs/audit/after/$row $after" + else + state=unresolved + fi + if [[ ! "$before" =~ ^0+$ ]]; then + if git cat-file -e "$before^{commit}" 2>/dev/null; then + echo "update refs/audit/before/$row $before" + else + state=unresolved + fi + fi + printf '%s\t%s\t%s\t%s\n' "$row" "$type" "$actor" "$state" >> "$ROWS" + done < "$ACTIVITY" | git update-ref --stdin + } + + # The earliest activity row whose range contains the commit, as + # `\t\t`; nothing if no retained row does. + # An unresolved row might be the introduction: an admin's is skipped + # (the worst case is that a later row decides), anyone else's ends the + # walk as `unresolved`. + first_introduction() { + local after before row type actor state + after=" $(git for-each-ref --contains "$1" --format='%(refname:lstrip=3)' refs/audit/after/ | tr '\n' ' ')" + before=" $(git for-each-ref --contains "$1" --format='%(refname:lstrip=3)' refs/audit/before/ | tr '\n' ' ')" + while IFS=$'\t' read -r row type actor state; do + if [ "$state" = unresolved ]; then + [[ "$ADMIN_LOGINS" = *" $actor "* ]] && continue + printf 'unresolved\t-\t%s\n' "$row" + return fi - printf '%s\t%s\n' "$type" "$actor" + [[ "$after" = *" $row "* && "$before" != *" $row "* ]] || continue + printf '%s\t%s\t%s\n' "$type" "$actor" "$row" return - done < "$ACTIVITY" + done < "$ROWS" } - # First pushed by an admin: the commit's first introduction is a push, - # force-push, or branch creation by an admin. A PR merge as the first - # introduction means the real push is missing from the log, so it is - # reported. So is a commit whose author or committer claims a bot, even - # if an admin re-pushed it: a rebased bot commit is still bot content. - is_admin_first_push() { - local sha="$1" type actor - if git show -s --format='%ae%n%ce%n%an%n%cn' "$sha" | grep -qiE '(\[bot\]|-bot)(@|$)'; then - return 1 - fi - IFS=$'\t' read -r type actor < <(first_introduction "$sha") || return 1 + # Introduced by an admin: the commit's first introduction is a push or + # branch creation by an admin, or an admin's force-push whose replaced + # window commits were each admin-introduced too. That last clause is + # the rebase case: an admin rewriting a bot branch re-pushes the bot's + # content under new SHAs, which its self-declared author cannot flag. + # A PR merge as the first introduction means the real push is missing + # from the log, so it is reported. + admin_introduced() { + local type actor row replaced old + IFS=$'\t' read -r type actor row < <(first_introduction "$1") || return 1 + [[ "$ADMIN_LOGINS" = *" $actor "* ]] || return 1 case "$type" in - push|force_push|branch_creation) is_admin "$actor" ;; + push|branch_creation) ;; + force_push) + replaced=$(git rev-list "refs/audit/before/$row" --not "refs/audit/after/$row" -- "${WINDOW[@]}") || return 1 + for old in $replaced; do + admin_introduced "$old" || return 1 + done + ;; *) return 1 ;; esac } + # Also reports a commit whose author or committer claims a bot, even + # if an admin introduced it: a rebased bot commit is still bot content. + is_admin_first_push() { + if git show -s --format='%ae%n%ce%n%an%n%cn' "$1" | grep -qiE '(\[bot\]|-bot)(@|$)'; then + return 1 + fi + admin_introduced "$1" + } + REPORT=$(mktemp) SKIPPED=$(mktemp) COUNT=0 # The last quarter of ref updates (the API's longest `time_period` # short of a year), oldest first by GitHub's own timestamp. Any - # failure empties the file, which `is_admin_first_push` refuses. + # failure empties the file, which `is_admin_first_push` refuses. A + # null field is `-`, never empty: tab is IFS whitespace, so `read` + # would merge an empty column and shift every later one left. ACTIVITY=$(mktemp) if ! gh api --paginate \ "repos/$GITHUB_REPOSITORY/activity?per_page=100&time_period=quarter" \ --jq '.[] | select(.activity_type != "branch_deletion") - | [.timestamp, .activity_type, (.actor.login // ""), (.before // ""), (.after // "")] | @tsv' \ + | [.timestamp, .activity_type, (.actor.login // "-"), (.before // "-"), (.after // "-")] | @tsv' \ | sort > "$ACTIVITY.unsorted"; then echo "Activity log unavailable; the admin-push classifier explains nothing this run." \ | tee -a "$GITHUB_STEP_SUMMARY" >&2 : > "$ACTIVITY.unsorted" fi mv "$ACTIVITY.unsorted" "$ACTIVITY" + prepare_activity # What this commit itself changed under .github/workflows/. # diff --git a/docs/specs/security-ci.md b/docs/specs/security-ci.md index 013525c58..fb8a571f5 100644 --- a/docs/specs/security-ci.md +++ b/docs/specs/security-ci.md @@ -47,7 +47,7 @@ This repository runs the [tend](https://github.com/max-sixty/tend) agent harness - A **Renovate pin bump** — a valid GitHub-signed commit with `author.login == "renovate[bot]"` and `committer.login == "web-flow"`, associated only with Renovate-authored PRs, changing nothing but the ref of an already-referenced action (rationale). Residual: the ref Renovate selected inside that action's own repo, the trust every Renovate bump already rests on. - A **tend regeneration** — byte-for-byte reproducible from `uvx tend@ init` at the version in the files' own header, not touching `.config/tend.yaml` in the same commit (rationale). - A **clean merge** — a two-parent merge whose window paths equal `git merge-tree --write-tree` of its parents; a conflict outside the window does not disqualify it. -- An **admin push** — the earliest ref update in the repository activity log (`GET /repos/{repo}/activity`, last quarter, server-set timestamps) whose `before..after` range contains the commit is a `push`, `force_push`, or `branch_creation` by an actor whose collaborator permission is `admin`, and no author or committer field names a bot (rationale). Residual: a bot commit first pushed more than a quarter ago, invisible to the log, then pushed by an admin under a forged human author. +- An **admin push** — the earliest ref update in the repository activity log (`GET /repos/{repo}/activity`, last quarter, server-set timestamps) whose `before..after` range contains the commit is a `push` or `branch_creation` by an actor whose collaborator permission is `admin`, or an admin's `force_push` whose replaced window commits are each admin-introduced by the same rule; and no author or committer field names a bot (rationale). An update whose tips cannot be fetched ends the walk unexplained unless an admin made it. Residual: a bot commit carried under a forged human author into an admin push that replaces nothing — a cherry-pick, or a rebase pushed to a new branch — or first pushed more than a quarter ago. - **Never treat a commit's own author or committer as evidence**: `TEND_BOT_TOKEN` is precisely the credential in question. The pusher recorded by GitHub is evidence; the commit's self-declared identity only ever refuses. - **Must report a commit whose earliest retained introduction is a PR merge**; that is the admin reviewing, not pushing. diff --git a/docs/specs/security-ci.rationale.md b/docs/specs/security-ci.rationale.md index e2ad669dc..bab749e58 100644 --- a/docs/specs/security-ci.rationale.md +++ b/docs/specs/security-ci.rationale.md @@ -38,7 +38,7 @@ **Why regeneration materializes only its inputs.** In [tend 0.2.0's generator](https://github.com/max-sixty/tend/blob/0.2.0/generator/src/tend/cli.py), `init` writes workflows and `.github/actionlint.yaml` with `Path.write_text`, following symlinks. The former full worktree let an audited commit redirect those writes outside the checkout. Materializing only regular config/workflow blobs also excludes attacker-controlled ignore rules that could hide unexpected generated files from `git status`. The regression tests exercise both failures against the shipped classifier. -**Why the admin-push classifier keys on the pusher (2026-09-23).** GitHub records each ref update's authenticated actor in the repository activity log, which `TEND_BOT_TOKEN` cannot set to an admin. Before it, the six open audit issues held 24 commits, 15 of them the admin's own work re-reported on every rebase; a week's replay against the live log explained 32 of 45 window commits, leaving only dormouse-bot pushes and two admin squash merges of bot PRs. The earliest event is the one that counts because a later admin push (a rebase, a merge of `main` into a bot branch) re-contains bot content without authoring it; a PR merge as the earliest event means the actual push fell outside the log. The bot-name refusal covers the rebase case, where the admin push is genuinely the earliest record of a new SHA. +**Why the admin-push classifier keys on the pusher (2026-09-23).** GitHub records each ref update's authenticated actor in the repository activity log, which `TEND_BOT_TOKEN` cannot set to an admin. Before it, the six open audit issues held 24 commits, 15 of them the admin's own work re-reported on every rebase; a week's replay against the live log explained 32 of 45 window commits, leaving only dormouse-bot pushes and two admin squash merges of bot PRs. The earliest event is the one that counts because a later admin push (a rebase, a merge of `main` into a bot branch) re-contains bot content without authoring it; a PR merge as the earliest event means the actual push fell outside the log. A rebase is the one place the admin push is genuinely the earliest record of a new SHA, and the bot-name refusal reads self-declared fields a forging bot controls, so a `force_push` also requires that every window commit it replaced was admin-introduced; GitHub serves a replaced tip by SHA, so that check can fetch it. Rebasing a branch the bot pushed to is therefore still reported, which costs some of the noise this classifier removes. **Why merged commits are still reported.** Review is not proof — the social-engineering path ends in an admin merge. diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 397511b17..cee2b6b23 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -19,7 +19,7 @@ "docs/specs/remote-api.md": 4700, "docs/specs/remote-security-model.md": 4750, "docs/specs/security-audit.md": 1950, - "docs/specs/security-ci.md": 2850, + "docs/specs/security-ci.md": 2900, "docs/specs/security-hosted.md": 600, "docs/specs/security-local.md": 3150, "docs/specs/security-remote.md": 5750, diff --git a/scripts/workflow-audit.test.mjs b/scripts/workflow-audit.test.mjs index 0a533c9e3..f7bf135d9 100644 --- a/scripts/workflow-audit.test.mjs +++ b/scripts/workflow-audit.test.mjs @@ -66,6 +66,7 @@ if [[ " \${ADMINS:-} " = *" $login "* ]]; then echo admin; else echo write; fi WINDOW=(.github/workflows/ .config/tend.yaml .github/audit/ .vscode/) WINDOW_NON_WORKFLOW=("\${WINDOW[@]:1}") ${classifier} +prepare_activity ${fn} "$1" `, 'classifier', sha], { cwd: repo, env: { ...env, GITHUB_REPOSITORY: 'test/repo', ACTIVITY: activity, ...extraEnv }, encoding: 'utf8' }); return { dir, repo, git, write, commit, classify, activity }; @@ -233,3 +234,69 @@ test('reports a commit missing from the activity log', t => { const f = pushFixture(t); assert.equal(f.run(), 1); }); + +test('activity rows keep five columns when a field is null', () => { + const filter = auditBlock.match(/activity\?[^"]*" \\\n\s*--jq '([^']*)'/)[1]; + const row = execFileSync('jq', ['-r', filter], { + input: JSON.stringify([{ timestamp: 't', activity_type: 'push', actor: null, before: null, after: 'a' }]), + }).toString().trimEnd(); + assert.deepEqual(row.split('\t'), ['t', 'push', '-', '-', 'a']); +}); + +test('reports a commit whose first introducer has no login', t => { + const f = pushFixture(t); + f.events([ + ['2026-01-01T00:00:00Z', 'push', '-', f.before, f.sha], + ['2026-01-02T00:00:00Z', 'push', 'admin-user', f.before, f.sha], + ]); + assert.equal(f.run(), 1); +}); + +function rebaseFixture(t, firstPusher) { + const f = pushFixture(t); + const original = f.sha; + f.git('reset', '-q', '--hard', f.before); + f.write('unrelated.txt', 'x\n'); + f.commit(); + f.git('cherry-pick', original); + const rebased = f.git('rev-parse', 'HEAD'); + f.events([ + ['2026-01-01T00:00:00Z', 'push', firstPusher, f.before, original], + ['2026-01-02T00:00:00Z', 'force_push', 'admin-user', original, rebased], + ]); + return f.classify(rebased, { ADMINS: 'admin-user' }, 'is_admin_first_push'); +} + +test('reports an admin rebase of commits a non-admin pushed', t => { + const result = rebaseFixture(t, 'bot-user'); + assert.equal(result.status, 1, result.stderr); +}); + +test('explains an admin rebase of the admin\'s own commits', t => { + const result = rebaseFixture(t, 'admin-user'); + assert.equal(result.status, 0, result.stderr); +}); + +test('reports when a non-admin update cannot be resolved', t => { + const f = pushFixture(t); + f.events([ + ['2026-01-01T00:00:00Z', 'push', 'bot-user', f.before, 'f'.repeat(40)], + ['2026-01-02T00:00:00Z', 'push', 'admin-user', f.before, f.sha], + ]); + assert.equal(f.run(), 1); +}); + +test('skips an admin update that cannot be resolved', t => { + const f = pushFixture(t); + f.events([ + ['2026-01-01T00:00:00Z', 'push', 'admin-user', f.before, 'f'.repeat(40)], + ['2026-01-02T00:00:00Z', 'push', 'admin-user', f.before, f.sha], + ]); + assert.equal(f.run(), 0); +}); + +test('reports a commit a non-admin force-pushed first', t => { + const f = pushFixture(t); + f.events([['2026-01-01T00:00:00Z', 'force_push', 'bot-user', f.before, f.sha]]); + assert.equal(f.run(), 1); +}); From d973b0bd31a4cd3a035588767712001cf458fc08 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 23 Sep 2026 12:13:02 -0700 Subject: [PATCH 3/3] Retry failed tip fetches per SHA and never skip a lost admin force-push Review on #769: `git fetch` is all-or-nothing, so one garbage-collected tip cost its whole batch. A failed batch now retries one SHA at a time. The skip rule for unresolvable rows was also too broad: an admin force-push whose tips are lost may be the rewrite that introduced a bot commit under a new SHA, so it now ends the walk unexplained. Only an admin's plain push or branch creation with a lost new tip is skipped. A lost replaced tip needs no state of its own: it gets no ref, so the replaced-commit check fails closed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/workflow-audit.yaml | 25 ++++++++++-------- docs/specs/security-ci.md | 2 +- scripts/workflow-audit.test.mjs | 37 +++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 11 deletions(-) diff --git a/.github/workflows/workflow-audit.yaml b/.github/workflows/workflow-audit.yaml index acaffaf52..cbd321ecb 100644 --- a/.github/workflows/workflow-audit.yaml +++ b/.github/workflows/workflow-audit.yaml @@ -321,7 +321,9 @@ jobs: # missing from the clone is fetched by SHA first (a force-push leaves # the tip it replaced on no branch), and each actor is asked about # admin once. `$ROWS` holds `\t\t\t`, - # `state` being `ok`, or `unresolved` when a tip cannot be fetched. + # `state` being `ok`, or `after-lost` when the new tip cannot be + # fetched. A lost `before` just gets no ref. `git fetch` is all-or-nothing, so a failed batch + # retries one SHA at a time rather than losing its neighbours. prepare_activity() { local row=0 type actor before after state missing ROWS=$(mktemp) @@ -329,7 +331,9 @@ jobs: missing=$(cut -f4,5 "$ACTIVITY" | tr '\t' '\n' | grep -E '^[0-9a-f]{40}$' | grep -vE '^0+$' | sort -u \ | git cat-file --batch-check='%(objectname) %(objecttype)' | awk '$2 == "missing" { print $1 }') || true if [ -n "$missing" ]; then - printf '%s\n' "$missing" | xargs -n 100 git fetch -q origin 2>/dev/null || true + # shellcheck disable=SC2016 # the inner shell expands `$@` + printf '%s\n' "$missing" | xargs -n 100 sh -c \ + 'git fetch -q origin "$@" 2>/dev/null || for sha; do git fetch -q origin "$sha" 2>/dev/null; done' _ || true fi while IFS= read -r actor; do if is_admin "$actor"; then ADMIN_LOGINS="$ADMIN_LOGINS$actor "; fi @@ -340,13 +344,11 @@ jobs: if git cat-file -e "$after^{commit}" 2>/dev/null; then echo "update refs/audit/after/$row $after" else - state=unresolved + state="after-lost" fi if [[ ! "$before" =~ ^0+$ ]]; then if git cat-file -e "$before^{commit}" 2>/dev/null; then echo "update refs/audit/before/$row $before" - else - state=unresolved fi fi printf '%s\t%s\t%s\t%s\n' "$row" "$type" "$actor" "$state" >> "$ROWS" @@ -355,16 +357,18 @@ jobs: # The earliest activity row whose range contains the commit, as # `\t\t`; nothing if no retained row does. - # An unresolved row might be the introduction: an admin's is skipped - # (the worst case is that a later row decides), anyone else's ends the - # walk as `unresolved`. + # A row whose new tip is lost might be the introduction, and ends the + # walk as `unresolved`, with one exception: an admin's plain push or + # branch creation is skipped, since it replaces nothing and the worst + # case is that a later row decides. A `force_push` is never skipped, + # as it may be a rewrite whose replaced commits cannot be checked. first_introduction() { local after before row type actor state after=" $(git for-each-ref --contains "$1" --format='%(refname:lstrip=3)' refs/audit/after/ | tr '\n' ' ')" before=" $(git for-each-ref --contains "$1" --format='%(refname:lstrip=3)' refs/audit/before/ | tr '\n' ' ')" while IFS=$'\t' read -r row type actor state; do - if [ "$state" = unresolved ]; then - [[ "$ADMIN_LOGINS" = *" $actor "* ]] && continue + if [ "$state" = after-lost ]; then + [[ "$type" != force_push && "$ADMIN_LOGINS" = *" $actor "* ]] && continue printf 'unresolved\t-\t%s\n' "$row" return fi @@ -388,6 +392,7 @@ jobs: case "$type" in push|branch_creation) ;; force_push) + # A lost `before` has no ref, so this fails and is reported. replaced=$(git rev-list "refs/audit/before/$row" --not "refs/audit/after/$row" -- "${WINDOW[@]}") || return 1 for old in $replaced; do admin_introduced "$old" || return 1 diff --git a/docs/specs/security-ci.md b/docs/specs/security-ci.md index fb8a571f5..36ff090e8 100644 --- a/docs/specs/security-ci.md +++ b/docs/specs/security-ci.md @@ -47,7 +47,7 @@ This repository runs the [tend](https://github.com/max-sixty/tend) agent harness - A **Renovate pin bump** — a valid GitHub-signed commit with `author.login == "renovate[bot]"` and `committer.login == "web-flow"`, associated only with Renovate-authored PRs, changing nothing but the ref of an already-referenced action (rationale). Residual: the ref Renovate selected inside that action's own repo, the trust every Renovate bump already rests on. - A **tend regeneration** — byte-for-byte reproducible from `uvx tend@ init` at the version in the files' own header, not touching `.config/tend.yaml` in the same commit (rationale). - A **clean merge** — a two-parent merge whose window paths equal `git merge-tree --write-tree` of its parents; a conflict outside the window does not disqualify it. -- An **admin push** — the earliest ref update in the repository activity log (`GET /repos/{repo}/activity`, last quarter, server-set timestamps) whose `before..after` range contains the commit is a `push` or `branch_creation` by an actor whose collaborator permission is `admin`, or an admin's `force_push` whose replaced window commits are each admin-introduced by the same rule; and no author or committer field names a bot (rationale). An update whose tips cannot be fetched ends the walk unexplained unless an admin made it. Residual: a bot commit carried under a forged human author into an admin push that replaces nothing — a cherry-pick, or a rebase pushed to a new branch — or first pushed more than a quarter ago. +- An **admin push** — the earliest ref update in the repository activity log (`GET /repos/{repo}/activity`, last quarter, server-set timestamps) whose `before..after` range contains the commit is a `push` or `branch_creation` by an actor whose collaborator permission is `admin`, or an admin's `force_push` whose replaced window commits are each admin-introduced by the same rule; and no author or committer field names a bot (rationale). An update whose new tip cannot be fetched ends the walk unexplained unless it is an admin's `push` or `branch_creation`; an admin `force_push` whose replaced tip cannot be fetched is unexplained too. Residual: a bot commit carried under a forged human author into an admin push that replaces nothing — a cherry-pick, or a rebase pushed to a new branch — or first pushed more than a quarter ago. - **Never treat a commit's own author or committer as evidence**: `TEND_BOT_TOKEN` is precisely the credential in question. The pusher recorded by GitHub is evidence; the commit's self-declared identity only ever refuses. - **Must report a commit whose earliest retained introduction is a PR merge**; that is the admin reviewing, not pushing. diff --git a/scripts/workflow-audit.test.mjs b/scripts/workflow-audit.test.mjs index f7bf135d9..b71b0238d 100644 --- a/scripts/workflow-audit.test.mjs +++ b/scripts/workflow-audit.test.mjs @@ -300,3 +300,40 @@ test('reports a commit a non-admin force-pushed first', t => { f.events([['2026-01-01T00:00:00Z', 'force_push', 'bot-user', f.before, f.sha]]); assert.equal(f.run(), 1); }); + +test('reports an admin force-push whose replaced tip is lost', t => { + const f = pushFixture(t); + f.events([['2026-01-01T00:00:00Z', 'force_push', 'admin-user', 'f'.repeat(40), f.sha]]); + assert.equal(f.run(), 1); +}); + +test('never skips an admin force-push whose new tip is lost', t => { + const f = pushFixture(t); + f.events([ + ['2026-01-01T00:00:00Z', 'force_push', 'admin-user', f.before, 'f'.repeat(40)], + ['2026-01-02T00:00:00Z', 'push', 'admin-user', f.before, f.sha], + ]); + assert.equal(f.run(), 1); +}); + +test('one unfetchable tip does not cost its batch the others', t => { + const f = pushFixture(t); + const origin = join(f.dir, 'origin.git'); + execFileSync('git', ['init', '-q', '--bare', origin]); + execFileSync('git', ['config', 'uploadpack.allowAnySHA1InWant', 'true'], { cwd: origin }); + f.git('remote', 'add', 'origin', origin); + f.git('push', '-q', 'origin', `${f.sha}:refs/heads/gone`); + execFileSync('git', ['update-ref', '-d', 'refs/heads/gone'], { cwd: origin }); + f.git('reset', '-q', '--hard', f.before); + spawnSync('git', ['update-ref', '-d', 'refs/remotes/origin/gone'], { cwd: f.repo }); + spawnSync('git', ['update-ref', '-d', 'ORIG_HEAD'], { cwd: f.repo }); + f.git('reflog', 'expire', '--expire=now', '--all'); + f.git('gc', '-q', '--prune=now'); + assert.notEqual(spawnSync('git', ['cat-file', '-e', f.sha], { cwd: f.repo }).status, 0); + f.events([ + ['2026-01-01T00:00:00Z', 'push', 'admin-user', f.before, 'f'.repeat(40)], + ['2026-01-02T00:00:00Z', 'push', 'admin-user', f.before, f.sha], + ]); + const result = f.classify(f.sha, {}, 'git cat-file -e'); + assert.equal(result.status, 0, result.stderr); +});