From 88300dba7d8939744c54400b8569eb232e59cf46 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 17:24:35 +0000 Subject: [PATCH 1/3] ci: gate Dependabot auto-merge on the head it will merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workflow-security audit reports `if: github.actor == 'dependabot[bot]'` as a spoofable actor check. Reading it against the sibling workflow turned up a different, larger hole than the one reported. `github.actor` names whoever triggered the run. A push by someone else therefore made this job SKIP — and auto-merge, once enabled, survives later pushes to the head branch. The armed auto-merge stayed armed on a tip nobody re-checked, so a commit appended by anyone with push access could ride an approval it never got. Keying on the pull request's author instead — a fact no push can change — makes the job run on every event of a Dependabot pull request, which is exactly when it needs to act. dependabot/fetch-metadata is a real second gate and was never bypassed by the actor check: reading its source, it re-checks the pull request author, the FIRST commit's author and that commit's signature, never consults github.actor, and fails closed by emitting no outputs (both its skip-*-verification inputs default to false). What it does not check is the TIP, which is what auto-merge acts on. So the head is inspected here, and the two guards are deliberately asymmetric. ARMING requires Dependabot's own GitHub-signed commit — author names forge freely, the signature does not. WITHDRAWING triggers on the weaker signal, an author that is not Dependabot, because withdrawing is the fail-safe direction: at worst a human merges by hand. Keying the withdrawal on the signature instead would fight dependabot-autofix, which deliberately keeps auto-merge on after a trivial fix — its --amend and rebase preserve Dependabot as the author but drop the signature, and that case is now left alone rather than disarmed. Verified with the auditor: the finding is gone, no new one appears, and actionlint still passes on both files. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017bxVrnCNsXW3RvwmLc9Kvy --- .github/workflows/dependabot-automerge.yml | 77 +++++++++++++++++++++- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dependabot-automerge.yml b/.github/workflows/dependabot-automerge.yml index 78d315e2..aa647f0a 100644 --- a/.github/workflows/dependabot-automerge.yml +++ b/.github/workflows/dependabot-automerge.yml @@ -10,6 +10,32 @@ name: dependabot-automerge # Without a branch-protection rule on `main` that marks the CI checks as required, # auto-merge would merge immediately. The required checks are therefore the safety # gate, not this workflow. +# +# Why the job keys on the pull request's AUTHOR and not on `github.actor`. Auto-merge, +# once enabled, SURVIVES later pushes to the head branch. `github.actor` names whoever +# triggered the run, so a push by someone else made the job skip — leaving the +# auto-merge armed on a tip that nobody re-checked. The author of a pull request never +# changes, so keying on it means this job runs on EVERY event of a Dependabot pull +# request, including a push by someone else, which is precisely when it needs to act +# (Sonar githubactions:S8232, zizmor `bot-conditions`). +# +# Identity is then settled twice, and the two answers do different work: +# - `dependabot/fetch-metadata` re-checks the pull request's author and the FIRST +# commit's signature (verified by reading the action's source; both its +# skip-*-verification inputs default to false). It fails closed — no outputs, so +# the steps below are skipped. +# - The TIP check here is the one the action does not do. `fetch-metadata` validates +# the first commit; auto-merge acts on the tip. Somebody with push access can append +# to a Dependabot branch without changing either the author or the first commit. +# +# The two guards are deliberately asymmetric: +# - ENABLING requires the strong proof — the tip is Dependabot's own GitHub-signed +# commit. Commit author NAMES forge freely; GitHub's signature does not. +# - DISABLING triggers on the weaker signal — the tip's author is not Dependabot — +# because disabling is the fail-safe direction: at worst a human merges by hand. +# Keying the disable on the SIGNATURE instead would fight `dependabot-autofix`, +# which deliberately keeps auto-merge on after a trivial fix; its `--amend` and +# `rebase` preserve Dependabot as the author but drop the signature. on: pull_request: @@ -26,8 +52,8 @@ jobs: runs-on: ubuntu-latest # Fetch metadata + enable auto-merge is seconds; cap a stuck runner. timeout-minutes: 10 - # Only act on PRs raised by Dependabot. - if: github.actor == 'dependabot[bot]' + # Only act on pull requests Dependabot OPENED — a fact no later push can change. + if: github.event.pull_request.user.login == 'dependabot[bot]' permissions: # Enable auto-merge (contents) on the pull request (pull-requests). contents: write @@ -37,11 +63,56 @@ jobs: id: meta uses: dependabot/fetch-metadata@25dd0e34f4fe68f24cc83900b1fe3fe149efef98 # v3 + - name: Inspect the head commit this event is about + id: tip + # Reads the tip the checks actually ran against, not whatever the branch points at + # by the time this job runs. A newer push raises its own event and re-enters here. + # Fail closed: anything unreadable answers "not Dependabot's", which only ever + # withholds or withdraws auto-merge. + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + tip="$(gh api "repos/${GH_REPO}/commits/${HEAD_SHA}" \ + --jq '"\(.author.login // "")|\(.commit.verification.verified)"' 2>/dev/null || true)" + author="${tip%%|*}" + echo "head ${HEAD_SHA}: author='${author:-unknown}' signed='${tip##*|}'" + # Signed by Dependabot — the strong proof, required to ARM auto-merge. + if [ "$tip" = "dependabot[bot]|true" ]; then + echo "state=dependabot-signed" >> "$GITHUB_OUTPUT" + # Authored by Dependabot but unsigned: what dependabot-autofix leaves behind + # after rewording or rebasing. Not proof enough to arm, not foreign enough to + # disarm — leave whatever is already set alone. + elif [ "$author" = "dependabot[bot]" ]; then + echo "state=dependabot-unsigned" >> "$GITHUB_OUTPUT" + else + echo "state=foreign" >> "$GITHUB_OUTPUT" + fi + - name: Enable auto-merge for patch and minor updates # Major updates fall through this condition, so they never get auto-merge # and stay open for a human to review. - if: steps.meta.outputs.update-type == 'version-update:semver-patch' || steps.meta.outputs.update-type == 'version-update:semver-minor' + if: >- + steps.tip.outputs.state == 'dependabot-signed' && + (steps.meta.outputs.update-type == 'version-update:semver-patch' || + steps.meta.outputs.update-type == 'version-update:semver-minor') env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_URL: ${{ github.event.pull_request.html_url }} run: gh pr merge --auto --merge "$PR_URL" + + - name: Withdraw auto-merge from a foreign head + # The step that closes the gap: auto-merge armed on Dependabot's own commit stays + # armed across later pushes, so a commit appended by someone with push access would + # ride an approval it never got. Withdrawing is idempotent — `|| true` because the + # call errors when auto-merge was not enabled in the first place, which is the + # ordinary case here. + if: steps.tip.outputs.state == 'foreign' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_URL: ${{ github.event.pull_request.html_url }} + run: | + echo "Head is not Dependabot's commit; withdrawing auto-merge so a human decides." + gh pr merge --disable-auto "$PR_URL" || true From 270bacbfdfc0c3f9ce732f54394a1a06f7be06b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 17:24:44 +0000 Subject: [PATCH 2/3] ci: record why dependabot-autofix keeps its workflow_run trigger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same audit reports this trigger as a privilege-escalation vector, and the pattern generally is one. It is kept, and the reasoning now sits next to it rather than in a review thread. The trigger is not replaceable: a `pull_request` run raised by Dependabot gets a read-only token and no secrets, so it could neither read ANTHROPIC_API_KEY nor push the repair, and no other trigger grants both. The escalation it describes is already closed: the checkout is the BASE and never the pull request, the repair does only git operations so the bumped dependency is never built, and identity is settled against the API with the branch tip required to be Dependabot's own signed commit. That last check is stricter than dependabot/fetch-metadata, which validates the first commit rather than the tip — the gap the sibling workflow just had to close for itself. Silenced for the auditor with an inline directive carrying its reason, per ADR-0060, so the refusal is answered where it fires and not re-litigated the next time the audit runs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017bxVrnCNsXW3RvwmLc9Kvy --- .github/workflows/dependabot-autofix.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dependabot-autofix.yml b/.github/workflows/dependabot-autofix.yml index 8adec3de..396bfb8c 100644 --- a/.github/workflows/dependabot-autofix.yml +++ b/.github/workflows/dependabot-autofix.yml @@ -29,7 +29,22 @@ name: dependabot-autofix # is still pushed, but the checks must be re-triggered by hand. # See doc/handwritten/for-maintainers/workflows/dependabot-autofix.en.md. -on: +# A workflow-security audit (zizmor `dangerous-triggers`) reports this trigger as a +# privilege-escalation vector, and the pattern generally is one: `workflow_run` runs in the +# base context with the repository secrets and a writable token, off the completion of a run +# that may have executed untrusted code. It is kept, because the escalation it describes is +# already closed here and the trigger is not replaceable: +# - Not replaceable — a `pull_request` run raised by Dependabot gets a READ-ONLY token and +# NO secrets, so it could neither read ANTHROPIC_API_KEY nor push the repair. There is no +# other trigger that grants both. +# - The untrusted code is never run — the checkout below is the BASE, never the pull +# request, and the repair does only git operations. The bumped dependency is never built, +# so the write token and the API key never meet freshly bumped third-party code. +# - The identity is settled against the API, not the event payload, and the branch TIP must +# be Dependabot's own GitHub-signed commit (see the resolve step). That is a stricter +# check than `dependabot/fetch-metadata` performs, which validates the FIRST commit. +# Recorded rather than silenced, per ADR-0060: the refusal belongs next to what it refuses. +on: # zizmor: ignore[dangerous-triggers] workflow_run: # Every gating workflow a Dependabot PR passes through. Any completion re-runs # the triage against the head commit's current, combined check status. From f1a6dcac9f182cbcbe7771bd1c4f5024aa127f2f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 17:30:13 +0000 Subject: [PATCH 3/3] docs: describe the Dependabot auto-merge guards as they now stand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The automerge reference documented `github.actor` in three places, including a "the actor guard matters" bullet that is now the opposite of the rule. It describes the author guard instead, why it must not go back to the actor, the head classification and the withdrawal step, and the asymmetry between arming and withdrawing — with the reason the symmetric version would break the sibling workflow, so nobody tidies it into one check. The autofix reference gains one bullet: the audit reports its `workflow_run` trigger, and the answer — not replaceable, escalation already closed, refusal recorded inline — belongs beside the guards it builds on. English and French both. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017bxVrnCNsXW3RvwmLc9Kvy --- .../workflows/dependabot-autofix.en.md | 11 +++++ .../workflows/dependabot-autofix.fr.md | 12 +++++ .../workflows/dependabot-automerge.en.md | 37 +++++++++++++--- .../workflows/dependabot-automerge.fr.md | 44 ++++++++++++++++--- 4 files changed, 91 insertions(+), 13 deletions(-) diff --git a/doc/handwritten/for-maintainers/workflows/dependabot-autofix.en.md b/doc/handwritten/for-maintainers/workflows/dependabot-autofix.en.md index 8ed07dbb..c0c0611f 100644 --- a/doc/handwritten/for-maintainers/workflows/dependabot-autofix.en.md +++ b/doc/handwritten/for-maintainers/workflows/dependabot-autofix.en.md @@ -125,6 +125,17 @@ context. user names), and the branch tip's GitHub signature says who wrote the code the job is about to read, patch and force-push. Commit author and committer *names* are `git config` values and forge freely; the signature does not. +- **A workflow-security audit reports the trigger itself, and the answer is in the + file.** `zizmor`'s `dangerous-triggers` flags `workflow_run` as a + privilege-escalation vector, which the pattern generally is. It is kept because the + trigger is not replaceable — a `pull_request` run raised by Dependabot has a + read-only token and no secrets, so it could neither read the API key nor push — and + because the escalation is already closed by the three properties above: base-only + checkout, no build of the bumped dependency, and API-settled identity down to the + signed tip. That is stricter than `dependabot/fetch-metadata`, which validates the + *first* commit rather than the tip. The refusal is recorded as an inline + `# zizmor: ignore[dangerous-triggers]` carrying its reason (ADR-0060), so it is + answered where it fires rather than re-litigated on each run. ## Related diff --git a/doc/handwritten/for-maintainers/workflows/dependabot-autofix.fr.md b/doc/handwritten/for-maintainers/workflows/dependabot-autofix.fr.md index 8afd0b66..1d479ed3 100644 --- a/doc/handwritten/for-maintainers/workflows/dependabot-autofix.fr.md +++ b/doc/handwritten/for-maintainers/workflows/dependabot-autofix.fr.md @@ -137,6 +137,18 @@ dans son propre contexte Dependabot en lecture seule. écrit le code que le job s'apprête à lire, corriger et force-pusher. Les *noms* d'auteur et de committer d'un commit sont des valeurs `git config` et se forgent librement ; la signature, non. +- **Un audit de sécurité des workflows signale le déclencheur lui-même, et la réponse + est dans le fichier.** La règle `dangerous-triggers` de `zizmor` signale + `workflow_run` comme vecteur d'escalade de privilèges, ce que le motif est en + général. Il est conservé parce qu'il n'est pas remplaçable — un run `pull_request` + levé par Dependabot n'a qu'un token en lecture seule et aucun secret, il ne pourrait + ni lire la clé d'API ni pousser — et parce que l'escalade est déjà fermée par les + trois propriétés ci-dessus : checkout de la base uniquement, aucune compilation de la + dépendance bumpée, identité établie via l'API jusqu'à la pointe signée. C'est plus + strict que `dependabot/fetch-metadata`, qui valide le *premier* commit et non la + pointe. Le refus est consigné par un `# zizmor: ignore[dangerous-triggers]` en ligne + portant sa raison (ADR-0060), pour être répondu là où il se déclenche plutôt que + rejoué à chaque exécution. ## Liens connexes diff --git a/doc/handwritten/for-maintainers/workflows/dependabot-automerge.en.md b/doc/handwritten/for-maintainers/workflows/dependabot-automerge.en.md index a422c524..5e58f2ad 100644 --- a/doc/handwritten/for-maintainers/workflows/dependabot-automerge.en.md +++ b/doc/handwritten/for-maintainers/workflows/dependabot-automerge.en.md @@ -22,15 +22,22 @@ here. ## When it runs - On every **pull request targeting `main`**, but the job is gated on - `github.actor == 'dependabot[bot]'`, so it acts only on Dependabot's PRs. + `github.event.pull_request.user.login == 'dependabot[bot]'` — the pull + request's **author** — so it acts only on Dependabot's PRs. That is + deliberately not `github.actor`; see *Handle with care*. ## How it runs One job, `automerge`: 1. `dependabot/fetch-metadata` reads the update type (patch / minor / major). -2. For **patch or minor** updates, `gh pr merge --auto` enables auto-merge. Major - updates fall through the condition and stay open. +2. The **head commit of this event** is inspected and classified: Dependabot's + own GitHub-signed commit, Dependabot-authored but unsigned, or foreign. +3. For a **signed** head and a **patch or minor** update, `gh pr merge --auto` + enables auto-merge. Major updates fall through the condition and stay open. +4. For a **foreign** head, auto-merge is **withdrawn** (`--disable-auto`). An + unsigned Dependabot-authored head — what `dependabot-autofix` leaves behind + after a reword or a rebase — is left exactly as it is. ## Permissions & security @@ -48,9 +55,27 @@ Workflow default `contents: read`; the job widens to `contents: write` and - **The `major` exclusion is intentional.** Only `semver-patch` and `semver-minor` get auto-merge; majors are left for a human because they are the ones most likely to break. Do not broaden the condition to majors. -- **The actor guard matters.** `if: github.actor == 'dependabot[bot]'` keeps the - elevated `contents: write` / `pull-requests: write` path from running on - human PRs. +- **The guard is the pull request's AUTHOR, and it must not go back to + `github.actor`.** Both keep the elevated `contents: write` / + `pull-requests: write` path off human PRs, but `github.actor` names whoever + triggered the run, so a push by someone else made the job *skip*. Auto-merge + survives later pushes to the head branch, so skipping left it armed on a tip + nobody re-checked. The author of a pull request never changes, so the job now + runs on every event of a Dependabot PR — which is exactly when it needs to act. +- **The two head guards are asymmetric on purpose.** *Arming* requires + Dependabot's own GitHub-signed commit: commit author names are `git config` + values and forge freely, GitHub's signature does not. *Withdrawing* triggers on + the weaker signal — an author that is not Dependabot — because withdrawing is + the fail-safe direction; at worst a human merges by hand. Do not "tidy" this + into one symmetric check: keying the withdrawal on the signature would fight + [`dependabot-autofix`](dependabot-autofix.en.md), whose `--amend` and `rebase` + keep Dependabot as the author but drop the signature, and which deliberately + keeps auto-merge on after a trivial fix. +- **`dependabot/fetch-metadata` is a second gate, but not this one.** It + re-checks the PR author, the **first** commit's author and that commit's + signature, never consults `github.actor`, and fails closed by emitting no + outputs (both its `skip-*-verification` inputs default to `false`). What it + does not check is the **tip** — and the tip is what auto-merge merges. ## Related diff --git a/doc/handwritten/for-maintainers/workflows/dependabot-automerge.fr.md b/doc/handwritten/for-maintainers/workflows/dependabot-automerge.fr.md index f54d8a73..603da8cb 100644 --- a/doc/handwritten/for-maintainers/workflows/dependabot-automerge.fr.md +++ b/doc/handwritten/for-maintainers/workflows/dependabot-automerge.fr.md @@ -23,16 +23,24 @@ ici. ## Quand il s'exécute - À chaque **pull request visant `main`**, mais le job est conditionné à - `github.actor == 'dependabot[bot]'`, donc il n'agit que sur les PR de - Dependabot. + `github.event.pull_request.user.login == 'dependabot[bot]'` — l'**auteur** de + la pull request — donc il n'agit que sur les PR de Dependabot. Ce n'est + délibérément pas `github.actor` ; voir *À manipuler avec précaution*. ## Comment il s'exécute Un seul job, `automerge` : 1. `dependabot/fetch-metadata` lit le type de mise à jour (patch / minor / major). -2. Pour les mises à jour **patch ou minor**, `gh pr merge --auto` active - l'auto-merge. Les majeures ne passent pas la condition et restent ouvertes. +2. Le **commit de tête de cet événement** est inspecté et classé : commit signé + par GitHub appartenant à Dependabot, commit de Dependabot non signé, ou + étranger. +3. Pour une tête **signée** et une mise à jour **patch ou minor**, + `gh pr merge --auto` active l'auto-merge. Les majeures ne passent pas la + condition et restent ouvertes. +4. Pour une tête **étrangère**, l'auto-merge est **retiré** (`--disable-auto`). + Une tête de Dependabot non signée — ce que laisse `dependabot-autofix` après + une réécriture ou un rebase — est laissée telle quelle. ## Permissions & sécurité @@ -52,9 +60,31 @@ la PR. `semver-minor` obtiennent l'auto-merge ; les majeures sont laissées à un humain parce que ce sont elles qui risquent le plus de casser. N'élargissez pas la condition aux majeures. -- **Le garde-fou sur l'acteur compte.** `if: github.actor == 'dependabot[bot]'` - empêche le chemin élevé `contents: write` / `pull-requests: write` de tourner - sur des PR humaines. +- **Le garde-fou porte sur l'AUTEUR de la pull request, et ne doit pas revenir à + `github.actor`.** Les deux empêchent le chemin élevé `contents: write` / + `pull-requests: write` de tourner sur des PR humaines, mais `github.actor` + nomme celui qui a déclenché le run : un push par quelqu'un d'autre faisait donc + *sauter* le job. Or l'auto-merge survit aux pushes suivants sur la branche de + tête ; sauter le laissait donc armé sur une tête que personne ne revérifiait. + L'auteur d'une pull request ne change jamais, si bien que le job tourne + désormais à chaque événement d'une PR Dependabot — c'est-à-dire précisément + quand il doit agir. +- **Les deux gardes sur la tête sont asymétriques à dessein.** *Armer* exige le + commit signé par GitHub appartenant à Dependabot : les noms d'auteur de commit + sont des valeurs `git config` et se forgent librement, la signature de GitHub + non. *Retirer* se déclenche sur le signal faible — un auteur qui n'est pas + Dependabot — parce que retirer est le sens sûr ; au pire un humain merge à la + main. Ne « rangez » pas cela en une vérification symétrique : conditionner le + retrait à la signature entrerait en conflit avec + [`dependabot-autofix`](dependabot-autofix.fr.md), dont les `--amend` et + `rebase` conservent Dependabot comme auteur mais perdent la signature, et qui + garde délibérément l'auto-merge après un correctif trivial. +- **`dependabot/fetch-metadata` est une seconde barrière, mais pas celle-ci.** + Elle revérifie l'auteur de la PR, l'auteur du **premier** commit et la + signature de ce commit, ne consulte jamais `github.actor`, et échoue fermé en + n'émettant aucune sortie (ses deux entrées `skip-*-verification` valent `false` + par défaut). Ce qu'elle ne vérifie pas, c'est la **tête** — et c'est la tête que + l'auto-merge merge. ## En rapport