From 05303290f2eeef2dd67ae647bb80ffe563fbcb42 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:27:01 -0500 Subject: [PATCH 01/17] fix(security): gate Dependabot approve/merge on check success and commit provenance (#1399, #1400) dependabot-rebase-reusable merged with the ruleset-bypassing App once checks were merely complete, so a failed check did not stop the merge (#1399). dependabot-automerge-reusable approved and enabled auto-merge for the whole PR from first-commit Dependabot metadata, including later commits pushed by someone else (#1400). - New bats-tested libs in .github/scripts/dependabot/lib/: pure gates (gates.sh) and gh I/O glue (gh-io.sh); both reusables check them out at job.workflow_sha and source them. - Merger: requires every check run on the head commit to conclude success/neutral/skipped and every status to be success, every ruleset-required check present, and every commit from Dependabot or an update-branch merge by the App; fails closed on unreadable state; merges with sha=. - Automerge: verifies commit provenance and head SHA before approving; approval bound via commit_id, auto-merge via --match-head-commit; on a foreign commit, disables auto-merge and dismisses the App's approvals. - dependabot-tests.yml runs shellcheck + the new bats suite. - standards/dependabot-policy.md documents the new gates. Caller stubs and reusable interfaces are unchanged. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 36 +++ .github/scripts/dependabot/lib/gates.sh | 175 +++++++++++ .github/scripts/dependabot/lib/gh-io.sh | 183 ++++++++++++ .../dependabot-automerge-reusable.yml | 57 +++- .../workflows/dependabot-rebase-reusable.yml | 80 ++--- .github/workflows/dependabot-tests.yml | 68 +++++ standards/dependabot-policy.md | 35 ++- .../dependabot/automerge-provenance.bats | 131 +++++++++ test/workflows/dependabot/gates.bats | 278 ++++++++++++++++++ test/workflows/dependabot/helpers/setup.bash | 197 +++++++++++++ .../dependabot/rebase-merge-gate.bats | 174 +++++++++++ 11 files changed, 1369 insertions(+), 45 deletions(-) create mode 100644 .github/scripts/dependabot/README.md create mode 100644 .github/scripts/dependabot/lib/gates.sh create mode 100644 .github/scripts/dependabot/lib/gh-io.sh create mode 100644 .github/workflows/dependabot-tests.yml create mode 100644 test/workflows/dependabot/automerge-provenance.bats create mode 100644 test/workflows/dependabot/gates.bats create mode 100644 test/workflows/dependabot/helpers/setup.bash create mode 100644 test/workflows/dependabot/rebase-merge-gate.bats diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md new file mode 100644 index 000000000..f3a579278 --- /dev/null +++ b/.github/scripts/dependabot/README.md @@ -0,0 +1,36 @@ +# Dependabot gate scripts + +Decision logic for the org-level Dependabot reusables +(`.github/workflows/dependabot-automerge-reusable.yml` and +`.github/workflows/dependabot-rebase-reusable.yml`). Both workflows feed the +same privileged sink — a GitHub App that approves Dependabot PRs and merges +them with ruleset bypass — so the gates that protect it live here, unit-tested +with bats (`test/workflows/dependabot/`), instead of inline in YAML. + +Each reusable checks this repo out at its own commit (`job.workflow_sha`) into +`.dependabot-tooling/` and sources both libs, so the gates always match the +channel the caller pinned. + +## `lib/gates.sh` — pure decisions + +No network calls. Each gate prints a one-line reason and returns `0` (pass) or +`1` (block). Malformed or missing input always blocks. + +| Function | Passes only when | +|----------|------------------| +| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Every check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present; the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). | +| `dependabot_provenance_gate PR_JSON COMMITS_JSON [APP_LOGIN] [SHA]` | The full commit list was read, at least one commit is Dependabot's, and every commit is either authored by `dependabot[bot]` and committed by GitHub (`web-flow`) or `dependabot[bot]` with a verified signature, or a merge commit by `APP_LOGIN` committed by `web-flow` with a verified signature (the App's update-branch merges). Anything else — e.g. a commit someone else pushed to the branch — blocks (#1400). | +| `dependabot_required_contexts RULES_JSON` | Prints the unique required status-check names from `GET /repos/{r}/rules/branches/{b}` (returns `1` on a non-array). | + +## `lib/gh-io.sh` — GitHub I/O + +Reads state through `gh`, applies the gates, and performs the writes. Any read +failure fails closed. + +| Function | Does | +|----------|------| +| `dependabot_merge_gate REPO PR APP_LOGIN` | Auto-merge requested + `MERGEABLE` + checks gate + provenance gate, all bound to one head SHA. Sets `DEPENDABOT_GATE_SHA` / `DEPENDABOT_GATE_REASON`. Used by the merger before every direct merge. | +| `dependabot_provenance_check REPO PR APP_LOGIN [SHA]` | Provenance gate on the live PR. Returns `0` pass, `1` block (sets `DEPENDABOT_GATE_STALE=true` when the head moved past `SHA`), `2` state unreadable. | +| `dependabot_merge_pr REPO PR SHA` | Squash-merges with `sha=SHA`, so GitHub rejects the merge if the head moved after the gate ran. | +| `dependabot_approve_and_automerge REPO PR SHA` | Approves with `commit_id=SHA` and enables auto-merge with `--match-head-commit SHA`. | +| `dependabot_revoke_automation REPO PR APP_LOGIN REASON` | Best effort: disables auto-merge and dismisses `APP_LOGIN`'s approvals. | diff --git a/.github/scripts/dependabot/lib/gates.sh b/.github/scripts/dependabot/lib/gates.sh new file mode 100644 index 000000000..beaa6581e --- /dev/null +++ b/.github/scripts/dependabot/lib/gates.sh @@ -0,0 +1,175 @@ +#!/usr/bin/env bash +# Merge / approval gates for the Dependabot reusables (issues #1399, #1400). +# +# Pure decisions: no network calls, no sleeps. Each gate takes the JSON the +# I/O glue (lib/gh-io.sh) fetched, prints a one-line reason, and returns +# 0 pass — safe to approve / merge the PR at the verified head SHA +# 1 block — skip the PR (fail closed); the reason says why +# Malformed or missing input always blocks. +# +# Tested by test/workflows/dependabot/gates.bats. +# Contract: see .github/scripts/dependabot/README.md + +# Logins GitHub attributes to genuine Dependabot commits. Dependabot commits are +# authored by dependabot[bot] and committed by GitHub (`web-flow`) with a +# GitHub-verified signature. +DEPENDABOT_LOGIN="dependabot[bot]" + +# dependabot_checks_gate PR_JSON REQUIRED_JSON [EXPECTED_SHA] +# PR_JSON GraphQL `pullRequest` object (or the whole `{data:{repository: +# {pullRequest:…}}}` response) with headRefOid and +# commits(last:1){nodes{commit{oid statusCheckRollup{contexts{ +# pageInfo{hasNextPage} nodes{CheckRun{name status conclusion} +# StatusContext{context state}}}}}}}. +# REQUIRED_JSON JSON array of check names the base branch's rules require +# (`[]` when none). +# EXPECTED_SHA optional — block unless the PR head is this commit. +# +# Passes only when every check run on the head commit COMPLETED as success, +# neutral or skipped, every commit status is success, at least one check was +# reported, and every required check name is present. Anything else — +# failure, cancelled, timed_out, action_required, stale, startup_failure, +# pending, missing, or more contexts than one page — blocks. +dependabot_checks_gate() { + local pr_json="$1" required_json="${2:-[]}" expected="${3:-}" out verdict reason + + out=$(jq -r --argjson required "$required_json" --arg expected "$expected" ' + (if has("data") then .data.repository.pullRequest else . end) as $pr + | ($pr.headRefOid // "") as $head + | ($pr.commits.nodes[0].commit // {}) as $commit + | ($commit.statusCheckRollup.contexts // null) as $ctx + | ($ctx.nodes // []) as $nodes + | def name_of: if .__typename == "StatusContext" then (.context // "") + else (.name // "") end; + def ok: if .__typename == "CheckRun" then + ((.status // "" | ascii_upcase) == "COMPLETED") + and ((.conclusion // "" | ascii_upcase) + | IN("SUCCESS", "NEUTRAL", "SKIPPED")) + elif .__typename == "StatusContext" then + ((.state // "" | ascii_upcase) == "SUCCESS") + else false end; + def outcome: if .__typename == "StatusContext" then (.state // "MISSING") + elif ((.status // "" | ascii_upcase) != "COMPLETED") then (.status // "MISSING") + else (.conclusion // "MISSING") end | ascii_downcase; + ([$nodes[] | name_of]) as $names + | ([$required[] | select(. as $r | $names | index($r) | not)]) as $missing + | ([$nodes[] | select(ok | not) | "\(name_of) (\(outcome))"]) as $bad + | if $head == "" then "block\tPR head commit unknown" + elif ($expected != "" and $head != $expected) then + "block\thead moved to \($head[0:7]) (expected \($expected[0:7]))" + elif ($commit.oid // "") != $head then + "block\tcheck rollup is not for the head commit \($head[0:7])" + elif $ctx == null or ($nodes | length) == 0 then + "block\tno checks reported on \($head[0:7])" + elif ($ctx.pageInfo.hasNextPage // false) then + "block\tmore checks than fit in one page on \($head[0:7]) — cannot verify them all" + elif ($bad | length) > 0 then + "block\t\($bad | length) check(s) not successful on \($head[0:7]): \($bad | join(", "))" + elif ($missing | length) > 0 then + "block\trequired check(s) not reported on \($head[0:7]): \($missing | join(", "))" + else + "pass\tall \($nodes | length) check(s) succeeded on \($head[0:7])" + end + ' <<<"$pr_json" 2>/dev/null) || out="" + + verdict="${out%%$'\t'*}" + reason="${out#*$'\t'}" + if [[ "$verdict" == "pass" ]]; then + echo "$reason" + return 0 + fi + if [[ "$verdict" == "block" ]]; then + echo "$reason" + else + echo "could not evaluate checks (malformed input)" + fi + return 1 +} + +# dependabot_provenance_gate PR_JSON COMMITS_JSON [TRUSTED_MERGE_LOGIN] [EXPECTED_SHA] +# PR_JSON REST `GET /repos/{r}/pulls/{n}` object (head.sha, commits). +# COMMITS_JSON REST `GET /repos/{r}/pulls/{n}/commits` — a flat array, or the +# array of pages `gh api --paginate --slurp` produces. +# TRUSTED_MERGE_LOGIN login of the merge App (e.g. `my-app[bot]`); merge +# commits it created through the update-branch API are allowed. +# Empty — only Dependabot commits are allowed. +# EXPECTED_SHA optional — block unless the PR head is this commit. +# +# Passes only when the full commit list was read (its length equals the PR's +# commit count and contains the head), at least one commit is Dependabot's, +# and EVERY commit is either +# - authored by dependabot[bot], committed by web-flow or dependabot[bot], +# with a GitHub-verified signature; or +# - a merge commit (2+ parents) authored by TRUSTED_MERGE_LOGIN, committed +# by web-flow, with a GitHub-verified signature (update-branch). +# Any other commit — e.g. one pushed to the branch by someone else — blocks. +dependabot_provenance_gate() { + local pr_json="$1" commits_json="$2" trusted="${3:-}" expected="${4:-}" out verdict reason + + # Both documents go through stdin: a long commit list can exceed the + # per-argument size limit of --argjson. + out=$({ printf '%s\n' "$pr_json"; printf '%s\n' "$commits_json"; } | jq -rs \ + --arg trusted "$trusted" --arg expected "$expected" --arg bot "$DEPENDABOT_LOGIN" ' + if length != 2 then error("expected two documents") else . end + | .[0] as $pr | .[1] as $pages + | ($pr.head.sha // "") as $head + | ($pr.commits // -1) as $count + | ([$pages | if type == "array" then flatten[] else empty end + | select(type == "object")]) as $commits + | def verified: (.commit.verification.verified // false) == true; + def author: (.author.login // ""); + def committer: (.committer.login // ""); + def from_dependabot: + author == $bot and (committer | IN("web-flow", $bot)) and verified; + def trusted_merge: + $trusted != "" and author == $trusted and committer == "web-flow" + and ((.parents // []) | length) >= 2 and verified; + ([$commits[] | select((from_dependabot or trusted_merge) | not) + | "\(.sha // "?" | .[0:7]) (author \(author | if . == "" then "unknown" else . end), committer \(committer | if . == "" then "unknown" else . end)\(if verified then "" else ", unverified" end))"]) as $foreign + | if $head == "" then "block\tPR head commit unknown" + elif ($expected != "" and $head != $expected) then + "block\thead moved to \($head[0:7]) (expected \($expected[0:7]))" + elif ($commits | length) == 0 then "block\tno commits read" + elif ($commits | length) != $count then + "block\tread \($commits | length) of \($count) commits — cannot verify them all" + elif ([$commits[] | .sha] | index($head)) == null then + "block\thead \($head[0:7]) not in the commit list" + elif ($foreign | length) > 0 then + "block\t\($foreign | length) commit(s) not from Dependabot: \($foreign | join(", "))" + elif ([$commits[] | select(from_dependabot)] | length) == 0 then + "block\tno Dependabot commit on the PR" + else + "pass\tall \($commits | length) commit(s) from Dependabot\(if $trusted != "" then " or \($trusted) branch updates" else "" end) (head \($head[0:7]))" + end + ' 2>/dev/null) || out="" + + verdict="${out%%$'\t'*}" + reason="${out#*$'\t'}" + if [[ "$verdict" == "pass" ]]; then + echo "$reason" + return 0 + fi + if [[ "$verdict" == "block" ]]; then + echo "$reason" + else + echo "could not evaluate commit provenance (malformed input)" + fi + return 1 +} + +# dependabot_required_contexts RULES_JSON +# RULES_JSON REST `GET /repos/{r}/rules/branches/{b}` — a flat array of +# rules or the array of pages `--paginate --slurp` produces. +# Prints a JSON array of the unique required status-check names. Returns 1 +# (printing nothing) when the input is not an array. +dependabot_required_contexts() { + local rules_json="$1" out + out=$(jq -c ' + if type != "array" then error("not an array") else . end + | [flatten[] | select(type == "object" and .type == "required_status_checks") + | (.parameters.required_status_checks // [])[] | .context + | select(type == "string" and . != "")] | unique + ' <<<"$rules_json" 2>/dev/null) || return 1 + [[ -n "$out" ]] || return 1 + echo "$out" +} diff --git a/.github/scripts/dependabot/lib/gh-io.sh b/.github/scripts/dependabot/lib/gh-io.sh new file mode 100644 index 000000000..351734922 --- /dev/null +++ b/.github/scripts/dependabot/lib/gh-io.sh @@ -0,0 +1,183 @@ +#!/usr/bin/env bash +# GitHub I/O glue for the Dependabot reusables (issues #1399, #1400). +# +# Reads PR / check / commit / ruleset state through `gh` and performs the +# privileged writes (approve, enable auto-merge, merge, revoke). It makes NO +# decision itself — every verdict comes from the pure gates in lib/gates.sh, +# which must be sourced alongside. Any read failure fails closed (skip). +# +# Tested by test/workflows/dependabot/gh-io.bats and the end-to-end suites in +# test/workflows/dependabot/. +# Contract: see .github/scripts/dependabot/README.md + +# dependabot_fetch_pr_state REPO PR_NUMBER +# Prints the GraphQL pullRequest object (headRefOid, baseRefName, mergeable, +# autoMergeRequest and the head commit's check rollup). Returns 1 on failure. +dependabot_fetch_pr_state() { + local repo="$1" pr="$2" out + # shellcheck disable=SC2016 # GraphQL variables, not shell expansions + local query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){headRefOid baseRefName mergeable autoMergeRequest{enabledAt} commits(last:1){nodes{commit{oid statusCheckRollup{contexts(first:100){pageInfo{hasNextPage} nodes{__typename ... on CheckRun{name status conclusion} ... on StatusContext{context state}}}}}}}}}}' + + out=$(gh api graphql -f query="$query" -f owner="${repo%%/*}" \ + -f name="${repo#*/}" -F number="$pr" 2>/dev/null) || return 1 + out=$(jq -c '.data.repository.pullRequest // empty' <<<"$out" 2>/dev/null) || return 1 + [[ -n "$out" ]] || return 1 + echo "$out" +} + +# dependabot_fetch_required_contexts REPO BRANCH +# Prints a JSON array of the status-check names the branch's active rules +# require. Returns 1 when the rules cannot be read. +dependabot_fetch_required_contexts() { + local repo="$1" branch="$2" out + out=$(gh api "repos/${repo}/rules/branches/${branch}?per_page=100" \ + --paginate --slurp 2>/dev/null) || return 1 + dependabot_required_contexts "$out" +} + +# dependabot_fetch_pr REPO PR_NUMBER — prints the REST pull request object. +dependabot_fetch_pr() { + local repo="$1" pr="$2" + gh api "repos/${repo}/pulls/${pr}" 2>/dev/null +} + +# dependabot_fetch_pr_commits REPO PR_NUMBER — prints every page of the PR's +# commits (REST, as `--paginate --slurp` pages). +dependabot_fetch_pr_commits() { + local repo="$1" pr="$2" + gh api "repos/${repo}/pulls/${pr}/commits?per_page=100" --paginate --slurp 2>/dev/null +} + +# dependabot_provenance_check REPO PR_NUMBER TRUSTED_MERGE_LOGIN [EXPECTED_SHA] +# Reads the PR and all its commits, then applies dependabot_provenance_gate. +# Sets DEPENDABOT_GATE_REASON, and DEPENDABOT_GATE_SHA (the verified head) on +# pass. DEPENDABOT_GATE_STALE=true when the head is no longer EXPECTED_SHA +# (a newer push; the run for that push decides). +# Returns 0 pass, 1 block (a verdict), 2 block (the state could not be read — +# no verdict; callers should not act on it beyond not approving/merging). +dependabot_provenance_check() { + local repo="$1" pr="$2" trusted="${3:-}" expected="${4:-}" pr_json commits_json head + DEPENDABOT_GATE_SHA="" + DEPENDABOT_GATE_REASON="" + DEPENDABOT_GATE_STALE=false + + if ! pr_json=$(dependabot_fetch_pr "$repo" "$pr"); then + DEPENDABOT_GATE_REASON="could not read the pull request" + return 2 + fi + head=$(jq -r '.head.sha // empty' <<<"$pr_json" 2>/dev/null) || head="" + if [[ -n "$expected" && -n "$head" && "$head" != "$expected" ]]; then + # shellcheck disable=SC2034 # read by the workflow glue + DEPENDABOT_GATE_STALE=true + DEPENDABOT_GATE_REASON="head moved to ${head:0:7} (expected ${expected:0:7})" + return 1 + fi + if ! commits_json=$(dependabot_fetch_pr_commits "$repo" "$pr"); then + DEPENDABOT_GATE_REASON="could not read the pull request's commits" + return 2 + fi + if ! DEPENDABOT_GATE_REASON=$(dependabot_provenance_gate "$pr_json" "$commits_json" "$trusted" "$expected"); then + return 1 + fi + DEPENDABOT_GATE_SHA="$head" + [[ -n "$DEPENDABOT_GATE_SHA" ]] +} + +# dependabot_merge_gate REPO PR_NUMBER TRUSTED_MERGE_LOGIN +# Decides whether the merger may merge the PR now: auto-merge requested, +# MERGEABLE, every check on the head commit successful (incl. every check the +# base branch's rules require), and every commit from Dependabot (or a branch +# update by TRUSTED_MERGE_LOGIN) — all bound to one head SHA. +# Sets DEPENDABOT_GATE_REASON, and DEPENDABOT_GATE_SHA on pass. +# Returns 0 merge / 1 skip. +dependabot_merge_gate() { + local repo="$1" pr="$2" trusted="${3:-}" state head base required reason + DEPENDABOT_GATE_SHA="" + DEPENDABOT_GATE_REASON="" + + if ! state=$(dependabot_fetch_pr_state "$repo" "$pr"); then + DEPENDABOT_GATE_REASON="could not read the pull request state" + return 1 + fi + if [[ "$(jq -r '.autoMergeRequest != null' <<<"$state")" != "true" ]]; then + DEPENDABOT_GATE_REASON="auto-merge not enabled" + return 1 + fi + if [[ "$(jq -r '.mergeable // "UNKNOWN"' <<<"$state")" != "MERGEABLE" ]]; then + DEPENDABOT_GATE_REASON="not mergeable ($(jq -r '.mergeable // "UNKNOWN"' <<<"$state"))" + return 1 + fi + head=$(jq -r '.headRefOid // empty' <<<"$state") + base=$(jq -r '.baseRefName // empty' <<<"$state") + if [[ -z "$head" || -z "$base" ]]; then + DEPENDABOT_GATE_REASON="could not read the head commit / base branch" + return 1 + fi + if ! required=$(dependabot_fetch_required_contexts "$repo" "$base"); then + DEPENDABOT_GATE_REASON="could not read the required checks for ${base}" + return 1 + fi + if ! reason=$(dependabot_checks_gate "$state" "$required" "$head"); then + DEPENDABOT_GATE_REASON="$reason" + return 1 + fi + if ! dependabot_provenance_check "$repo" "$pr" "$trusted" "$head"; then + DEPENDABOT_GATE_SHA="" + return 1 + fi + DEPENDABOT_GATE_REASON="${reason}; ${DEPENDABOT_GATE_REASON}" + DEPENDABOT_GATE_SHA="$head" +} + +# dependabot_merge_pr REPO PR_NUMBER SHA +# Squash-merges the PR, bound to SHA: GitHub refuses the merge (409) if the +# head moved after it was verified. +dependabot_merge_pr() { + local repo="$1" pr="$2" sha="$3" + [[ -n "$sha" ]] || return 1 + gh api "repos/${repo}/pulls/${pr}/merge" -X PUT \ + -f merge_method=squash -f sha="$sha" --silent +} + +# dependabot_approve_and_automerge REPO PR_NUMBER SHA +# Approves the PR at commit SHA (the review is bound to that commit) and +# enables squash auto-merge only while the head is still SHA. +dependabot_approve_and_automerge() { + local repo="$1" pr="$2" sha="$3" + [[ -n "$sha" ]] || return 1 + gh api "repos/${repo}/pulls/${pr}/reviews" -X POST \ + -f event=APPROVE -f commit_id="$sha" --silent || return 1 + gh pr merge "$pr" --repo "$repo" --auto --squash --match-head-commit "$sha" +} + +# dependabot_revoke_automation REPO PR_NUMBER APP_LOGIN REASON +# Best effort, always returns 0: disables auto-merge and dismisses approvals +# APP_LOGIN gave earlier, so a revision that failed the provenance gate is not +# merged on the strength of an approval of an earlier revision. +dependabot_revoke_automation() { + local repo="$1" pr="$2" app_login="$3" reason="$4" reviews id + + if ! gh pr merge "$pr" --repo "$repo" --disable-auto >/dev/null 2>&1; then + echo " auto-merge was not enabled (or could not be disabled)" + else + echo " Disabled auto-merge" + fi + + [[ -n "$app_login" ]] || return 0 + if ! reviews=$(gh api "repos/${repo}/pulls/${pr}/reviews?per_page=100" --paginate --slurp 2>/dev/null); then + echo "::warning::Could not list reviews on PR #${pr} to dismiss earlier automated approvals" + return 0 + fi + while IFS= read -r id; do + [[ -n "$id" ]] || continue + if gh api "repos/${repo}/pulls/${pr}/reviews/${id}/dismissals" -X PUT \ + -f event=DISMISS -f message="Automated approval withdrawn: ${reason}" --silent; then + echo " Dismissed automated approval ${id}" + else + echo "::warning::Could not dismiss automated approval ${id} on PR #${pr}" + fi + done < <(jq -r --arg login "$app_login" \ + 'flatten[] | select(.user.login == $login and .state == "APPROVED") | .id' \ + <<<"$reviews" 2>/dev/null) + return 0 +} diff --git a/.github/workflows/dependabot-automerge-reusable.yml b/.github/workflows/dependabot-automerge-reusable.yml index be2fa7cfd..610510a1a 100644 --- a/.github/workflows/dependabot-automerge-reusable.yml +++ b/.github/workflows/dependabot-automerge-reusable.yml @@ -15,6 +15,14 @@ # security updates. This workflow adds defense-in-depth by also checking # the package ecosystem. # +# Provenance gate (issue #1400): Dependabot metadata describes only the +# Dependabot commit, so before approving, every commit on the PR must be from +# Dependabot (or a branch update made by this App via update-branch) and the +# head must still be the commit this event was for. The approval and the +# auto-merge request are bound to that head SHA. If a later commit fails the +# gate, auto-merge is disabled and this App's earlier approvals are dismissed. +# Decision logic lives in .github/scripts/dependabot/lib/ (bats-tested). +# # Required org/repo secrets (passed via `secrets: inherit` from caller): # APP_ID — GitHub App ID with contents:write and pull-requests:write # APP_PRIVATE_KEY — GitHub App private key @@ -88,11 +96,54 @@ jobs: app-id: ${{ secrets.APP_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} + - name: Checkout Dependabot gate tooling + if: steps.eligible.outputs.eligible == 'true' + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + repository: combsmith/.github + # This reusable's own commit, so the gate scripts always match the + # channel the caller pinned (same pattern as feature-ideation, #839). + ref: ${{ job.workflow_sha }} + path: .dependabot-tooling + fetch-depth: 1 + persist-credentials: false + - name: Approve and enable auto-merge if: steps.eligible.outputs.eligible == 'true' run: | - gh pr review --approve "$PR_URL" - gh pr merge --auto --squash "$PR_URL" + # shellcheck source=/dev/null + . .dependabot-tooling/.github/scripts/dependabot/lib/gates.sh + # shellcheck source=/dev/null + . .dependabot-tooling/.github/scripts/dependabot/lib/gh-io.sh + + APP_LOGIN="${APP_SLUG:+${APP_SLUG}[bot]}" + + # Reads use the job's GITHUB_TOKEN; only the writes use the App token. + rc=0 + GH_TOKEN="$READ_TOKEN" dependabot_provenance_check "$REPO" "$PR_NUMBER" "$APP_LOGIN" "$EVENT_HEAD_SHA" || rc=$? + if [[ "$rc" -eq 2 ]]; then + echo "::error::Not approving PR #$PR_NUMBER — $DEPENDABOT_GATE_REASON" + exit 1 + fi + if [[ "$rc" -ne 0 ]]; then + if [[ "$DEPENDABOT_GATE_STALE" == "true" ]]; then + # A newer push superseded this event; its own run decides. + echo "Not approving PR #$PR_NUMBER — $DEPENDABOT_GATE_REASON" + exit 0 + fi + # Commits on the PR that did not come from Dependabot: withdraw any + # earlier automated approval / auto-merge and leave it to a human. + echo "::warning::Not approving PR #$PR_NUMBER — $DEPENDABOT_GATE_REASON" + dependabot_revoke_automation "$REPO" "$PR_NUMBER" "$APP_LOGIN" "$DEPENDABOT_GATE_REASON" + exit 0 + fi + + echo "Provenance gate passed ($DEPENDABOT_GATE_REASON) — approving $DEPENDABOT_GATE_SHA" + dependabot_approve_and_automerge "$REPO" "$PR_NUMBER" "$DEPENDABOT_GATE_SHA" env: - PR_URL: ${{ github.event.pull_request.html_url }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + EVENT_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + APP_SLUG: ${{ steps.app-token.outputs.app-slug }} + READ_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_TOKEN: ${{ steps.app-token.outputs.token }} diff --git a/.github/workflows/dependabot-rebase-reusable.yml b/.github/workflows/dependabot-rebase-reusable.yml index 0aec46c99..17470f7fa 100644 --- a/.github/workflows/dependabot-rebase-reusable.yml +++ b/.github/workflows/dependabot-rebase-reusable.yml @@ -17,7 +17,13 @@ # bringing it up to date. CI triggers normally because the APP_TOKEN is # a GitHub App installation token — NOT the repository's GITHUB_TOKEN, # which is the only token subject to GitHub's recursive-trigger guard. -# 2. Merges any Dependabot PR that is up-to-date and passing required CI. +# 2. Merges any Dependabot PR that is up-to-date and whose head commit passes +# the merge gate (issues #1399, #1400) — auto-merge requested, MERGEABLE, +# EVERY check on the head commit successful (success/neutral/skipped; +# every check the base branch's rules require must be present), and every +# commit on the PR from Dependabot or a branch update by this App. The +# merge is bound to the verified head SHA. Decision logic lives in +# .github/scripts/dependabot/lib/ (bats-tested); this file is glue. # # Why update-branch with APP_TOKEN (not GITHUB_TOKEN or @dependabot rebase): # - GITHUB_TOKEN: GitHub's recursive-trigger guard prevents events from @@ -36,7 +42,9 @@ # its dependabot/fetch-metadata verification step (since the latest commit # is not from Dependabot), but this is safe — the rebase workflow merges # directly via the APP_TOKEN bypass actor and does not rely on the automerge -# workflow's re-approval. +# workflow's re-approval. These App-made merge commits are the only +# non-Dependabot commits the merge gate accepts; any other commit on the +# branch keeps the PR out of the automated merge. # # Required org/repo secrets (passed explicitly by caller via secrets: mapping): # APP_ID — GitHub App ID with contents:write and pull-requests:write @@ -59,8 +67,20 @@ jobs: update-and-merge: runs-on: ubuntu-latest permissions: + contents: read # check out the gate scripts from combsmith/.github pull-requests: write steps: + - name: Checkout Dependabot gate tooling + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + repository: combsmith/.github + # This reusable's own commit, so the gate scripts always match the + # channel the caller pinned (same pattern as feature-ideation, #839). + ref: ${{ job.workflow_sha }} + path: .dependabot-tooling + fetch-depth: 1 + persist-credentials: false + - name: Check app secrets env: APP_ID: ${{ secrets.APP_ID }} @@ -81,8 +101,18 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} APP_TOKEN: ${{ steps.app-token.outputs.token }} + APP_SLUG: ${{ steps.app-token.outputs.app-slug }} REPO: ${{ github.repository }} run: | + # shellcheck source=/dev/null + . .dependabot-tooling/.github/scripts/dependabot/lib/gates.sh + # shellcheck source=/dev/null + . .dependabot-tooling/.github/scripts/dependabot/lib/gh-io.sh + + # Branch updates this App makes (update-branch merge commits) are the + # only non-Dependabot commits the merge gate accepts. + APP_LOGIN="${APP_SLUG:+${APP_SLUG}[bot]}" + # Find open Dependabot PRs PRS=$(gh pr list --repo "$REPO" --author "app/dependabot" \ --json number,headRefName \ @@ -139,45 +169,25 @@ jobs: continue fi - # Check if auto-merge is enabled (set by the automerge workflow) - AUTO_MERGE=$(gh pr view "$PR_NUMBER" --repo "$REPO" \ - --json autoMergeRequest --jq '.autoMergeRequest != null') - - if [[ "$AUTO_MERGE" != "true" ]]; then - echo " Skipping — auto-merge not enabled" - continue - fi - - # Trust GitHub's MERGEABLE state: it already accounts for all - # REQUIRED status checks, review rules, and branch-up-to-date. - # Non-required checks (e.g. gitleaks false positives) that happen - # to fail do not block merges as long as required checks pass. - MERGEABLE=$(gh pr view "$PR_NUMBER" --repo "$REPO" \ - --json mergeable --jq '.mergeable') - - if [[ "$MERGEABLE" != "MERGEABLE" ]]; then - echo " Skipping — not mergeable ($MERGEABLE)" - continue - fi - - # Also confirm no checks are still in progress — avoids merging - # mid-CI when GitHub reports MERGEABLE based on a previous run. - CHECKS_PENDING=$(gh pr view "$PR_NUMBER" --repo "$REPO" \ - --json statusCheckRollup | \ - jq '[.statusCheckRollup[]? | select(.name != null and .status != "COMPLETED")] | length') - - if [[ "$CHECKS_PENDING" -gt 0 ]]; then - echo " Skipping — $CHECKS_PENDING check(s) still pending" + # The App merges with ruleset bypass, so GitHub's own gates do not + # stop it — the merge gate must. It requires auto-merge requested + # (set by the automerge workflow), MERGEABLE, EVERY check on the + # head commit successful (fail closed on failure, cancelled, + # timed_out, action_required, stale, startup_failure, pending or a + # missing required check), and every commit from Dependabot or a + # branch update by this App (#1399, #1400). + if ! dependabot_merge_gate "$REPO" "$PR_NUMBER" "$APP_LOGIN"; then + echo " Skipping — $DEPENDABOT_GATE_REASON" continue fi - echo " All required checks pass — merging PR #$PR_NUMBER" + echo " Merge gate passed ($DEPENDABOT_GATE_REASON) — merging PR #$PR_NUMBER at $DEPENDABOT_GATE_SHA" # Use app token for merge so the resulting push to main is attributed to the # app bot — this triggers the workflow again via push event, enabling the # self-sustaining chain that serializes Dependabot PR merges one at a time. - if GH_TOKEN="$APP_TOKEN" gh api "repos/$REPO/pulls/$PR_NUMBER/merge" \ - -X PUT -f merge_method=squash \ - --silent; then + # The merge is bound to the verified head SHA: GitHub rejects it if + # the branch moved after the gate ran. + if GH_TOKEN="$APP_TOKEN" dependabot_merge_pr "$REPO" "$PR_NUMBER" "$DEPENDABOT_GATE_SHA"; then echo " Merged PR #$PR_NUMBER" MERGED=true else diff --git a/.github/workflows/dependabot-tests.yml b/.github/workflows/dependabot-tests.yml new file mode 100644 index 000000000..0525a2b5e --- /dev/null +++ b/.github/workflows/dependabot-tests.yml @@ -0,0 +1,68 @@ +# Quality gates for the Dependabot automerge / update-and-merge reusables and +# their merge / approval gate scripts (issues #1399, #1400). +# +# Triggered on any PR that touches: +# - .github/workflows/dependabot-*.yml +# - .github/scripts/dependabot/** +# - test/workflows/dependabot/** +# - standards/workflows/dependabot-*.yml +# +# Gates (all must pass before merge): +# 1. shellcheck — static analysis for the gate libs +# 2. bats — unit tests for the pure gates + end-to-end tests of the +# reusables' approve / merge run-blocks against a stubbed gh +# +# Contract: .github/scripts/dependabot/README.md + +name: Dependabot Gate Tests + +on: + pull_request: + paths: + - '.github/workflows/dependabot-*.yml' + - '.github/scripts/dependabot/**' + - 'test/workflows/dependabot/**' + - 'standards/workflows/dependabot-*.yml' + push: + branches: + - main + paths: + - '.github/workflows/dependabot-*.yml' + - '.github/scripts/dependabot/**' + - 'test/workflows/dependabot/**' + - 'standards/workflows/dependabot-*.yml' + +permissions: + contents: read + +concurrency: + group: dependabot-tests-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: Lint and bats + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 1 + persist-credentials: false + + - name: Install bats, shellcheck, and jq (yq is preinstalled on the runner) + run: | + set -euo pipefail + sudo apt-get update -qq + sudo apt-get install -y --no-install-recommends bats shellcheck jq + + - name: shellcheck + run: | + set -euo pipefail + shellcheck -x \ + .github/scripts/dependabot/lib/gates.sh \ + .github/scripts/dependabot/lib/gh-io.sh + + - name: Run bats suite + run: bats --print-output-on-failure test/workflows/dependabot/ diff --git a/standards/dependabot-policy.md b/standards/dependabot-policy.md index ed577214a..0a94187d4 100644 --- a/standards/dependabot-policy.md +++ b/standards/dependabot-policy.md @@ -168,6 +168,14 @@ Behavior: - For **app ecosystems**: approves **patch** and **minor** updates (and indirect dependency updates); **major** updates are left for human review - Uses `gh pr merge --auto --squash` so the merge only happens after CI passes +- **Provenance gate:** Dependabot metadata describes only Dependabot's commit, so + before approving, every commit on the PR must be from Dependabot (or a branch + update the App made via `update-branch`) and the head must still be the commit + the event was for. The approval (`commit_id`) and the auto-merge request + (`--match-head-commit`) are bound to that head SHA. If any other commit lands + on the branch, the workflow does not approve, disables auto-merge and dismisses + its earlier approval — the PR then needs a human review. To hand it back to + automation, have Dependabot recreate it (`@dependabot recreate`). ## Update and Merge Behind PRs Workflow @@ -189,7 +197,8 @@ This workflow fires on every push to `main` (and can be triggered manually via commit from `main` onto the PR branch. CI triggers normally because the APP_TOKEN is a GitHub App installation token, not `GITHUB_TOKEN`. 2. **Merges ready PRs** — directly merges any Dependabot PR that is up-to-date, - has auto-merge enabled, and is `MERGEABLE` with no pending checks. + has auto-merge enabled, is `MERGEABLE`, and passes the merge gate (see + *Merge-readiness check* below). The merge is bound to the verified head SHA. Using the app token for merges ensures each merge triggers a new push to `main`, creating a self-sustaining chain that serializes Dependabot PR merges. @@ -234,12 +243,24 @@ date before merge), the fallback direct merge will also fail — granting the identity and is **not** subject to the `GITHUB_TOKEN` recursive-trigger guard. Pushes via APP_TOKEN trigger CI workflow runs normally. -**Merge-readiness check:** The workflow checks GitHub's native `mergeable` state -(`MERGEABLE`) rather than inspecting every individual check conclusion. This is -correct because `mergeable` already accounts for all *required* status checks — -non-required checks that happen to fail (e.g. a gitleaks false positive) do not -block the merge. An additional guard ensures no checks are still `IN_PROGRESS` -before merging. +**Merge-readiness check:** The App merges with ruleset bypass, so GitHub's +`mergeable` state is not enough — bypass also skips the required-check +requirement. Before each direct merge the workflow requires, for the PR's +current head commit: + +- auto-merge requested and `mergeable` is `MERGEABLE`; +- **every** check run concluded `success`, `neutral` or `skipped` and every + commit status is `success` — a failed, cancelled, timed-out, action-required, + stale or still-pending check (required or not) keeps the PR out of the + automated merge; +- every status check the base branch's rulesets require has reported; +- every commit on the PR is from Dependabot or is an `update-branch` merge by + the App (same provenance gate as the auto-merge workflow). + +The merge call passes the verified head SHA, so GitHub rejects it if the branch +moved after the check. A Dependabot PR held back by a failing non-required check +(e.g. a gitleaks false positive) needs a human to fix the check or merge it. +Gate logic: [`.github/scripts/dependabot/`](../.github/scripts/dependabot/README.md). ### Caller Stub Format diff --git a/test/workflows/dependabot/automerge-provenance.bats b/test/workflows/dependabot/automerge-provenance.bats new file mode 100644 index 000000000..8d6c10388 --- /dev/null +++ b/test/workflows/dependabot/automerge-provenance.bats @@ -0,0 +1,131 @@ +#!/usr/bin/env bats +# End-to-end tests for the approval step of dependabot-automerge-reusable.yml +# (issue #1400). +# +# dependabot/fetch-metadata describes only the Dependabot commit, but the +# workflow approved and enabled auto-merge for the WHOLE PR — including any +# commits pushed to the branch later by someone else. These tests run the +# workflow's real "Approve and enable auto-merge" run-block against a +# fixture-driven `gh` stub and assert on what it approves. + +load 'helpers/setup' + +REUSABLE="${TT_REPO_ROOT}/.github/workflows/dependabot-automerge-reusable.yml" + +setup() { + tt_setup + RUN_SCRIPT="${TT_TMP}/run.sh" + tt_extract_run "$REUSABLE" dependabot "Approve and enable auto-merge" > "$RUN_SCRIPT" +} + +teardown() { + tt_teardown +} + +# _run_approve_step [EVENT_HEAD_SHA] +_run_approve_step() { + cd "$TT_WORK" || return 1 + REPO="owner/repo" PR_NUMBER=5 EVENT_HEAD_SHA="${1:-$HEAD_SHA}" \ + APP_SLUG="$APP_SLUG_FIXTURE" READ_TOKEN="read-token" GH_TOKEN="app-token" \ + PR_URL="https://github.com/owner/repo/pull/5" \ + run bash -eo pipefail "$RUN_SCRIPT" +} + +_writes() { cat "${FIX_DIR}/writes.log" 2>/dev/null || true; } + +_seed() { # COMMITS_JSON + tt_seed_pr "$(tt_pr_state "[$(tt_check_run ci COMPLETED SUCCESS)]")" "$1" +} + +_foreign_commits() { + printf '[%s,%s]' \ + "$(tt_commit "$DEP_SHA" 'dependabot[bot]' web-flow true 1)" \ + "$(tt_commit "$HEAD_SHA" mallory mallory true 1)" +} + +@test "automerge: the approval step sources the gate libraries" { + [ -s "$RUN_SCRIPT" ] + grep -F '.dependabot-tooling/.github/scripts/dependabot/lib/gates.sh' "$RUN_SCRIPT" + grep -F '.dependabot-tooling/.github/scripts/dependabot/lib/gh-io.sh' "$RUN_SCRIPT" +} + +@test "automerge: tooling checkout runs at the reusable's own commit, only when eligible" { + run yq -r '.jobs.dependabot.steps[] | select(.name == "Checkout Dependabot gate tooling") | .with.ref' "$REUSABLE" + [ "$output" = '${{ job.workflow_sha }}' ] + run yq -r '.jobs.dependabot.steps[] | select(.name == "Checkout Dependabot gate tooling") | .if' "$REUSABLE" + [ "$output" = "steps.eligible.outputs.eligible == 'true'" ] +} + +@test "automerge: Dependabot-only PR is approved at the head SHA and auto-merge is bound to it" { + _seed "[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' web-flow true 1)]" + _run_approve_step + [ "$status" -eq 0 ] + run _writes + [[ "$output" == *"TOKEN=app-token POST repos/owner/repo/pulls/5/reviews"*"event=APPROVE"*"commit_id=${HEAD_SHA}"* ]] + [[ "$output" == *"TOKEN=app-token pr merge 5 --repo owner/repo --auto --squash --match-head-commit ${HEAD_SHA}"* ]] +} + +@test "automerge: the App's update-branch merge commit does not block re-approval" { + _seed "[$(tt_commit "$DEP_SHA" 'dependabot[bot]' web-flow true 1),$(tt_commit "$HEAD_SHA" "$APP_LOGIN_FIXTURE" web-flow true 2)]" + _run_approve_step + [ "$status" -eq 0 ] + [[ "$(_writes)" == *"event=APPROVE"* ]] +} + +@test "automerge: a later non-Dependabot commit is NOT approved (regression #1400)" { + _seed "$(_foreign_commits)" + _run_approve_step + [ "$status" -eq 0 ] + [[ "$output" == *"Not approving PR #5"*"not from Dependabot"* ]] + run _writes + [[ "$output" != *"event=APPROVE"* ]] + [[ "$output" != *"--approve"* ]] + [[ "$output" != *"--auto --squash"* ]] +} + +@test "automerge: a non-Dependabot commit disables auto-merge and dismisses only the App's approvals" { + _seed "$(_foreign_commits)" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" \ + "[{\"id\":11,\"state\":\"APPROVED\",\"user\":{\"login\":\"${APP_LOGIN_FIXTURE}\"}},{\"id\":12,\"state\":\"APPROVED\",\"user\":{\"login\":\"alice\"}},{\"id\":13,\"state\":\"COMMENTED\",\"user\":{\"login\":\"${APP_LOGIN_FIXTURE}\"}}]" + _run_approve_step + [ "$status" -eq 0 ] + run _writes + [[ "$output" == *"pr merge 5 --repo owner/repo --disable-auto"* ]] + [[ "$output" == *"PUT repos/owner/repo/pulls/5/reviews/11/dismissals"* ]] + [[ "$output" != *"reviews/12/dismissals"* ]] + [[ "$output" != *"reviews/13/dismissals"* ]] +} + +@test "automerge: revocation is best effort (failures do not fail the step)" { + _seed "$(_foreign_commits)" + printf '1\n' > "${FIX_DIR}/pr_merge.rc" + _run_approve_step + [ "$status" -eq 0 ] + [[ "$(_writes)" != *"event=APPROVE"* ]] +} + +@test "automerge: a stale event (head moved since) approves nothing and revokes nothing" { + _seed "[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' web-flow true 1)]" + _run_approve_step "$OTHER_SHA" + [ "$status" -eq 0 ] + [[ "$output" == *"head moved"* ]] + [ -z "$(_writes)" ] +} + +@test "automerge: unreadable commits fail the step without approving or revoking" { + _seed "[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' web-flow true 1)]" + tt_fixture "repos/owner/repo/pulls/5/commits?per_page=100" '{"message":"Server Error","status":"500"}' 1 + _run_approve_step + [ "$status" -eq 1 ] + [[ "$output" == *"could not read the pull request's commits"* ]] + [ -z "$(_writes)" ] +} + +@test "automerge: provenance reads use the job token, writes use the App token" { + _seed "[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' web-flow true 1)]" + _run_approve_step + [ "$status" -eq 0 ] + run grep -E 'api repos/owner/repo/pulls/5($| |/commits)' "${FIX_DIR}/calls.log" + [ "$status" -eq 0 ] + ! printf '%s\n' "$output" | grep -v '^TOKEN=read-token ' +} diff --git a/test/workflows/dependabot/gates.bats b/test/workflows/dependabot/gates.bats new file mode 100644 index 000000000..8bbd6b892 --- /dev/null +++ b/test/workflows/dependabot/gates.bats @@ -0,0 +1,278 @@ +#!/usr/bin/env bats +# Unit tests for the pure Dependabot merge / approval gates +# (.github/scripts/dependabot/lib/gates.sh). +# +# #1399 — the merger must not merge unless EVERY check on the head commit +# concluded success / neutral / skipped (fail closed otherwise). +# #1400 — approval / merge must not apply to commits that did not come from +# Dependabot (or the merge App's own branch updates). + +load 'helpers/setup' + +setup() { + # shellcheck source=/dev/null + . "${TT_LIB_DIR}/gates.sh" +} + +_ok_nodes() { + printf '[%s,%s,%s]' \ + "$(tt_check_run "CodeQL" COMPLETED SUCCESS)" \ + "$(tt_check_run "dev-lead / push" COMPLETED SKIPPED)" \ + "$(tt_status "CodeRabbit" SUCCESS)" +} + +# ── dependabot_checks_gate (#1399) ────────────────────────────────────────── + +@test "checks: all success / skipped / status success passes" { + run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)")" '[]' + [ "$status" -eq 0 ] + [[ "$output" == *"all 3 check(s) succeeded"* ]] +} + +@test "checks: neutral conclusion passes" { + run dependabot_checks_gate "$(tt_pr_state "[$(tt_check_run lint COMPLETED NEUTRAL)]")" '[]' + [ "$status" -eq 0 ] +} + +@test "checks: lowercase REST-style values are accepted" { + run dependabot_checks_gate "$(tt_pr_state "[$(tt_check_run lint completed success)]")" '[]' + [ "$status" -eq 0 ] +} + +@test "checks: accepts the whole GraphQL response envelope" { + run dependabot_checks_gate "{\"data\":{\"repository\":{\"pullRequest\":$(tt_pr_state "$(_ok_nodes)")}}}" '[]' + [ "$status" -eq 0 ] +} + +@test "checks: a completed FAILURE blocks (regression #1399)" { + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run 'dependency-audit / npm audit' COMPLETED FAILURE)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] + [[ "$output" == *"dependency-audit / npm audit (failure)"* ]] +} + +@test "checks: every non-success conclusion blocks" { + for c in FAILURE CANCELLED TIMED_OUT ACTION_REQUIRED STALE STARTUP_FAILURE SOMETHING_NEW; do + run dependabot_checks_gate "$(tt_pr_state "[$(tt_check_run ci COMPLETED "$c")]")" '[]' + [ "$status" -eq 1 ] || { echo "conclusion $c did not block"; return 1; } + done +} + +@test "checks: completed with a null conclusion blocks" { + run dependabot_checks_gate "$(tt_pr_state "[$(tt_check_run ci COMPLETED "")]")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: pending / queued / in-progress check runs block" { + for s in QUEUED IN_PROGRESS WAITING PENDING REQUESTED; do + run dependabot_checks_gate "$(tt_pr_state "[$(tt_check_run ci "$s" "")]")" '[]' + [ "$status" -eq 1 ] || { echo "status $s did not block"; return 1; } + done +} + +@test "checks: commit status pending / failure / error / expected blocks" { + for s in PENDING FAILURE ERROR EXPECTED; do + run dependabot_checks_gate "$(tt_pr_state "[$(tt_status ext "$s")]")" '[]' + [ "$status" -eq 1 ] || { echo "status context $s did not block"; return 1; } + done +} + +@test "checks: an unknown context type blocks" { + run dependabot_checks_gate "$(tt_pr_state '[{"__typename":"Mystery","name":"x"}]')" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: no checks reported blocks" { + run dependabot_checks_gate "$(tt_pr_state '[]')" '[]' + [ "$status" -eq 1 ] + [[ "$output" == *"no checks reported"* ]] +} + +@test "checks: a second page of checks blocks (cannot verify them all)" { + run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)" MERGEABLE true "$HEAD_SHA" "$HEAD_SHA" true)" '[]' + [ "$status" -eq 1 ] + [[ "$output" == *"cannot verify"* ]] +} + +@test "checks: a rollup for a commit other than the head blocks" { + run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)" MERGEABLE true "$HEAD_SHA" "$OTHER_SHA")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: head differing from the expected SHA blocks" { + run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)")" '[]' "$OTHER_SHA" + [ "$status" -eq 1 ] + [[ "$output" == *"head moved"* ]] +} + +@test "checks: head matching the expected SHA passes" { + run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)")" '[]' "$HEAD_SHA" + [ "$status" -eq 0 ] +} + +@test "checks: a required check that never reported blocks" { + run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)")" '["CodeQL","SonarCloud"]' + [ "$status" -eq 1 ] + [[ "$output" == *"required check(s) not reported"*"SonarCloud"* ]] +} + +@test "checks: required checks satisfied by check runs and statuses pass" { + run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)")" '["CodeQL","CodeRabbit"]' + [ "$status" -eq 0 ] +} + +@test "checks: malformed input blocks" { + run dependabot_checks_gate 'not json' '[]' + [ "$status" -eq 1 ] + run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)")" 'not json' + [ "$status" -eq 1 ] + run dependabot_checks_gate '' '[]' + [ "$status" -eq 1 ] +} + +# ── dependabot_provenance_gate (#1400) ────────────────────────────────────── + +_dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } + +@test "provenance: a single verified Dependabot commit passes" { + commits="[$(_dep_commit "$HEAD_SHA")]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" + [ "$status" -eq 0 ] +} + +@test "provenance: committer dependabot[bot] is also accepted" { + commits="[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' 'dependabot[bot]' true 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" + [ "$status" -eq 0 ] +} + +@test "provenance: Dependabot commit + the App's update-branch merge passes" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" "$APP_LOGIN_FIXTURE" web-flow true 2)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "$APP_LOGIN_FIXTURE" "$HEAD_SHA" + [ "$status" -eq 0 ] +} + +@test "provenance: paginated (slurped) commit pages are flattened" { + pages="[[$(_dep_commit "$DEP_SHA")],[$(tt_commit "$HEAD_SHA" "$APP_LOGIN_FIXTURE" web-flow true 2)]]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$pages" "$APP_LOGIN_FIXTURE" + [ "$status" -eq 0 ] +} + +@test "provenance: a later commit pushed by someone else blocks (regression #1400)" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" mallory mallory true 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "$APP_LOGIN_FIXTURE" + [ "$status" -eq 1 ] + [[ "$output" == *"not from Dependabot"*"author mallory"* ]] +} + +@test "provenance: a web-UI commit by a person (committer web-flow) blocks" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" alice web-flow true 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "$APP_LOGIN_FIXTURE" + [ "$status" -eq 1 ] +} + +@test "provenance: a Dependabot-attributed commit without a verified signature blocks" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" 'dependabot[bot]' web-flow false 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "" + [ "$status" -eq 1 ] + [[ "$output" == *"unverified"* ]] +} + +@test "provenance: a Dependabot-attributed commit committed by someone else blocks" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" 'dependabot[bot]' mallory true 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "" + [ "$status" -eq 1 ] +} + +@test "provenance: an App commit that is not a merge blocks" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" "$APP_LOGIN_FIXTURE" web-flow true 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "$APP_LOGIN_FIXTURE" + [ "$status" -eq 1 ] +} + +@test "provenance: an App merge commit blocks when no trusted login is given" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" "$APP_LOGIN_FIXTURE" web-flow true 2)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "" + [ "$status" -eq 1 ] +} + +@test "provenance: a merge commit by another bot blocks" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" 'other-app[bot]' web-flow true 2)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "$APP_LOGIN_FIXTURE" + [ "$status" -eq 1 ] +} + +@test "provenance: a commit with no linked author account blocks" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" '' web-flow true 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "" + [ "$status" -eq 1 ] +} + +@test "provenance: only App merge commits (no Dependabot commit) blocks" { + commits="[$(tt_commit "$HEAD_SHA" "$APP_LOGIN_FIXTURE" web-flow true 2)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "$APP_LOGIN_FIXTURE" + [ "$status" -eq 1 ] + [[ "$output" == *"no Dependabot commit"* ]] +} + +@test "provenance: an incomplete commit list (fewer than the PR's count) blocks" { + commits="[$(_dep_commit "$HEAD_SHA")]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 251)" "$commits" "" + [ "$status" -eq 1 ] + [[ "$output" == *"cannot verify them all"* ]] +} + +@test "provenance: head not in the commit list blocks" { + commits="[$(_dep_commit "$DEP_SHA")]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" + [ "$status" -eq 1 ] +} + +@test "provenance: head differing from the expected SHA blocks" { + commits="[$(_dep_commit "$HEAD_SHA")]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" "$OTHER_SHA" + [ "$status" -eq 1 ] + [[ "$output" == *"head moved"* ]] +} + +@test "provenance: empty or malformed input blocks" { + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 0)" '[]' "" + [ "$status" -eq 1 ] + run dependabot_provenance_gate 'nope' '[]' "" + [ "$status" -eq 1 ] + run dependabot_provenance_gate "$(tt_pr_rest)" 'nope' "" + [ "$status" -eq 1 ] +} + +@test "provenance: a long commit list (beyond the argv size limit) is evaluated" { + local i commits="" pad + pad=$(printf 'x%.0s' $(seq 1 3000)) + for i in $(seq 1 99); do + commits+="$(tt_commit "$(printf '%040d' "$i")" 'dependabot[bot]' web-flow true 1 | jq -c --arg p "$pad" '.commit.message = $p')," + done + commits="[${commits}$(_dep_commit "$HEAD_SHA")]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 100)" "$commits" "" + [ "$status" -eq 0 ] +} + +# ── dependabot_required_contexts ──────────────────────────────────────────── + +@test "required: collects unique required check names across rulesets and pages" { + rules='[[{"type":"required_status_checks","parameters":{"required_status_checks":[{"context":"CodeQL"},{"context":"SonarCloud"}]}},{"type":"pull_request","parameters":{}}],[{"type":"required_status_checks","parameters":{"required_status_checks":[{"context":"CodeQL","integration_id":1}]}}]]' + run dependabot_required_contexts "$rules" + [ "$status" -eq 0 ] + [ "$output" = '["CodeQL","SonarCloud"]' ] +} + +@test "required: no rules yields an empty list" { + run dependabot_required_contexts '[]' + [ "$status" -eq 0 ] + [ "$output" = '[]' ] +} + +@test "required: a non-array response fails" { + run dependabot_required_contexts '{"message":"Not Found"}' + [ "$status" -eq 1 ] + run dependabot_required_contexts 'garbage' + [ "$status" -eq 1 ] +} diff --git a/test/workflows/dependabot/helpers/setup.bash b/test/workflows/dependabot/helpers/setup.bash new file mode 100644 index 000000000..b106bf72d --- /dev/null +++ b/test/workflows/dependabot/helpers/setup.bash @@ -0,0 +1,197 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2034 # fixture constants are read by the .bats files +# Common helpers for the Dependabot merge-gate bats suite (#1399, #1400). + +TT_REPO_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/../../../.." && pwd)" +export TT_REPO_ROOT +TT_LIB_DIR="${TT_REPO_ROOT}/.github/scripts/dependabot/lib" +export TT_LIB_DIR + +# Fixture constants, used by the .bats files that load this helper. +HEAD_SHA="1111111111111111111111111111111111111111" +DEP_SHA="2222222222222222222222222222222222222222" +OTHER_SHA="3333333333333333333333333333333333333333" +APP_SLUG_FIXTURE="my-merge-app" +APP_LOGIN_FIXTURE="my-merge-app[bot]" + +# tt_setup — scratch dir, fixture dir and a fixture-driven `gh` stub on PATH. +tt_setup() { + TT_TMP="$(mktemp -d)" + FIX_DIR="${TT_TMP}/fixtures" + TT_BIN="${TT_TMP}/bin" + TT_WORK="${TT_TMP}/work" + mkdir -p "$FIX_DIR" "$TT_BIN" "$TT_WORK" + ln -s "$TT_REPO_ROOT" "${TT_WORK}/.dependabot-tooling" + export FIX_DIR TT_TMP TT_WORK + _tt_install_gh_stub + PATH="${TT_BIN}:${PATH}" + export PATH +} + +tt_teardown() { + if [ -n "${TT_TMP:-}" ] && [ -d "${TT_TMP}" ]; then + rm -rf "${TT_TMP}" + fi +} + +_tt_key() { printf '%s' "$1" | tr -c 'A-Za-z0-9' '_'; } + +# tt_fixture ENDPOINT BODY [RC] — serve BODY for `gh api ENDPOINT`. +tt_fixture() { + local key + key="$(_tt_key "$1")" + printf '%s\n' "$2" > "${FIX_DIR}/${key}.json" + rm -f "${FIX_DIR}/${key}.rc" + if [ -n "${3:-}" ]; then printf '%s\n' "$3" > "${FIX_DIR}/${key}.rc"; fi +} + +# ── `gh` stub ──────────────────────────────────────────────────────────────── +# Every call is appended to calls.log as "TOKEN= ". +# gh api graphql … -F number=N → graphql_N.json +# gh api ENDPOINT (GET) → .json (404 when absent); +# `--paginate --slurp` wraps it in a page array +# gh api ENDPOINT -X PUT|POST → recorded in writes.log with its fields; +# succeeds unless .rc exists +# gh pr list … --jq E → pr_list.json through jq +# gh pr view N --json … [--jq E] → pr_view_N.json (through jq when given) +# gh pr merge / gh pr review → recorded in writes.log; `pr_merge.rc` fails +_tt_install_gh_stub() { + cat > "${TT_BIN}/gh" <<'STUB' +#!/usr/bin/env bash +set -uo pipefail +printf 'TOKEN=%s %s\n' "${GH_TOKEN:-}" "$*" >> "${FIX_DIR}/calls.log" +_key() { printf '%s' "$1" | tr -c 'A-Za-z0-9' '_'; } + +_emit() { # BODY JQ + if [ -n "$2" ]; then printf '%s' "$1" | jq -r "$2"; else printf '%s\n' "$1"; fi +} + +if [ "$1" = "api" ]; then + shift + endpoint="" jq_expr="" number="" method="GET" paginate=false slurp=false fields="" + while [ "$#" -gt 0 ]; do + case "$1" in + --jq|-q) jq_expr="$2"; shift 2 ;; + -X|--method) method="$2"; shift 2 ;; + -H) shift 2 ;; + --paginate) paginate=true; shift ;; + --slurp) slurp=true; shift ;; + --silent) shift ;; + -f|-F) + case "$2" in number=*) number="${2#number=}" ;; query=*) ;; *) fields="${fields} $2" ;; esac + shift 2 ;; + *) endpoint="$1"; shift ;; + esac + done + if [ "$endpoint" = "graphql" ]; then + key="graphql_${number}" + else + key="$(_key "$endpoint")" + fi + if [ "$method" != "GET" ]; then + printf 'TOKEN=%s %s %s%s\n' "${GH_TOKEN:-}" "$method" "$endpoint" "$fields" >> "${FIX_DIR}/writes.log" + if [ -f "${FIX_DIR}/${key}.rc" ]; then exit "$(cat "${FIX_DIR}/${key}.rc")"; fi + exit 0 + fi + if [ ! -f "${FIX_DIR}/${key}.json" ]; then + echo '{"message":"Not Found","status":"404"}' + echo "gh: Not Found (HTTP 404)" >&2 + exit 1 + fi + body="$(cat "${FIX_DIR}/${key}.json")" + if [ -f "${FIX_DIR}/${key}.rc" ]; then + printf '%s\n' "$body"; exit "$(cat "${FIX_DIR}/${key}.rc")" + fi + if [ "$paginate" = true ] && [ "$slurp" = true ]; then body="[${body}]"; fi + _emit "$body" "$jq_expr" + exit $? +fi + +if [ "$1" = "pr" ] && [ "$2" = "list" ]; then + shift 2; jq_expr="" + while [ "$#" -gt 0 ]; do case "$1" in --jq) jq_expr="$2"; shift 2 ;; *) shift ;; esac; done + _emit "$(cat "${FIX_DIR}/pr_list.json" 2>/dev/null || echo '[]')" "$jq_expr" + exit $? +fi + +if [ "$1" = "pr" ] && [ "$2" = "view" ]; then + pr="$3"; shift 3; jq_expr="" + while [ "$#" -gt 0 ]; do case "$1" in --jq) jq_expr="$2"; shift 2 ;; *) shift ;; esac; done + [ -f "${FIX_DIR}/pr_view_${pr}.json" ] || exit 1 + _emit "$(cat "${FIX_DIR}/pr_view_${pr}.json")" "$jq_expr" + exit $? +fi + +if [ "$1" = "pr" ] && { [ "$2" = "merge" ] || [ "$2" = "review" ]; }; then + printf 'TOKEN=%s %s\n' "${GH_TOKEN:-}" "$*" >> "${FIX_DIR}/writes.log" + if [ "$2" = "merge" ] && [ -f "${FIX_DIR}/pr_merge.rc" ]; then exit "$(cat "${FIX_DIR}/pr_merge.rc")"; fi + exit 0 +fi + +exit 0 +STUB + chmod +x "${TT_BIN}/gh" +} + +# ── fixture builders ───────────────────────────────────────────────────────── + +# tt_check_run NAME STATUS CONCLUSION — one GraphQL CheckRun node. +tt_check_run() { + jq -cn --arg n "$1" --arg s "$2" --arg c "$3" \ + '{__typename:"CheckRun",name:$n,status:$s,conclusion:(if $c == "" then null else $c end)}' +} + +# tt_status NAME STATE — one GraphQL StatusContext node. +tt_status() { + jq -cn --arg n "$1" --arg s "$2" '{__typename:"StatusContext",context:$n,state:$s}' +} + +# tt_pr_state NODES_JSON [MERGEABLE] [AUTO_MERGE true|false] [HEAD] [ROLLUP_SHA] [HAS_NEXT] +# A GraphQL pullRequest object for the check gate. +tt_pr_state() { + jq -cn --argjson nodes "$1" --arg m "${2:-MERGEABLE}" --arg am "${3:-true}" \ + --arg head "${4:-$HEAD_SHA}" --arg oid "${5:-${4:-$HEAD_SHA}}" --arg next "${6:-false}" ' + {headRefOid:$head, baseRefName:"main", mergeable:$m, + autoMergeRequest:(if $am == "true" then {enabledAt:"2026-01-01T00:00:00Z"} else null end), + commits:{nodes:[{commit:{oid:$oid,statusCheckRollup:{contexts:{ + pageInfo:{hasNextPage:($next == "true")}, nodes:$nodes}}}}]}}' +} + +# tt_commit SHA AUTHOR COMMITTER [VERIFIED true|false] [PARENTS] +tt_commit() { + jq -cn --arg sha "$1" --arg a "$2" --arg c "$3" --arg v "${4:-true}" --argjson p "${5:-1}" ' + {sha:$sha, + author:(if $a == "" then null else {login:$a} end), + committer:(if $c == "" then null else {login:$c} end), + parents:[range(0;$p) | {sha:"p\(.)"}], + commit:{verification:{verified:($v == "true"), reason:(if $v == "true" then "valid" else "unsigned" end)}}}' +} + +# tt_pr_rest [HEAD] [COMMIT_COUNT] — REST pull request object. +tt_pr_rest() { + jq -cn --arg head "${1:-$HEAD_SHA}" --argjson n "${2:-1}" '{number:5,head:{sha:$head},base:{ref:"main"},commits:$n}' +} + +# tt_seed_pr STATE_JSON COMMITS_JSON [REQUIRED_RULES_JSON] +# PR #5 (head branch dependabot/npm/foo, up to date with main) with the given +# GraphQL state, REST commits and base-branch rules. Also writes a +# `gh pr view` fixture mirroring the state (used by the pre-fix workflow). +tt_seed_pr() { + local state="$1" commits="$2" rules="${3:-[]}" head count + head=$(jq -r '.headRefOid' <<<"$state") + count=$(jq 'length' <<<"$commits") + printf '%s\n' '[{"number":5,"headRefName":"dependabot/npm/foo"}]' > "${FIX_DIR}/pr_list.json" + tt_fixture "repos/owner/repo/compare/main...dependabot/npm/foo" '{"behind_by":0}' + printf '%s\n' "{\"data\":{\"repository\":{\"pullRequest\":${state}}}}" > "${FIX_DIR}/graphql_5.json" + tt_fixture "repos/owner/repo/rules/branches/main?per_page=100" "$rules" + tt_fixture "repos/owner/repo/pulls/5" "$(tt_pr_rest "$head" "$count")" + tt_fixture "repos/owner/repo/pulls/5/commits?per_page=100" "$commits" + jq -c '{autoMergeRequest, mergeable, + statusCheckRollup: [.commits.nodes[0].commit.statusCheckRollup.contexts.nodes[]]}' \ + <<<"$state" > "${FIX_DIR}/pr_view_5.json" +} + +# tt_extract_run WORKFLOW JOB STEP_NAME — print a step's run block. +tt_extract_run() { + yq -r ".jobs.\"$2\".steps[] | select(.name == \"$3\") | .run" "$1" +} diff --git a/test/workflows/dependabot/rebase-merge-gate.bats b/test/workflows/dependabot/rebase-merge-gate.bats new file mode 100644 index 000000000..e0ee7ee48 --- /dev/null +++ b/test/workflows/dependabot/rebase-merge-gate.bats @@ -0,0 +1,174 @@ +#!/usr/bin/env bats +# End-to-end tests for the merge step of dependabot-rebase-reusable.yml +# (issues #1399, #1400). +# +# The reusable merges Dependabot PRs with a GitHub App that bypasses the +# branch rulesets, so its own gate is the only thing standing between a PR and +# the default branch. It used to merge once checks were merely COMPLETE (a +# failed check passed) and never looked at who authored the commits. These +# tests run the workflow's real "Update and merge Dependabot PRs" run-block +# against a fixture-driven `gh` stub and assert on the merge call it makes. + +load 'helpers/setup' + +REUSABLE="${TT_REPO_ROOT}/.github/workflows/dependabot-rebase-reusable.yml" + +setup() { + tt_setup + RUN_SCRIPT="${TT_TMP}/run.sh" + tt_extract_run "$REUSABLE" update-and-merge "Update and merge Dependabot PRs" > "$RUN_SCRIPT" +} + +teardown() { + tt_teardown +} + +_run_merge_step() { + cd "$TT_WORK" || return 1 + GH_TOKEN="read-token" APP_TOKEN="app-token" APP_SLUG="$APP_SLUG_FIXTURE" \ + REPO="owner/repo" run bash -eo pipefail "$RUN_SCRIPT" +} + +_green_nodes() { + printf '[%s,%s,%s]' \ + "$(tt_check_run CodeQL COMPLETED SUCCESS)" \ + "$(tt_check_run 'dev-lead / push' COMPLETED SKIPPED)" \ + "$(tt_status CodeRabbit SUCCESS)" +} + +_dependabot_only() { printf '[%s]' "$(tt_commit "$HEAD_SHA" 'dependabot[bot]' web-flow true 1)"; } + +_rules_requiring() { # NAME... + local ctx + ctx=$(printf '%s\n' "$@" | jq -R '{context: .}' | jq -cs .) + printf '[{"type":"required_status_checks","parameters":{"strict_required_status_checks_policy":false,"required_status_checks":%s}}]' "$ctx" +} + +_merge_calls() { grep -F '/pulls/5/merge' "${FIX_DIR}/writes.log" 2>/dev/null || true; } + +@test "rebase: run-block exists and sources the gate libraries" { + [ -s "$RUN_SCRIPT" ] + grep -F '.dependabot-tooling/.github/scripts/dependabot/lib/gates.sh' "$RUN_SCRIPT" + grep -F '.dependabot-tooling/.github/scripts/dependabot/lib/gh-io.sh' "$RUN_SCRIPT" +} + +@test "rebase: the job checks out the gate tooling at the reusable's own commit" { + run yq -r '.jobs.update-and-merge.steps[] | select(.name == "Checkout Dependabot gate tooling") | .with.ref' "$REUSABLE" + [ "$status" -eq 0 ] + [ "$output" = '${{ job.workflow_sha }}' ] + run yq -r '.jobs.update-and-merge.steps[] | select(.name == "Checkout Dependabot gate tooling") | .with.path' "$REUSABLE" + [ "$output" = ".dependabot-tooling" ] +} + +@test "rebase: all checks green + Dependabot-only commits merges, bound to the head SHA" { + tt_seed_pr "$(tt_pr_state "$(_green_nodes)")" "$(_dependabot_only)" "$(_rules_requiring CodeQL)" + _run_merge_step + [ "$status" -eq 0 ] + [[ "$output" == *"Merged PR #5"* ]] + run _merge_calls + [ "$(printf '%s\n' "$output" | grep -c .)" -eq 1 ] + [[ "$output" == "TOKEN=app-token PUT repos/owner/repo/pulls/5/merge"* ]] + [[ "$output" == *"merge_method=squash"* ]] + [[ "$output" == *"sha=${HEAD_SHA}"* ]] +} + +@test "rebase: a completed but FAILED check is not merged (regression #1399)" { + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run 'dependency-audit / npm audit' COMPLETED FAILURE)]" + tt_seed_pr "$(tt_pr_state "$nodes")" "$(_dependabot_only)" + _run_merge_step + [ "$status" -eq 0 ] + [[ "$output" == *"Skipping"*"dependency-audit / npm audit (failure)"* ]] + run _merge_calls + [ -z "$output" ] +} + +@test "rebase: cancelled / timed-out / action-required / stale checks are not merged" { + for c in CANCELLED TIMED_OUT ACTION_REQUIRED STALE STARTUP_FAILURE; do + rm -f "${FIX_DIR}/writes.log" + tt_seed_pr "$(tt_pr_state "[$(tt_check_run ci COMPLETED "$c")]")" "$(_dependabot_only)" + _run_merge_step + [ "$status" -eq 0 ] + [ -z "$(_merge_calls)" ] || { echo "merged despite $c"; return 1; } + done +} + +@test "rebase: a failing commit status is not merged" { + tt_seed_pr "$(tt_pr_state "[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_status ext FAILURE)]")" "$(_dependabot_only)" + _run_merge_step + [ "$status" -eq 0 ] + [ -z "$(_merge_calls)" ] +} + +@test "rebase: a pending check is not merged" { + tt_seed_pr "$(tt_pr_state "[$(tt_check_run ci IN_PROGRESS "")]")" "$(_dependabot_only)" + _run_merge_step + [ "$status" -eq 0 ] + [ -z "$(_merge_calls)" ] +} + +@test "rebase: a required check that never reported is not merged" { + tt_seed_pr "$(tt_pr_state "$(_green_nodes)")" "$(_dependabot_only)" "$(_rules_requiring CodeQL SonarCloud)" + _run_merge_step + [ "$status" -eq 0 ] + [[ "$output" == *"required check(s) not reported"*"SonarCloud"* ]] + [ -z "$(_merge_calls)" ] +} + +@test "rebase: unreadable base-branch rules fail closed" { + tt_seed_pr "$(tt_pr_state "$(_green_nodes)")" "$(_dependabot_only)" + tt_fixture "repos/owner/repo/rules/branches/main?per_page=100" '{"message":"Server Error","status":"500"}' 1 + _run_merge_step + [ "$status" -eq 0 ] + [[ "$output" == *"could not read the required checks"* ]] + [ -z "$(_merge_calls)" ] +} + +@test "rebase: a non-Dependabot commit on the branch is not merged (#1400)" { + commits="[$(tt_commit "$DEP_SHA" 'dependabot[bot]' web-flow true 1),$(tt_commit "$HEAD_SHA" mallory mallory true 1)]" + tt_seed_pr "$(tt_pr_state "$(_green_nodes)")" "$commits" + _run_merge_step + [ "$status" -eq 0 ] + [[ "$output" == *"not from Dependabot"* ]] + [ -z "$(_merge_calls)" ] +} + +@test "rebase: the App's own update-branch merge commit does not block" { + commits="[$(tt_commit "$DEP_SHA" 'dependabot[bot]' web-flow true 1),$(tt_commit "$HEAD_SHA" "$APP_LOGIN_FIXTURE" web-flow true 2)]" + tt_seed_pr "$(tt_pr_state "$(_green_nodes)")" "$commits" + _run_merge_step + [ "$status" -eq 0 ] + [[ "$(_merge_calls)" == *"sha=${HEAD_SHA}"* ]] +} + +@test "rebase: auto-merge not requested is not merged" { + tt_seed_pr "$(tt_pr_state "$(_green_nodes)" MERGEABLE false)" "$(_dependabot_only)" + _run_merge_step + [ "$status" -eq 0 ] + [[ "$output" == *"auto-merge not enabled"* ]] + [ -z "$(_merge_calls)" ] +} + +@test "rebase: a conflicting PR is not merged" { + tt_seed_pr "$(tt_pr_state "$(_green_nodes)" CONFLICTING)" "$(_dependabot_only)" + _run_merge_step + [ "$status" -eq 0 ] + [[ "$output" == *"not mergeable (CONFLICTING)"* ]] + [ -z "$(_merge_calls)" ] +} + +@test "rebase: a rejected merge (head moved) is logged and the run continues" { + tt_seed_pr "$(tt_pr_state "$(_green_nodes)")" "$(_dependabot_only)" + tt_fixture "repos/owner/repo/pulls/5/merge" '{"message":"Head branch was modified"}' 1 + _run_merge_step + [ "$status" -eq 0 ] + [[ "$output" == *"Warning: failed to merge PR #5"* ]] +} + +@test "rebase: gate reads use the job token, only the merge uses the App token" { + tt_seed_pr "$(tt_pr_state "$(_green_nodes)")" "$(_dependabot_only)" + _run_merge_step + [ "$status" -eq 0 ] + run grep -E 'graphql|/rules/|/commits' "${FIX_DIR}/calls.log" + [ "$status" -eq 0 ] + ! printf '%s\n' "$output" | grep -v '^TOKEN=read-token ' +} From 217817934140c9247b6fb5459c947956cb0d3864 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:39:44 -0600 Subject: [PATCH 02/17] fix(security): withdraw Dependabot approval and auto-merge when the head moves (#1400) An approval POST accepts an older commit, the App's approval escapes the stale-review rules because it is a bypass actor, and a writer's push does not disable auto-merge. A push landing during the approval therefore left approval and auto-merge on an unverified head, and --match-head-commit failing left the approval behind. Re-read the PR after both writes and withdraw the state when the head moved, fails the gate or cannot be read. Revocation now reads back the result and fails the run if it could not complete, instead of being best effort. Also fix the stale test reference in the gh-io.sh header. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 13 +- .github/scripts/dependabot/lib/gh-io.sh | 116 ++++++++++++++---- .../dependabot-automerge-reusable.yml | 12 +- standards/dependabot-policy.md | 10 +- .../dependabot/automerge-provenance.bats | 95 +++++++++++++- test/workflows/dependabot/helpers/setup.bash | 11 +- 6 files changed, 217 insertions(+), 40 deletions(-) diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index f3a579278..7db392f52 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -32,5 +32,14 @@ failure fails closed. | `dependabot_merge_gate REPO PR APP_LOGIN` | Auto-merge requested + `MERGEABLE` + checks gate + provenance gate, all bound to one head SHA. Sets `DEPENDABOT_GATE_SHA` / `DEPENDABOT_GATE_REASON`. Used by the merger before every direct merge. | | `dependabot_provenance_check REPO PR APP_LOGIN [SHA]` | Provenance gate on the live PR. Returns `0` pass, `1` block (sets `DEPENDABOT_GATE_STALE=true` when the head moved past `SHA`), `2` state unreadable. | | `dependabot_merge_pr REPO PR SHA` | Squash-merges with `sha=SHA`, so GitHub rejects the merge if the head moved after the gate ran. | -| `dependabot_approve_and_automerge REPO PR SHA` | Approves with `commit_id=SHA` and enables auto-merge with `--match-head-commit SHA`. | -| `dependabot_revoke_automation REPO PR APP_LOGIN REASON` | Best effort: disables auto-merge and dismisses `APP_LOGIN`'s approvals. | +| `dependabot_approve_and_automerge REPO PR SHA APP_LOGIN` | Approves with `commit_id=SHA`, enables auto-merge with `--match-head-commit SHA`, then re-reads the PR. If the head moved to a verified commit, it dismisses the approval of `SHA`; if the new head fails the gate or the PR cannot be read, it revokes everything. | +| `dependabot_dismiss_app_approvals REPO PR APP_LOGIN REASON [KEEP_SHA]` | Dismisses `APP_LOGIN`'s approvals except those of `KEEP_SHA`. Returns `1` if one may have survived. | +| `dependabot_revoke_automation REPO PR APP_LOGIN REASON` | Disables auto-merge and dismisses all of `APP_LOGIN`'s approvals. Returns `1` unless the PR reads back with auto-merge off and every dismissal succeeded, and the workflow then fails. | + +**What this cannot make atomic.** Native auto-merge has no head-bound form that +survives a push: a writer's push does not disable it, and `--match-head-commit` +applies only when it is enabled. The re-read and the `synchronize` run together +withdraw that state on every interleaving, but not in the same instant, so a +foreign push has a short window before its run revokes. The merger is not +exposed to it: it re-runs both gates and merges with `sha=`. If the `synchronize` +run does not run at all, the state stays until something re-evaluates the PR. diff --git a/.github/scripts/dependabot/lib/gh-io.sh b/.github/scripts/dependabot/lib/gh-io.sh index 351734922..a16ed5dd1 100644 --- a/.github/scripts/dependabot/lib/gh-io.sh +++ b/.github/scripts/dependabot/lib/gh-io.sh @@ -6,8 +6,8 @@ # decision itself — every verdict comes from the pure gates in lib/gates.sh, # which must be sourced alongside. Any read failure fails closed (skip). # -# Tested by test/workflows/dependabot/gh-io.bats and the end-to-end suites in -# test/workflows/dependabot/. +# Tested end to end by test/workflows/dependabot/automerge-provenance.bats and +# test/workflows/dependabot/rebase-merge-gate.bats. # Contract: see .github/scripts/dependabot/README.md # dependabot_fetch_pr_state REPO PR_NUMBER @@ -139,34 +139,73 @@ dependabot_merge_pr() { -f merge_method=squash -f sha="$sha" --silent } -# dependabot_approve_and_automerge REPO PR_NUMBER SHA -# Approves the PR at commit SHA (the review is bound to that commit) and -# enables squash auto-merge only while the head is still SHA. +# dependabot_approve_and_automerge REPO PR_NUMBER SHA APP_LOGIN +# Approves the PR at commit SHA and enables squash auto-merge only while the +# head is still SHA, then re-reads the PR, because neither write is atomic +# with the head: GitHub accepts a review for an older commit, the App's +# approval is not subject to the stale-review rules (it is a bypass actor), +# and a push by a writer does not disable auto-merge that is already on. +# +# The re-read closes the race with the run for a newer push. That push came +# either before this re-read (seen here) or after it, in which case its own +# run starts after these writes and revokes them. So whichever way the runs +# interleave, an unverified head does not keep this state. +# +# Returns 0 when the state is settled: SHA is still the head and auto-merge +# is on, or the head moved and the state was withdrawn. Returns non-zero when +# auto-merge was refused at SHA, the PR could not be re-read (the state is +# withdrawn first), or a withdrawal could not be completed. dependabot_approve_and_automerge() { - local repo="$1" pr="$2" sha="$3" + local repo="$1" pr="$2" sha="$3" app_login="${4:-}" enable_rc=0 rc=0 [[ -n "$sha" ]] || return 1 gh api "repos/${repo}/pulls/${pr}/reviews" -X POST \ -f event=APPROVE -f commit_id="$sha" --silent || return 1 - gh pr merge "$pr" --repo "$repo" --auto --squash --match-head-commit "$sha" + gh pr merge "$pr" --repo "$repo" --auto --squash --match-head-commit "$sha" || enable_rc=$? + + GH_TOKEN="${READ_TOKEN:-${GH_TOKEN:-}}" dependabot_provenance_check "$repo" "$pr" "$app_login" || rc=$? + case "$rc" in + 0) + if [[ "$DEPENDABOT_GATE_SHA" == "$sha" ]]; then + return "$enable_rc" + fi + # A newer verified head: its own run owns approval and auto-merge, so + # withdraw only what this run approved for the older commit. + echo "Head moved to ${DEPENDABOT_GATE_SHA:0:7} during approval; withdrawing the approval of ${sha:0:7}" + dependabot_dismiss_app_approvals "$repo" "$pr" "$app_login" \ + "the head moved past ${sha:0:7}" "$DEPENDABOT_GATE_SHA" + ;; + 1) + echo "::warning::Withdrawing automation on PR #${pr} — ${DEPENDABOT_GATE_REASON}" + dependabot_revoke_automation "$repo" "$pr" "$app_login" "$DEPENDABOT_GATE_REASON" + ;; + *) + echo "::error::Could not re-verify PR #${pr} after approving — ${DEPENDABOT_GATE_REASON}; withdrawing" + dependabot_revoke_automation "$repo" "$pr" "$app_login" "$DEPENDABOT_GATE_REASON" || true + return 1 + ;; + esac } -# dependabot_revoke_automation REPO PR_NUMBER APP_LOGIN REASON -# Best effort, always returns 0: disables auto-merge and dismisses approvals -# APP_LOGIN gave earlier, so a revision that failed the provenance gate is not -# merged on the strength of an approval of an earlier revision. -dependabot_revoke_automation() { - local repo="$1" pr="$2" app_login="$3" reason="$4" reviews id +# dependabot_dismiss_app_approvals REPO PR_NUMBER APP_LOGIN REASON [KEEP_SHA] +# Dismisses APP_LOGIN's approvals, except those of commit KEEP_SHA. Returns 1 +# when an approval may have survived (the login is unknown, the reviews could +# not be listed, or a dismissal failed). +dependabot_dismiss_app_approvals() { + local repo="$1" pr="$2" app_login="$3" reason="$4" keep="${5:-}" reviews ids id failed=0 - if ! gh pr merge "$pr" --repo "$repo" --disable-auto >/dev/null 2>&1; then - echo " auto-merge was not enabled (or could not be disabled)" - else - echo " Disabled auto-merge" + if [[ -z "$app_login" ]]; then + echo "::error::Cannot identify automated approvals on PR #${pr}: no App login" + return 1 fi - - [[ -n "$app_login" ]] || return 0 if ! reviews=$(gh api "repos/${repo}/pulls/${pr}/reviews?per_page=100" --paginate --slurp 2>/dev/null); then - echo "::warning::Could not list reviews on PR #${pr} to dismiss earlier automated approvals" - return 0 + echo "::error::Could not list reviews on PR #${pr} to dismiss automated approvals" + return 1 + fi + if ! ids=$(jq -r --arg login "$app_login" --arg keep "$keep" \ + 'flatten[] | select(.user.login == $login and .state == "APPROVED" and .commit_id != $keep) | .id' \ + <<<"$reviews" 2>/dev/null); then + echo "::error::Could not parse reviews on PR #${pr} to dismiss automated approvals" + return 1 fi while IFS= read -r id; do [[ -n "$id" ]] || continue @@ -174,10 +213,35 @@ dependabot_revoke_automation() { -f event=DISMISS -f message="Automated approval withdrawn: ${reason}" --silent; then echo " Dismissed automated approval ${id}" else - echo "::warning::Could not dismiss automated approval ${id} on PR #${pr}" + echo "::error::Could not dismiss automated approval ${id} on PR #${pr}" + failed=1 fi - done < <(jq -r --arg login "$app_login" \ - 'flatten[] | select(.user.login == $login and .state == "APPROVED") | .id' \ - <<<"$reviews" 2>/dev/null) - return 0 + done <<<"$ids" + return "$failed" +} + +# dependabot_revoke_automation REPO PR_NUMBER APP_LOGIN REASON +# Disables auto-merge and dismisses every approval APP_LOGIN gave, so a +# revision that failed the provenance gate is not merged on the strength of an +# approval of an earlier one. Attempts both, then returns 1 unless the PR reads +# back with auto-merge off and no approval failed to dismiss, so a withdrawal +# that did not complete fails the run instead of passing silently. +dependabot_revoke_automation() { + local repo="$1" pr="$2" app_login="$3" reason="$4" pr_json failed=0 + + # `--disable-auto` also fails when auto-merge was never on, so the result is + # judged from the PR below rather than from its exit status. + if gh pr merge "$pr" --repo "$repo" --disable-auto >/dev/null 2>&1; then + echo " Disabled auto-merge" + fi + if ! pr_json=$(GH_TOKEN="${READ_TOKEN:-${GH_TOKEN:-}}" dependabot_fetch_pr "$repo" "$pr"); then + echo "::error::Could not confirm auto-merge is off on PR #${pr}" + failed=1 + elif [[ "$(jq -r '.auto_merge != null' <<<"$pr_json" 2>/dev/null)" != "false" ]]; then + echo "::error::auto-merge is still enabled on PR #${pr}" + failed=1 + fi + + dependabot_dismiss_app_approvals "$repo" "$pr" "$app_login" "$reason" || failed=1 + return "$failed" } diff --git a/.github/workflows/dependabot-automerge-reusable.yml b/.github/workflows/dependabot-automerge-reusable.yml index 610510a1a..8f776b1c5 100644 --- a/.github/workflows/dependabot-automerge-reusable.yml +++ b/.github/workflows/dependabot-automerge-reusable.yml @@ -19,8 +19,9 @@ # Dependabot commit, so before approving, every commit on the PR must be from # Dependabot (or a branch update made by this App via update-branch) and the # head must still be the commit this event was for. The approval and the -# auto-merge request are bound to that head SHA. If a later commit fails the -# gate, auto-merge is disabled and this App's earlier approvals are dismissed. +# auto-merge request are bound to that head SHA, and the PR is re-read after +# both writes. If a later commit fails the gate, auto-merge is disabled and this +# App's approvals are dismissed; a withdrawal that cannot complete fails the run. # Decision logic lives in .github/scripts/dependabot/lib/ (bats-tested). # # Required org/repo secrets (passed via `secrets: inherit` from caller): @@ -134,12 +135,15 @@ jobs: # Commits on the PR that did not come from Dependabot: withdraw any # earlier automated approval / auto-merge and leave it to a human. echo "::warning::Not approving PR #$PR_NUMBER — $DEPENDABOT_GATE_REASON" - dependabot_revoke_automation "$REPO" "$PR_NUMBER" "$APP_LOGIN" "$DEPENDABOT_GATE_REASON" + if ! dependabot_revoke_automation "$REPO" "$PR_NUMBER" "$APP_LOGIN" "$DEPENDABOT_GATE_REASON"; then + echo "::error::Could not fully withdraw automation on PR #$PR_NUMBER — a human must check it" + exit 1 + fi exit 0 fi echo "Provenance gate passed ($DEPENDABOT_GATE_REASON) — approving $DEPENDABOT_GATE_SHA" - dependabot_approve_and_automerge "$REPO" "$PR_NUMBER" "$DEPENDABOT_GATE_SHA" + dependabot_approve_and_automerge "$REPO" "$PR_NUMBER" "$DEPENDABOT_GATE_SHA" "$APP_LOGIN" env: REPO: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }} diff --git a/standards/dependabot-policy.md b/standards/dependabot-policy.md index 0a94187d4..fd0fea101 100644 --- a/standards/dependabot-policy.md +++ b/standards/dependabot-policy.md @@ -172,10 +172,12 @@ Behavior: before approving, every commit on the PR must be from Dependabot (or a branch update the App made via `update-branch`) and the head must still be the commit the event was for. The approval (`commit_id`) and the auto-merge request - (`--match-head-commit`) are bound to that head SHA. If any other commit lands - on the branch, the workflow does not approve, disables auto-merge and dismisses - its earlier approval — the PR then needs a human review. To hand it back to - automation, have Dependabot recreate it (`@dependabot recreate`). + (`--match-head-commit`) are bound to that head SHA, and the workflow re-reads + the PR after both writes. If any other commit lands on the branch, the workflow + does not approve, disables auto-merge and dismisses its earlier approval — the + PR then needs a human review. A withdrawal that cannot be completed fails the + run. To hand it back to automation, have Dependabot recreate it + (`@dependabot recreate`). ## Update and Merge Behind PRs Workflow diff --git a/test/workflows/dependabot/automerge-provenance.bats b/test/workflows/dependabot/automerge-provenance.bats index 8d6c10388..de4d71a8f 100644 --- a/test/workflows/dependabot/automerge-provenance.bats +++ b/test/workflows/dependabot/automerge-provenance.bats @@ -35,6 +35,26 @@ _writes() { cat "${FIX_DIR}/writes.log" 2>/dev/null || true; } _seed() { # COMMITS_JSON tt_seed_pr "$(tt_pr_state "[$(tt_check_run ci COMPLETED SUCCESS)]")" "$1" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" '[]' +} + +_dep_commit() { tt_commit "$HEAD_SHA" 'dependabot[bot]' web-flow true 1; } + +# _push_during_approval HEAD COMMITS_JSON — the head moves to HEAD once the +# approval POST lands, so the run's post-write read sees the new revision. +_push_during_approval() { + printf '%s\n' "$(tt_pr_rest "$1" "$(jq 'length' <<<"$2")")" > "${FIX_DIR}/moved_pr.json" + printf '%s\n' "$2" > "${FIX_DIR}/moved_commits.json" + cat > "${FIX_DIR}/on_write" <<'HOOK' +#!/usr/bin/env bash +cp "${FIX_DIR}/moved_pr.json" "${FIX_DIR}/repos_owner_repo_pulls_5.json" +cp "${FIX_DIR}/moved_commits.json" "${FIX_DIR}/repos_owner_repo_pulls_5_commits_per_page_100.json" +HOOK + chmod +x "${FIX_DIR}/on_write" +} + +_app_approval_of() { # ID COMMIT_ID + printf '{"id":%s,"state":"APPROVED","commit_id":"%s","user":{"login":"%s"}}' "$1" "$2" "$APP_LOGIN_FIXTURE" } _foreign_commits() { @@ -96,7 +116,7 @@ _foreign_commits() { [[ "$output" != *"reviews/13/dismissals"* ]] } -@test "automerge: revocation is best effort (failures do not fail the step)" { +@test "automerge: --disable-auto failing on a PR with no auto-merge is not an error" { _seed "$(_foreign_commits)" printf '1\n' > "${FIX_DIR}/pr_merge.rc" _run_approve_step @@ -104,6 +124,79 @@ _foreign_commits() { [[ "$(_writes)" != *"event=APPROVE"* ]] } +@test "automerge: revocation that leaves auto-merge enabled fails the step" { + _seed "$(_foreign_commits)" + tt_fixture "repos/owner/repo/pulls/5" "$(tt_pr_rest "$HEAD_SHA" 2 true)" + printf '1\n' > "${FIX_DIR}/pr_merge.rc" + _run_approve_step + [ "$status" -ne 0 ] + [[ "$output" == *"auto-merge is still enabled"* ]] +} + +@test "automerge: revocation that cannot list or dismiss approvals fails the step" { + _seed "$(_foreign_commits)" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" '{"message":"Server Error"}' 1 + _run_approve_step + [ "$status" -ne 0 ] + + _seed "$(_foreign_commits)" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" "[$(_app_approval_of 11 "$DEP_SHA")]" + tt_fixture "repos/owner/repo/pulls/5/reviews/11/dismissals" '{}' 1 + _run_approve_step + [ "$status" -ne 0 ] +} + +@test "automerge: a foreign push landing during approval revokes the approval and auto-merge" { + _seed "[$(_dep_commit)]" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" "[$(_app_approval_of 11 "$HEAD_SHA")]" + _push_during_approval "$OTHER_SHA" "[$(_dep_commit),$(tt_commit "$OTHER_SHA" mallory mallory true 1)]" + _run_approve_step + [ "$status" -eq 0 ] + [[ "$output" == *"not from Dependabot"* ]] + run _writes + [[ "$output" == *"event=APPROVE"*"commit_id=${HEAD_SHA}"* ]] + [[ "$output" == *"pr merge 5 --repo owner/repo --disable-auto"* ]] + [[ "$output" == *"PUT repos/owner/repo/pulls/5/reviews/11/dismissals"* ]] +} + +@test "automerge: a verified Dependabot push during approval keeps auto-merge and drops only the stale approval" { + _seed "[$(_dep_commit)]" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" \ + "[$(_app_approval_of 11 "$HEAD_SHA"),$(_app_approval_of 12 "$OTHER_SHA")]" + _push_during_approval "$OTHER_SHA" "[$(tt_commit "$OTHER_SHA" 'dependabot[bot]' web-flow true 1)]" + printf '1\n' > "${FIX_DIR}/pr_merge.rc" # --match-head-commit refuses the stale head + _run_approve_step + [ "$status" -eq 0 ] + run _writes + [[ "$output" != *"--disable-auto"* ]] + [[ "$output" == *"PUT repos/owner/repo/pulls/5/reviews/11/dismissals"* ]] + [[ "$output" != *"reviews/12/dismissals"* ]] +} + +@test "automerge: an unreadable post-write state revokes and fails the step" { + _seed "[$(_dep_commit)]" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" "[$(_app_approval_of 11 "$HEAD_SHA")]" + cat > "${FIX_DIR}/on_write" <<'HOOK' +#!/usr/bin/env bash +printf '1 +' > "${FIX_DIR}/repos_owner_repo_pulls_5_commits_per_page_100.rc" +HOOK + chmod +x "${FIX_DIR}/on_write" + _run_approve_step + [ "$status" -ne 0 ] + run _writes + [[ "$output" == *"--disable-auto"* ]] + [[ "$output" == *"PUT repos/owner/repo/pulls/5/reviews/11/dismissals"* ]] +} + +@test "automerge: auto-merge refused at an unchanged head fails the step and keeps the approval" { + _seed "[$(_dep_commit)]" + printf '1\n' > "${FIX_DIR}/pr_merge.rc" + _run_approve_step + [ "$status" -ne 0 ] + [[ "$(_writes)" != *"dismissals"* ]] +} + @test "automerge: a stale event (head moved since) approves nothing and revokes nothing" { _seed "[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' web-flow true 1)]" _run_approve_step "$OTHER_SHA" diff --git a/test/workflows/dependabot/helpers/setup.bash b/test/workflows/dependabot/helpers/setup.bash index b106bf72d..e9be70a39 100644 --- a/test/workflows/dependabot/helpers/setup.bash +++ b/test/workflows/dependabot/helpers/setup.bash @@ -51,7 +51,9 @@ tt_fixture() { # gh api ENDPOINT (GET) → .json (404 when absent); # `--paginate --slurp` wraps it in a page array # gh api ENDPOINT -X PUT|POST → recorded in writes.log with its fields; -# succeeds unless .rc exists +# succeeds unless .rc exists; runs +# on_write first when present (e.g. a push +# landing while the workflow is writing) # gh pr list … --jq E → pr_list.json through jq # gh pr view N --json … [--jq E] → pr_view_N.json (through jq when given) # gh pr merge / gh pr review → recorded in writes.log; `pr_merge.rc` fails @@ -90,6 +92,7 @@ if [ "$1" = "api" ]; then fi if [ "$method" != "GET" ]; then printf 'TOKEN=%s %s %s%s\n' "${GH_TOKEN:-}" "$method" "$endpoint" "$fields" >> "${FIX_DIR}/writes.log" + if [ -x "${FIX_DIR}/on_write" ]; then "${FIX_DIR}/on_write"; fi if [ -f "${FIX_DIR}/${key}.rc" ]; then exit "$(cat "${FIX_DIR}/${key}.rc")"; fi exit 0 fi @@ -167,9 +170,11 @@ tt_commit() { commit:{verification:{verified:($v == "true"), reason:(if $v == "true" then "valid" else "unsigned" end)}}}' } -# tt_pr_rest [HEAD] [COMMIT_COUNT] — REST pull request object. +# tt_pr_rest [HEAD] [COMMIT_COUNT] [AUTO_MERGE true|false] — REST pull request object. tt_pr_rest() { - jq -cn --arg head "${1:-$HEAD_SHA}" --argjson n "${2:-1}" '{number:5,head:{sha:$head},base:{ref:"main"},commits:$n}' + jq -cn --arg head "${1:-$HEAD_SHA}" --argjson n "${2:-1}" --arg am "${3:-false}" \ + '{number:5,head:{sha:$head},base:{ref:"main"},commits:$n, + auto_merge:(if $am == "true" then {merge_method:"squash"} else null end)}' } # tt_seed_pr STATE_JSON COMMITS_JSON [REQUIRED_RULES_JSON] From 64d692a8ca7503f8bd894446e8fe4cb0b9645b13 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:48:52 -0600 Subject: [PATCH 03/17] fix(security): withdraw automation when a Dependabot update becomes ineligible (#1400) The merger trusts the auto-merge request, but a new head that no longer qualifies skipped every later step, so the approval and auto-merge earned by the earlier head survived and the App could merge an update that needs human review. Withdraw the App's approvals and any auto-merge the App enabled; auto-merge a human enabled stays. Found by the AGY review of #1421. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 1 + .github/scripts/dependabot/lib/gh-io.sh | 20 ++++++ .../dependabot-automerge-reusable.yml | 26 ++++++- .../dependabot/automerge-provenance.bats | 70 ++++++++++++++++++- test/workflows/dependabot/helpers/setup.bash | 7 +- 5 files changed, 118 insertions(+), 6 deletions(-) diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index 7db392f52..e5ceb2eeb 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -35,6 +35,7 @@ failure fails closed. | `dependabot_approve_and_automerge REPO PR SHA APP_LOGIN` | Approves with `commit_id=SHA`, enables auto-merge with `--match-head-commit SHA`, then re-reads the PR. If the head moved to a verified commit, it dismisses the approval of `SHA`; if the new head fails the gate or the PR cannot be read, it revokes everything. | | `dependabot_dismiss_app_approvals REPO PR APP_LOGIN REASON [KEEP_SHA]` | Dismisses `APP_LOGIN`'s approvals except those of `KEEP_SHA`. Returns `1` if one may have survived. | | `dependabot_revoke_automation REPO PR APP_LOGIN REASON` | Disables auto-merge and dismisses all of `APP_LOGIN`'s approvals. Returns `1` unless the PR reads back with auto-merge off and every dismissal succeeded, and the workflow then fails. | +| `dependabot_withdraw_app_automation REPO PR APP_LOGIN REASON` | For an update that became ineligible: dismisses `APP_LOGIN`'s approvals and disables auto-merge only if `APP_LOGIN` enabled it. | **What this cannot make atomic.** Native auto-merge has no head-bound form that survives a push: a writer's push does not disable it, and `--match-head-commit` diff --git a/.github/scripts/dependabot/lib/gh-io.sh b/.github/scripts/dependabot/lib/gh-io.sh index a16ed5dd1..6104cd744 100644 --- a/.github/scripts/dependabot/lib/gh-io.sh +++ b/.github/scripts/dependabot/lib/gh-io.sh @@ -245,3 +245,23 @@ dependabot_revoke_automation() { dependabot_dismiss_app_approvals "$repo" "$pr" "$app_login" "$reason" || failed=1 return "$failed" } + +# dependabot_withdraw_app_automation REPO PR_NUMBER APP_LOGIN REASON +# For an update that no longer qualifies for automation: dismisses APP_LOGIN's +# approvals and disables auto-merge only when APP_LOGIN enabled it, since a +# human may have enabled it deliberately after reviewing. Returns 1 when the +# state cannot be read or the withdrawal cannot be completed. +dependabot_withdraw_app_automation() { + local repo="$1" pr="$2" app_login="$3" reason="$4" pr_json enabled_by + + if ! pr_json=$(GH_TOKEN="${READ_TOKEN:-${GH_TOKEN:-}}" dependabot_fetch_pr "$repo" "$pr"); then + echo "::error::Could not read PR #${pr} to withdraw automation" + return 1 + fi + enabled_by=$(jq -r '.auto_merge.enabled_by.login // empty' <<<"$pr_json" 2>/dev/null) || enabled_by="" + if [[ -n "$enabled_by" && "$enabled_by" == "$app_login" ]]; then + dependabot_revoke_automation "$repo" "$pr" "$app_login" "$reason" + return + fi + dependabot_dismiss_app_approvals "$repo" "$pr" "$app_login" "$reason" +} diff --git a/.github/workflows/dependabot-automerge-reusable.yml b/.github/workflows/dependabot-automerge-reusable.yml index 8f776b1c5..4c1e69931 100644 --- a/.github/workflows/dependabot-automerge-reusable.yml +++ b/.github/workflows/dependabot-automerge-reusable.yml @@ -80,7 +80,6 @@ jobs: echo "Auto-merge eligible: ecosystem=$ECOSYSTEM update=$UPDATE_TYPE" - name: Check app secrets - if: steps.eligible.outputs.eligible == 'true' env: APP_ID: ${{ secrets.APP_ID }} run: | @@ -90,7 +89,6 @@ jobs: fi - name: Generate app token - if: steps.eligible.outputs.eligible == 'true' id: app-token uses: actions/create-github-app-token@64ab729a6294000f5aa4a4bc0b73d1f32bc45510 # v3 with: @@ -98,7 +96,6 @@ jobs: private-key: ${{ secrets.APP_PRIVATE_KEY }} - name: Checkout Dependabot gate tooling - if: steps.eligible.outputs.eligible == 'true' uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: repository: combsmith/.github @@ -109,6 +106,29 @@ jobs: fetch-depth: 1 persist-credentials: false + # Eligibility can change with a new head. The merger trusts the auto-merge + # request, so an update that now needs human review must not keep the + # automation an earlier head earned. Auto-merge a human enabled is theirs. + - name: Withdraw automation from an ineligible update + if: steps.eligible.outputs.eligible == 'false' + run: | + # shellcheck source=/dev/null + . .dependabot-tooling/.github/scripts/dependabot/lib/gates.sh + # shellcheck source=/dev/null + . .dependabot-tooling/.github/scripts/dependabot/lib/gh-io.sh + + APP_LOGIN="${APP_SLUG:+${APP_SLUG}[bot]}" + if ! dependabot_withdraw_app_automation "$REPO" "$PR_NUMBER" "$APP_LOGIN" "the update now requires human review"; then + echo "::error::Could not fully withdraw automation on PR #$PR_NUMBER — a human must check it" + exit 1 + fi + env: + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + APP_SLUG: ${{ steps.app-token.outputs.app-slug }} + READ_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ steps.app-token.outputs.token }} + - name: Approve and enable auto-merge if: steps.eligible.outputs.eligible == 'true' run: | diff --git a/test/workflows/dependabot/automerge-provenance.bats b/test/workflows/dependabot/automerge-provenance.bats index de4d71a8f..d8b226f4d 100644 --- a/test/workflows/dependabot/automerge-provenance.bats +++ b/test/workflows/dependabot/automerge-provenance.bats @@ -69,11 +69,77 @@ _foreign_commits() { grep -F '.dependabot-tooling/.github/scripts/dependabot/lib/gh-io.sh' "$RUN_SCRIPT" } -@test "automerge: tooling checkout runs at the reusable's own commit, only when eligible" { +@test "automerge: tooling checkout runs at the reusable's own commit, eligible or not" { run yq -r '.jobs.dependabot.steps[] | select(.name == "Checkout Dependabot gate tooling") | .with.ref' "$REUSABLE" [ "$output" = '${{ job.workflow_sha }}' ] - run yq -r '.jobs.dependabot.steps[] | select(.name == "Checkout Dependabot gate tooling") | .if' "$REUSABLE" + # An ineligible update still needs the libraries and the App token to withdraw + # earlier automation. + for step in "Checkout Dependabot gate tooling" "Generate app token" "Check app secrets"; do + run yq -r ".jobs.dependabot.steps[] | select(.name == \"$step\") | .if" "$REUSABLE" + [ "$output" = "null" ] + done +} + +@test "automerge: approval runs only when eligible, withdrawal only when not" { + run yq -r '.jobs.dependabot.steps[] | select(.name == "Approve and enable auto-merge") | .if' "$REUSABLE" [ "$output" = "steps.eligible.outputs.eligible == 'true'" ] + run yq -r '.jobs.dependabot.steps[] | select(.name == "Withdraw automation from an ineligible update") | .if' "$REUSABLE" + [ "$output" = "steps.eligible.outputs.eligible == 'false'" ] +} + +# _run_withdraw_step — the real withdrawal step for an ineligible update. +_run_withdraw_step() { + local script="${TT_TMP}/withdraw.sh" + tt_extract_run "$REUSABLE" dependabot "Withdraw automation from an ineligible update" > "$script" + cd "$TT_WORK" || return 1 + REPO="owner/repo" PR_NUMBER=5 APP_SLUG="$APP_SLUG_FIXTURE" READ_TOKEN="read-token" GH_TOKEN="app-token" \ + run bash -eo pipefail "$script" +} + +_pr_with_auto_merge_by() { # LOGIN + jq -cn --arg head "$HEAD_SHA" --arg by "$1" \ + '{number:5,head:{sha:$head},base:{ref:"main"},commits:1, + auto_merge:{merge_method:"squash",enabled_by:{login:$by}}}' +} + +@test "automerge: an update that became ineligible loses the App's approval and auto-merge" { + _seed "[$(_dep_commit)]" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" "[$(_app_approval_of 11 "$DEP_SHA")]" + # The withdrawal turns auto-merge off, so the read-back sees it off. + tt_fixture "repos/owner/repo/pulls/5" "$(_pr_with_auto_merge_by "$APP_LOGIN_FIXTURE")" + printf '%s\n' "$(tt_pr_rest "$HEAD_SHA" 1)" > "${FIX_DIR}/after_disable.json" + _run_withdraw_step + [ "$status" -eq 0 ] + run _writes + [[ "$output" == *"pr merge 5 --repo owner/repo --disable-auto"* ]] + [[ "$output" == *"PUT repos/owner/repo/pulls/5/reviews/11/dismissals"* ]] +} + +@test "automerge: auto-merge a human enabled on an ineligible update is left alone" { + _seed "[$(_dep_commit)]" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" \ + "[$(_app_approval_of 11 "$DEP_SHA"),{\"id\":12,\"state\":\"APPROVED\",\"commit_id\":\"${HEAD_SHA}\",\"user\":{\"login\":\"alice\"}}]" + tt_fixture "repos/owner/repo/pulls/5" "$(_pr_with_auto_merge_by alice)" + _run_withdraw_step + [ "$status" -eq 0 ] + run _writes + [[ "$output" != *"--disable-auto"* ]] + [[ "$output" == *"reviews/11/dismissals"* ]] + [[ "$output" != *"reviews/12/dismissals"* ]] +} + +@test "automerge: an ineligible update with no automation on it writes nothing" { + _seed "[$(_dep_commit)]" + _run_withdraw_step + [ "$status" -eq 0 ] + [ -z "$(_writes)" ] +} + +@test "automerge: an ineligible update whose state cannot be read fails the step" { + _seed "[$(_dep_commit)]" + tt_fixture "repos/owner/repo/pulls/5" '{"message":"Server Error"}' 1 + _run_withdraw_step + [ "$status" -ne 0 ] } @test "automerge: Dependabot-only PR is approved at the head SHA and auto-merge is bound to it" { diff --git a/test/workflows/dependabot/helpers/setup.bash b/test/workflows/dependabot/helpers/setup.bash index e9be70a39..35e65c026 100644 --- a/test/workflows/dependabot/helpers/setup.bash +++ b/test/workflows/dependabot/helpers/setup.bash @@ -56,7 +56,9 @@ tt_fixture() { # landing while the workflow is writing) # gh pr list … --jq E → pr_list.json through jq # gh pr view N --json … [--jq E] → pr_view_N.json (through jq when given) -# gh pr merge / gh pr review → recorded in writes.log; `pr_merge.rc` fails +# gh pr merge / gh pr review → recorded in writes.log; `pr_merge.rc` fails; +# `--disable-auto` serves after_disable.json as +# the PR when present _tt_install_gh_stub() { cat > "${TT_BIN}/gh" <<'STUB' #!/usr/bin/env bash @@ -128,6 +130,9 @@ fi if [ "$1" = "pr" ] && { [ "$2" = "merge" ] || [ "$2" = "review" ]; }; then printf 'TOKEN=%s %s\n' "${GH_TOKEN:-}" "$*" >> "${FIX_DIR}/writes.log" if [ "$2" = "merge" ] && [ -f "${FIX_DIR}/pr_merge.rc" ]; then exit "$(cat "${FIX_DIR}/pr_merge.rc")"; fi + if [ -f "${FIX_DIR}/after_disable.json" ] && [[ " $* " == *" --disable-auto "* ]]; then + cp "${FIX_DIR}/after_disable.json" "${FIX_DIR}/repos_owner_repo_pulls_5.json" + fi exit 0 fi From e70c253ce0d364086f112b6e3cbbc0582fe72943 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:14:56 -0600 Subject: [PATCH 04/17] test(dependabot): fail extraction of a missing run block; correct the provenance docs (#1400) A step without a run block produced a null script, which let negative tests pass because bash exited non-zero without running the workflow logic. Also state that every accepted commit needs a verified signature, and use [[ ]] in the helper. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 2 +- .../dependabot/automerge-provenance.bats | 8 ++++++++ test/workflows/dependabot/helpers/setup.bash | 16 ++++++++++++---- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index e5ceb2eeb..557175fe1 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -19,7 +19,7 @@ No network calls. Each gate prints a one-line reason and returns `0` (pass) or | Function | Passes only when | |----------|------------------| | `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Every check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present; the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). | -| `dependabot_provenance_gate PR_JSON COMMITS_JSON [APP_LOGIN] [SHA]` | The full commit list was read, at least one commit is Dependabot's, and every commit is either authored by `dependabot[bot]` and committed by GitHub (`web-flow`) or `dependabot[bot]` with a verified signature, or a merge commit by `APP_LOGIN` committed by `web-flow` with a verified signature (the App's update-branch merges). Anything else — e.g. a commit someone else pushed to the branch — blocks (#1400). | +| `dependabot_provenance_gate PR_JSON COMMITS_JSON [APP_LOGIN] [SHA]` | The full commit list was read, at least one commit is Dependabot's, and every commit has a verified signature and is either authored by `dependabot[bot]` and committed by `web-flow` or `dependabot[bot]`, or a merge commit by `APP_LOGIN` committed by `web-flow` (the App's update-branch merges). Anything else — e.g. a commit someone else pushed to the branch — blocks (#1400). | | `dependabot_required_contexts RULES_JSON` | Prints the unique required status-check names from `GET /repos/{r}/rules/branches/{b}` (returns `1` on a non-array). | ## `lib/gh-io.sh` — GitHub I/O diff --git a/test/workflows/dependabot/automerge-provenance.bats b/test/workflows/dependabot/automerge-provenance.bats index d8b226f4d..0c7429e3c 100644 --- a/test/workflows/dependabot/automerge-provenance.bats +++ b/test/workflows/dependabot/automerge-provenance.bats @@ -63,6 +63,14 @@ _foreign_commits() { "$(tt_commit "$HEAD_SHA" mallory mallory true 1)" } +@test "tt_extract_run rejects a missing step and a step with no run block" { + run tt_extract_run "$REUSABLE" dependabot "No such step" + [ "$status" -ne 0 ] + run tt_extract_run "$REUSABLE" dependabot "Generate app token" + [ "$status" -ne 0 ] + [[ "$output" == *"no run block"* ]] +} + @test "automerge: the approval step sources the gate libraries" { [ -s "$RUN_SCRIPT" ] grep -F '.dependabot-tooling/.github/scripts/dependabot/lib/gates.sh' "$RUN_SCRIPT" diff --git a/test/workflows/dependabot/helpers/setup.bash b/test/workflows/dependabot/helpers/setup.bash index 35e65c026..e54f8f9d8 100644 --- a/test/workflows/dependabot/helpers/setup.bash +++ b/test/workflows/dependabot/helpers/setup.bash @@ -29,7 +29,7 @@ tt_setup() { } tt_teardown() { - if [ -n "${TT_TMP:-}" ] && [ -d "${TT_TMP}" ]; then + if [[ -n "${TT_TMP:-}" && -d "${TT_TMP}" ]]; then rm -rf "${TT_TMP}" fi } @@ -42,7 +42,7 @@ tt_fixture() { key="$(_tt_key "$1")" printf '%s\n' "$2" > "${FIX_DIR}/${key}.json" rm -f "${FIX_DIR}/${key}.rc" - if [ -n "${3:-}" ]; then printf '%s\n' "$3" > "${FIX_DIR}/${key}.rc"; fi + if [[ -n "${3:-}" ]]; then printf '%s\n' "$3" > "${FIX_DIR}/${key}.rc"; fi } # ── `gh` stub ──────────────────────────────────────────────────────────────── @@ -201,7 +201,15 @@ tt_seed_pr() { <<<"$state" > "${FIX_DIR}/pr_view_5.json" } -# tt_extract_run WORKFLOW JOB STEP_NAME — print a step's run block. +# tt_extract_run WORKFLOW JOB STEP_NAME — print a step's run block. Fails when +# the step is missing or has no run block, so a negative test can't pass just +# because bash was handed an empty or `null` script. tt_extract_run() { - yq -r ".jobs.\"$2\".steps[] | select(.name == \"$3\") | .run" "$1" + local script + script="$(yq -r ".jobs.\"$2\".steps[] | select(.name == \"$3\") | .run" "$1")" || return 1 + if [[ -z "$script" || "$script" == "null" ]]; then + echo "no run block for step '$3' in job '$2' of $1" >&2 + return 1 + fi + printf '%s\n' "$script" } From 6150ffb59a9838526a5d3ffa3d0e7920834f3e97 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Mon, 2 Nov 2026 17:34:01 -0600 Subject: [PATCH 05/17] docs(dependabot): record the accepted auto-merge window (#1400) The maintainer chose to keep native auto-merge and accept that a foreign head can stay eligible for it until revocation completes. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 5 ++++- standards/dependabot-policy.md | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index 557175fe1..a2b00b48c 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -43,4 +43,7 @@ applies only when it is enabled. The re-read and the `synchronize` run together withdraw that state on every interleaving, but not in the same instant, so a foreign push has a short window before its run revokes. The merger is not exposed to it: it re-runs both gates and merges with `sha=`. If the `synchronize` -run does not run at all, the state stays until something re-evaluates the PR. +run does not run at all, the state stays until something re-evaluates the PR. The +required approval of the last push should block a merge in that window, but this +has not been tested end to end. The maintainer accepted this risk on 2026-11-02 +(see `standards/dependabot-policy.md`). diff --git a/standards/dependabot-policy.md b/standards/dependabot-policy.md index fd0fea101..0ce4ecbef 100644 --- a/standards/dependabot-policy.md +++ b/standards/dependabot-policy.md @@ -178,6 +178,12 @@ Behavior: PR then needs a human review. A withdrawal that cannot be completed fails the run. To hand it back to automation, have Dependabot recreate it (`@dependabot recreate`). +- **Accepted residual risk (2026-11-02):** native auto-merge has no form tied to + one commit, so a foreign head can stay eligible for it until the revocation + completes. The required approval of the last push should block a merge in that + window, but this has not been tested end to end. The maintainer accepted this + and kept native auto-merge. The direct merger is not exposed: it re-runs both + gates and merges with `sha=`. ## Update and Merge Behind PRs Workflow From 738f4f248c0d54fb773a8e862df148bf0fb81ec9 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:05:10 -0600 Subject: [PATCH 06/17] fix(dependabot): do not hold merges on org agent jobs The merge gate required every check on the head commit to succeed, so a cancelled or failed agent job held most Dependabot PRs even though those jobs do not test the change. Stop counting them, on an exact name from GitHub Actions only, and never when the rules require the check. Other failing checks still hold the merge. Also document a job permission and use [[ ]] in the test helper. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 2 +- .github/scripts/dependabot/lib/gates.sh | 40 ++++++++--- .github/scripts/dependabot/lib/gh-io.sh | 6 +- .../workflows/dependabot-rebase-reusable.yml | 7 +- standards/dependabot-policy.md | 4 +- test/workflows/dependabot/gates.bats | 71 ++++++++++++++++++- test/workflows/dependabot/helpers/setup.bash | 7 +- .../dependabot/rebase-merge-gate.bats | 10 +++ 8 files changed, 124 insertions(+), 23 deletions(-) diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index a2b00b48c..f2526b9bb 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -18,7 +18,7 @@ No network calls. Each gate prints a one-line reason and returns `0` (pass) or | Function | Passes only when | |----------|------------------| -| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Every check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present; the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). | +| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Agent jobs (`DEPENDABOT_AGENT_JOB_RE`, GitHub Actions only, never a required check) are not counted. Every other check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present; the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). | | `dependabot_provenance_gate PR_JSON COMMITS_JSON [APP_LOGIN] [SHA]` | The full commit list was read, at least one commit is Dependabot's, and every commit has a verified signature and is either authored by `dependabot[bot]` and committed by `web-flow` or `dependabot[bot]`, or a merge commit by `APP_LOGIN` committed by `web-flow` (the App's update-branch merges). Anything else — e.g. a commit someone else pushed to the branch — blocks (#1400). | | `dependabot_required_contexts RULES_JSON` | Prints the unique required status-check names from `GET /repos/{r}/rules/branches/{b}` (returns `1` on a non-array). | diff --git a/.github/scripts/dependabot/lib/gates.sh b/.github/scripts/dependabot/lib/gates.sh index beaa6581e..06524e357 100644 --- a/.github/scripts/dependabot/lib/gates.sh +++ b/.github/scripts/dependabot/lib/gates.sh @@ -15,25 +15,37 @@ # GitHub-verified signature. DEPENDABOT_LOGIN="dependabot[bot]" +# Org agent jobs that run on every PR but do not test the change. A cancelled or +# failed agent job says nothing about a dependency update, so it does not hold +# the merge. Exact names and prefixes only, and only from GitHub Actions, so a +# similarly named external check still counts. A check the base branch's rules +# require is never exempt. +# dev-lead / *, pr-auto-review / * caller jobs of those reusables +# pr-review / review the agent-ingress caller job +# review / review the pre-ingress caller job +DEPENDABOT_AGENT_JOB_RE='^(dev-lead / .+|pr-auto-review / .+|pr-review / review|review / review)$' + # dependabot_checks_gate PR_JSON REQUIRED_JSON [EXPECTED_SHA] # PR_JSON GraphQL `pullRequest` object (or the whole `{data:{repository: # {pullRequest:…}}}` response) with headRefOid and # commits(last:1){nodes{commit{oid statusCheckRollup{contexts{ -# pageInfo{hasNextPage} nodes{CheckRun{name status conclusion} -# StatusContext{context state}}}}}}}. +# pageInfo{hasNextPage} nodes{CheckRun{name status conclusion +# checkSuite{app{slug}}} StatusContext{context state}}}}}}}. # REQUIRED_JSON JSON array of check names the base branch's rules require # (`[]` when none). # EXPECTED_SHA optional — block unless the PR head is this commit. # -# Passes only when every check run on the head commit COMPLETED as success, -# neutral or skipped, every commit status is success, at least one check was -# reported, and every required check name is present. Anything else — -# failure, cancelled, timed_out, action_required, stale, startup_failure, -# pending, missing, or more contexts than one page — blocks. +# Agent jobs (DEPENDABOT_AGENT_JOB_RE) are not counted. Passes only when every +# other check run on the head commit COMPLETED as success, neutral or skipped, +# every commit status is success, at least one such check was reported, and +# every required check name is present. Anything else — failure, cancelled, +# timed_out, action_required, stale, startup_failure, pending, missing, or more +# contexts than one page — blocks. dependabot_checks_gate() { local pr_json="$1" required_json="${2:-[]}" expected="${3:-}" out verdict reason - out=$(jq -r --argjson required "$required_json" --arg expected "$expected" ' + out=$(jq -r --argjson required "$required_json" --arg expected "$expected" \ + --arg agent_re "$DEPENDABOT_AGENT_JOB_RE" ' (if has("data") then .data.repository.pullRequest else . end) as $pr | ($pr.headRefOid // "") as $head | ($pr.commits.nodes[0].commit // {}) as $commit @@ -51,15 +63,21 @@ dependabot_checks_gate() { def outcome: if .__typename == "StatusContext" then (.state // "MISSING") elif ((.status // "" | ascii_upcase) != "COMPLETED") then (.status // "MISSING") else (.conclusion // "MISSING") end | ascii_downcase; + def agent_job: .__typename == "CheckRun" + and (.checkSuite.app.slug? // "") == "github-actions" + and (name_of | test($agent_re)) + and (name_of as $n | $required | index($n) | not); ([$nodes[] | name_of]) as $names | ([$required[] | select(. as $r | $names | index($r) | not)]) as $missing - | ([$nodes[] | select(ok | not) | "\(name_of) (\(outcome))"]) as $bad + | ([$nodes[] | select(agent_job | not)]) as $counted + | (($nodes | length) - ($counted | length)) as $exempt + | ([$counted[] | select(ok | not) | "\(name_of) (\(outcome))"]) as $bad | if $head == "" then "block\tPR head commit unknown" elif ($expected != "" and $head != $expected) then "block\thead moved to \($head[0:7]) (expected \($expected[0:7]))" elif ($commit.oid // "") != $head then "block\tcheck rollup is not for the head commit \($head[0:7])" - elif $ctx == null or ($nodes | length) == 0 then + elif $ctx == null or ($counted | length) == 0 then "block\tno checks reported on \($head[0:7])" elif ($ctx.pageInfo.hasNextPage // false) then "block\tmore checks than fit in one page on \($head[0:7]) — cannot verify them all" @@ -68,7 +86,7 @@ dependabot_checks_gate() { elif ($missing | length) > 0 then "block\trequired check(s) not reported on \($head[0:7]): \($missing | join(", "))" else - "pass\tall \($nodes | length) check(s) succeeded on \($head[0:7])" + "pass\tall \($counted | length) check(s) succeeded on \($head[0:7])\(if $exempt > 0 then " (\($exempt) agent job(s) not counted)" else "" end)" end ' <<<"$pr_json" 2>/dev/null) || out="" diff --git a/.github/scripts/dependabot/lib/gh-io.sh b/.github/scripts/dependabot/lib/gh-io.sh index 6104cd744..c1eb095ef 100644 --- a/.github/scripts/dependabot/lib/gh-io.sh +++ b/.github/scripts/dependabot/lib/gh-io.sh @@ -16,7 +16,7 @@ dependabot_fetch_pr_state() { local repo="$1" pr="$2" out # shellcheck disable=SC2016 # GraphQL variables, not shell expansions - local query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){headRefOid baseRefName mergeable autoMergeRequest{enabledAt} commits(last:1){nodes{commit{oid statusCheckRollup{contexts(first:100){pageInfo{hasNextPage} nodes{__typename ... on CheckRun{name status conclusion} ... on StatusContext{context state}}}}}}}}}}' + local query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){headRefOid baseRefName mergeable autoMergeRequest{enabledAt} commits(last:1){nodes{commit{oid statusCheckRollup{contexts(first:100){pageInfo{hasNextPage} nodes{__typename ... on CheckRun{name status conclusion checkSuite{app{slug}}} ... on StatusContext{context state}}}}}}}}}}' out=$(gh api graphql -f query="$query" -f owner="${repo%%/*}" \ -f name="${repo#*/}" -F number="$pr" 2>/dev/null) || return 1 @@ -85,8 +85,8 @@ dependabot_provenance_check() { # dependabot_merge_gate REPO PR_NUMBER TRUSTED_MERGE_LOGIN # Decides whether the merger may merge the PR now: auto-merge requested, -# MERGEABLE, every check on the head commit successful (incl. every check the -# base branch's rules require), and every commit from Dependabot (or a branch +# MERGEABLE, every check on the head commit other than agent jobs successful +# (incl. every check the base branch's rules require), and every commit from Dependabot (or a branch # update by TRUSTED_MERGE_LOGIN) — all bound to one head SHA. # Sets DEPENDABOT_GATE_REASON, and DEPENDABOT_GATE_SHA on pass. # Returns 0 merge / 1 skip. diff --git a/.github/workflows/dependabot-rebase-reusable.yml b/.github/workflows/dependabot-rebase-reusable.yml index 17470f7fa..5ee2535d9 100644 --- a/.github/workflows/dependabot-rebase-reusable.yml +++ b/.github/workflows/dependabot-rebase-reusable.yml @@ -19,7 +19,8 @@ # which is the only token subject to GitHub's recursive-trigger guard. # 2. Merges any Dependabot PR that is up-to-date and whose head commit passes # the merge gate (issues #1399, #1400) — auto-merge requested, MERGEABLE, -# EVERY check on the head commit successful (success/neutral/skipped; +# EVERY check on the head commit other than org agent jobs successful +# (success/neutral/skipped; # every check the base branch's rules require must be present), and every # commit on the PR from Dependabot or a branch update by this App. The # merge is bound to the verified head SHA. Decision logic lives in @@ -68,7 +69,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read # check out the gate scripts from combsmith/.github - pull-requests: write + pull-requests: write # list Dependabot PRs and read their state with GITHUB_TOKEN steps: - name: Checkout Dependabot gate tooling uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -171,7 +172,7 @@ jobs: # The App merges with ruleset bypass, so GitHub's own gates do not # stop it — the merge gate must. It requires auto-merge requested - # (set by the automerge workflow), MERGEABLE, EVERY check on the + # (set by the automerge workflow), MERGEABLE, EVERY non-agent check on the # head commit successful (fail closed on failure, cancelled, # timed_out, action_required, stale, startup_failure, pending or a # missing required check), and every commit from Dependabot or a diff --git a/standards/dependabot-policy.md b/standards/dependabot-policy.md index 0ce4ecbef..e7e5ae649 100644 --- a/standards/dependabot-policy.md +++ b/standards/dependabot-policy.md @@ -266,7 +266,9 @@ current head commit: the App (same provenance gate as the auto-merge workflow). The merge call passes the verified head SHA, so GitHub rejects it if the branch -moved after the check. A Dependabot PR held back by a failing non-required check +moved after the check. Org agent jobs (`dev-lead / *`, `pr-auto-review / *`, +`pr-review / review`, `review / review`) from GitHub Actions are not counted, since +they do not test the change, unless the rulesets require them. A Dependabot PR held back by a failing non-required check (e.g. a gitleaks false positive) needs a human to fix the check or merge it. Gate logic: [`.github/scripts/dependabot/`](../.github/scripts/dependabot/README.md). diff --git a/test/workflows/dependabot/gates.bats b/test/workflows/dependabot/gates.bats index 8bbd6b892..eb9b4ccd5 100644 --- a/test/workflows/dependabot/gates.bats +++ b/test/workflows/dependabot/gates.bats @@ -26,7 +26,7 @@ _ok_nodes() { @test "checks: all success / skipped / status success passes" { run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)")" '[]' [ "$status" -eq 0 ] - [[ "$output" == *"all 3 check(s) succeeded"* ]] + [[ "$output" == *"all 2 check(s) succeeded"*"(1 agent job(s) not counted)"* ]] } @test "checks: neutral conclusion passes" { @@ -130,6 +130,75 @@ _ok_nodes() { [ "$status" -eq 1 ] } +# ── agent jobs: not counted, and only on an exact match from GitHub Actions ── + +@test "checks: a cancelled or failed agent job does not block" { + local job + for job in 'dev-lead / dispatch' 'dev-lead / ci-relay' 'pr-auto-review / check-and-dispatch' \ + 'pr-review / review' 'review / review'; do + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run "$job" COMPLETED CANCELLED),$(tt_check_run 'dev-lead / resume' COMPLETED FAILURE)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 0 ] + [[ "$output" == *"all 1 check(s) succeeded"*"(2 agent job(s) not counted)"* ]] + done +} + +@test "checks: a pending agent job does not block" { + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run 'dev-lead / dispatch' IN_PROGRESS '')]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 0 ] +} + +@test "checks: a similarly named check from GitHub Actions still blocks" { + local job + for job in 'dev-lead' 'dev-lead / ' 'dev-leads / dispatch' 'my dev-lead / dispatch' 'dev-lead/dispatch' \ + 'pr-auto-review' 'pr-review / lint' 'review / lint' 'review' 'Dev-Lead / dispatch'; do + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run "$job" COMPLETED FAILURE)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] + done +} + +@test "checks: an agent job name from another app still blocks" { + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run 'dev-lead / dispatch' COMPLETED FAILURE some-other-app)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] + [[ "$output" == *"dev-lead / dispatch (failure)"* ]] +} + +@test "checks: an agent job with no app data still blocks" { + node=$(jq -cn '{__typename:"CheckRun",name:"dev-lead / dispatch",status:"COMPLETED",conclusion:"FAILURE"}') + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$node]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: a commit status named like an agent job still blocks" { + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_status 'dev-lead / dispatch' FAILURE)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: an agent job the rules require is counted" { + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run 'dev-lead / dispatch' COMPLETED FAILURE)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '["dev-lead / dispatch"]' + [ "$status" -eq 1 ] +} + +@test "checks: agent jobs alone do not count as checks reported" { + nodes="[$(tt_check_run 'dev-lead / dispatch' COMPLETED SUCCESS),$(tt_check_run 'pr-auto-review / check-and-dispatch' COMPLETED SKIPPED)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] + [[ "$output" == *"no checks reported"* ]] +} + +@test "checks: a failing non-agent check still blocks beside an exempt agent job" { + nodes="[$(tt_check_run 'Secret scan (gitleaks)' COMPLETED FAILURE),$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run 'dev-lead / dispatch' COMPLETED CANCELLED)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] + [[ "$output" == *"1 check(s) not successful"*"gitleaks"* ]] +} + # ── dependabot_provenance_gate (#1400) ────────────────────────────────────── _dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } diff --git a/test/workflows/dependabot/helpers/setup.bash b/test/workflows/dependabot/helpers/setup.bash index e54f8f9d8..9a0b1c00d 100644 --- a/test/workflows/dependabot/helpers/setup.bash +++ b/test/workflows/dependabot/helpers/setup.bash @@ -143,10 +143,11 @@ STUB # ── fixture builders ───────────────────────────────────────────────────────── -# tt_check_run NAME STATUS CONCLUSION — one GraphQL CheckRun node. +# tt_check_run NAME STATUS CONCLUSION [APP_SLUG] — one GraphQL CheckRun node. tt_check_run() { - jq -cn --arg n "$1" --arg s "$2" --arg c "$3" \ - '{__typename:"CheckRun",name:$n,status:$s,conclusion:(if $c == "" then null else $c end)}' + jq -cn --arg n "$1" --arg s "$2" --arg c "$3" --arg a "${4:-github-actions}" \ + '{__typename:"CheckRun",name:$n,status:$s,conclusion:(if $c == "" then null else $c end), + checkSuite:{app:{slug:$a}}}' } # tt_status NAME STATE — one GraphQL StatusContext node. diff --git a/test/workflows/dependabot/rebase-merge-gate.bats b/test/workflows/dependabot/rebase-merge-gate.bats index e0ee7ee48..26696079b 100644 --- a/test/workflows/dependabot/rebase-merge-gate.bats +++ b/test/workflows/dependabot/rebase-merge-gate.bats @@ -72,6 +72,16 @@ _merge_calls() { grep -F '/pulls/5/merge' "${FIX_DIR}/writes.log" 2>/dev/null || [[ "$output" == *"sha=${HEAD_SHA}"* ]] } +@test "rebase: a cancelled agent job does not hold an otherwise green PR" { + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run 'dev-lead / dispatch' COMPLETED CANCELLED)]" + tt_seed_pr "$(tt_pr_state "$nodes")" "$(_dependabot_only)" + _run_merge_step + [ "$status" -eq 0 ] + [[ "$output" == *"1 agent job(s) not counted"* ]] + run _merge_calls + [[ "$output" == *"sha=${HEAD_SHA}"* ]] +} + @test "rebase: a completed but FAILED check is not merged (regression #1399)" { nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run 'dependency-audit / npm audit' COMPLETED FAILURE)]" tt_seed_pr "$(tt_pr_state "$nodes")" "$(_dependabot_only)" From 6015ebb78b90ee202ced3638f9c3fa12617d9881 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:11:09 -0600 Subject: [PATCH 07/17] fix(dependabot): withdraw automation when metadata fails; count only the newest check runs (#1399, #1400) From CodeRabbit review 5746555186: - a force-push of someone else's commit makes fetch-metadata fail and skipped every later step, so an earlier approval and auto-merge survived. The token, tooling and a new withdrawal step now run on that failure - statusCheckRollup includes runs superseded on the same commit (concurrency cancels, re-runs); only the newest run per app/workflow/event/name counts - a push between the provenance reads, or an incomplete read, is reported as unreadable (2), not as a foreign commit, and the head is re-read to confirm - standards/canary-rings.json watches .github/scripts/dependabot/ for both Dependabot agents, so script-only fixes reach autocut and drift Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 6 +- .github/scripts/dependabot/lib/gates.sh | 38 +++++++-- .github/scripts/dependabot/lib/gh-io.sh | 17 +++- .../dependabot-automerge-reusable.yml | 44 ++++++++++ .github/workflows/dependabot-tests.yml | 2 + standards/canary-rings.json | 16 ++++ .../dependabot/automerge-provenance.bats | 80 ++++++++++++++++++- .../workflows/dependabot/canary-registry.bats | 21 +++++ test/workflows/dependabot/gates.bats | 61 ++++++++++++-- test/workflows/dependabot/helpers/setup.bash | 8 ++ 10 files changed, 271 insertions(+), 22 deletions(-) create mode 100644 test/workflows/dependabot/canary-registry.bats diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index f2526b9bb..b7f2e266b 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -18,8 +18,8 @@ No network calls. Each gate prints a one-line reason and returns `0` (pass) or | Function | Passes only when | |----------|------------------| -| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Agent jobs (`DEPENDABOT_AGENT_JOB_RE`, GitHub Actions only, never a required check) are not counted. Every other check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present; the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). | -| `dependabot_provenance_gate PR_JSON COMMITS_JSON [APP_LOGIN] [SHA]` | The full commit list was read, at least one commit is Dependabot's, and every commit has a verified signature and is either authored by `dependabot[bot]` and committed by `web-flow` or `dependabot[bot]`, or a merge commit by `APP_LOGIN` committed by `web-flow` (the App's update-branch merges). Anything else — e.g. a commit someone else pushed to the branch — blocks (#1400). | +| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Agent jobs (`DEPENDABOT_AGENT_JOB_RE`, GitHub Actions only, never a required check) are not counted. Every other check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present; the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). A check run superseded on the same commit (same app, workflow, event and name, lower databaseId) is not counted. | +| `dependabot_provenance_gate PR_JSON COMMITS_JSON [APP_LOGIN] [SHA]` | The full commit list was read, at least one commit is Dependabot's, and every commit has a verified signature and is either authored by `dependabot[bot]` and committed by `web-flow` or `dependabot[bot]`, or a merge commit by `APP_LOGIN` committed by `web-flow` (the App's update-branch merges). Anything else — e.g. a commit someone else pushed to the branch — blocks (#1400). Returns `2`, not `1`, when the reads cannot be trusted (no head, no commits, a commit list that does not match the PR, malformed input), so a push between two reads is never taken as a foreign commit. | | `dependabot_required_contexts RULES_JSON` | Prints the unique required status-check names from `GET /repos/{r}/rules/branches/{b}` (returns `1` on a non-array). | ## `lib/gh-io.sh` — GitHub I/O @@ -30,7 +30,7 @@ failure fails closed. | Function | Does | |----------|------| | `dependabot_merge_gate REPO PR APP_LOGIN` | Auto-merge requested + `MERGEABLE` + checks gate + provenance gate, all bound to one head SHA. Sets `DEPENDABOT_GATE_SHA` / `DEPENDABOT_GATE_REASON`. Used by the merger before every direct merge. | -| `dependabot_provenance_check REPO PR APP_LOGIN [SHA]` | Provenance gate on the live PR. Returns `0` pass, `1` block (sets `DEPENDABOT_GATE_STALE=true` when the head moved past `SHA`), `2` state unreadable. | +| `dependabot_provenance_check REPO PR APP_LOGIN [SHA]` | Provenance gate on the live PR. Returns `0` pass, `1` block (sets `DEPENDABOT_GATE_STALE=true` when the head moved past `SHA`), `2` state unreadable. Reads the head again after the commit list and returns `2` if it moved in between. | | `dependabot_merge_pr REPO PR SHA` | Squash-merges with `sha=SHA`, so GitHub rejects the merge if the head moved after the gate ran. | | `dependabot_approve_and_automerge REPO PR SHA APP_LOGIN` | Approves with `commit_id=SHA`, enables auto-merge with `--match-head-commit SHA`, then re-reads the PR. If the head moved to a verified commit, it dismisses the approval of `SHA`; if the new head fails the gate or the PR cannot be read, it revokes everything. | | `dependabot_dismiss_app_approvals REPO PR APP_LOGIN REASON [KEEP_SHA]` | Dismisses `APP_LOGIN`'s approvals except those of `KEEP_SHA`. Returns `1` if one may have survived. | diff --git a/.github/scripts/dependabot/lib/gates.sh b/.github/scripts/dependabot/lib/gates.sh index 06524e357..7ea10e62d 100644 --- a/.github/scripts/dependabot/lib/gates.sh +++ b/.github/scripts/dependabot/lib/gates.sh @@ -29,12 +29,17 @@ DEPENDABOT_AGENT_JOB_RE='^(dev-lead / .+|pr-auto-review / .+|pr-review / review| # PR_JSON GraphQL `pullRequest` object (or the whole `{data:{repository: # {pullRequest:…}}}` response) with headRefOid and # commits(last:1){nodes{commit{oid statusCheckRollup{contexts{ -# pageInfo{hasNextPage} nodes{CheckRun{name status conclusion -# checkSuite{app{slug}}} StatusContext{context state}}}}}}}. +# pageInfo{hasNextPage} nodes{CheckRun{databaseId name status +# conclusion checkSuite{app{slug} workflowRun{event workflow{ +# databaseId}}}} StatusContext{context state}}}}}}}. # REQUIRED_JSON JSON array of check names the base branch's rules require # (`[]` when none). # EXPECTED_SHA optional — block unless the PR head is this commit. # +# A check run superseded on the same commit — an earlier run of the same check +# from the same app, workflow and event, e.g. one cancelled by a concurrency +# group or replaced by a re-run — is not counted; only the newest (highest +# databaseId) is. Runs without a databaseId are never collapsed. # Agent jobs (DEPENDABOT_AGENT_JOB_RE) are not counted. Passes only when every # other check run on the head commit COMPLETED as success, neutral or skipped, # every commit status is success, at least one such check was reported, and @@ -50,7 +55,15 @@ dependabot_checks_gate() { | ($pr.headRefOid // "") as $head | ($pr.commits.nodes[0].commit // {}) as $commit | ($commit.statusCheckRollup.contexts // null) as $ctx - | ($ctx.nodes // []) as $nodes + | def latest_runs: + # Newest run per (app, workflow, event, name); anything that cannot be + # keyed or ordered is kept as is, so nothing is dropped by accident. + ([.[] | select(.__typename == "CheckRun" and ((.databaseId | type) == "number"))] + | group_by([(.checkSuite.app.slug? // ""), (.checkSuite.workflowRun.workflow.databaseId? // ""), + (.checkSuite.workflowRun.event? // ""), (.name // "")]) + | map(max_by(.databaseId))) as $newest + | [.[] | select(.__typename != "CheckRun" or ((.databaseId | type) != "number"))] + $newest; + ($ctx.nodes // [] | latest_runs) as $nodes | def name_of: if .__typename == "StatusContext" then (.context // "") else (.name // "") end; def ok: if .__typename == "CheckRun" then @@ -121,6 +134,11 @@ dependabot_checks_gate() { # - a merge commit (2+ parents) authored by TRUSTED_MERGE_LOGIN, committed # by web-flow, with a GitHub-verified signature (update-branch). # Any other commit — e.g. one pushed to the branch by someone else — blocks. +# +# Returns 0 pass, 1 block (a verdict about the commits), 2 block (the reads +# cannot be trusted: no head, no commits, a commit list that does not match +# the PR — a push between the two reads — or malformed input). Callers must +# not treat 2 as evidence of a foreign commit. dependabot_provenance_gate() { local pr_json="$1" commits_json="$2" trusted="${3:-}" expected="${4:-}" out verdict reason @@ -144,14 +162,14 @@ dependabot_provenance_gate() { and ((.parents // []) | length) >= 2 and verified; ([$commits[] | select((from_dependabot or trusted_merge) | not) | "\(.sha // "?" | .[0:7]) (author \(author | if . == "" then "unknown" else . end), committer \(committer | if . == "" then "unknown" else . end)\(if verified then "" else ", unverified" end))"]) as $foreign - | if $head == "" then "block\tPR head commit unknown" + | if $head == "" then "unreadable\tPR head commit unknown" elif ($expected != "" and $head != $expected) then "block\thead moved to \($head[0:7]) (expected \($expected[0:7]))" - elif ($commits | length) == 0 then "block\tno commits read" + elif ($commits | length) == 0 then "unreadable\tno commits read" elif ($commits | length) != $count then - "block\tread \($commits | length) of \($count) commits — cannot verify them all" + "unreadable\tread \($commits | length) of \($count) commits — cannot verify them all" elif ([$commits[] | .sha] | index($head)) == null then - "block\thead \($head[0:7]) not in the commit list" + "unreadable\thead \($head[0:7]) not in the commit list" elif ($foreign | length) > 0 then "block\t\($foreign | length) commit(s) not from Dependabot: \($foreign | join(", "))" elif ([$commits[] | select(from_dependabot)] | length) == 0 then @@ -169,10 +187,14 @@ dependabot_provenance_gate() { fi if [[ "$verdict" == "block" ]]; then echo "$reason" + return 1 + fi + if [[ "$verdict" == "unreadable" ]]; then + echo "$reason" else echo "could not evaluate commit provenance (malformed input)" fi - return 1 + return 2 } # dependabot_required_contexts RULES_JSON diff --git a/.github/scripts/dependabot/lib/gh-io.sh b/.github/scripts/dependabot/lib/gh-io.sh index c1eb095ef..4a9c7b3b8 100644 --- a/.github/scripts/dependabot/lib/gh-io.sh +++ b/.github/scripts/dependabot/lib/gh-io.sh @@ -16,7 +16,7 @@ dependabot_fetch_pr_state() { local repo="$1" pr="$2" out # shellcheck disable=SC2016 # GraphQL variables, not shell expansions - local query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){headRefOid baseRefName mergeable autoMergeRequest{enabledAt} commits(last:1){nodes{commit{oid statusCheckRollup{contexts(first:100){pageInfo{hasNextPage} nodes{__typename ... on CheckRun{name status conclusion checkSuite{app{slug}}} ... on StatusContext{context state}}}}}}}}}}' + local query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){headRefOid baseRefName mergeable autoMergeRequest{enabledAt} commits(last:1){nodes{commit{oid statusCheckRollup{contexts(first:100){pageInfo{hasNextPage} nodes{__typename ... on CheckRun{databaseId name status conclusion checkSuite{app{slug} workflowRun{event workflow{databaseId}}}} ... on StatusContext{context state}}}}}}}}}}' out=$(gh api graphql -f query="$query" -f owner="${repo%%/*}" \ -f name="${repo#*/}" -F number="$pr" 2>/dev/null) || return 1 @@ -76,8 +76,19 @@ dependabot_provenance_check() { DEPENDABOT_GATE_REASON="could not read the pull request's commits" return 2 fi - if ! DEPENDABOT_GATE_REASON=$(dependabot_provenance_gate "$pr_json" "$commits_json" "$trusted" "$expected"); then - return 1 + local rc=0 + DEPENDABOT_GATE_REASON=$(dependabot_provenance_gate "$pr_json" "$commits_json" "$trusted" "$expected") || rc=$? + if [[ "$rc" -ne 0 ]]; then + return "$rc" + fi + # The two reads are not one snapshot: a push between them can pair one head + # with another commit list. Read the head again; if it moved, the verdict + # describes nothing that exists, so report the state as unreadable. + local again + if ! again=$(dependabot_fetch_pr "$repo" "$pr") \ + || [[ "$(jq -r '.head.sha // empty' <<<"$again" 2>/dev/null)" != "$head" ]]; then + DEPENDABOT_GATE_REASON="the head changed while it was being read" + return 2 fi DEPENDABOT_GATE_SHA="$head" [[ -n "$DEPENDABOT_GATE_SHA" ]] diff --git a/.github/workflows/dependabot-automerge-reusable.yml b/.github/workflows/dependabot-automerge-reusable.yml index 4c1e69931..595efe43f 100644 --- a/.github/workflows/dependabot-automerge-reusable.yml +++ b/.github/workflows/dependabot-automerge-reusable.yml @@ -79,7 +79,11 @@ jobs: echo "eligible=true" >> "$GITHUB_OUTPUT" echo "Auto-merge eligible: ecosystem=$ECOSYSTEM update=$UPDATE_TYPE" + # These three run even when Dependabot metadata cannot be read: a force-push + # of someone else's commit makes fetch-metadata fail, and the withdrawal + # below still needs the token and the gate scripts. - name: Check app secrets + if: ${{ !cancelled() }} env: APP_ID: ${{ secrets.APP_ID }} run: | @@ -89,6 +93,7 @@ jobs: fi - name: Generate app token + if: ${{ !cancelled() }} id: app-token uses: actions/create-github-app-token@64ab729a6294000f5aa4a4bc0b73d1f32bc45510 # v3 with: @@ -96,6 +101,7 @@ jobs: private-key: ${{ secrets.APP_PRIVATE_KEY }} - name: Checkout Dependabot gate tooling + if: ${{ !cancelled() }} uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: repository: combsmith/.github @@ -106,6 +112,44 @@ jobs: fetch-depth: 1 persist-credentials: false + # fetch-metadata fails when the PR's first commit is not Dependabot's, e.g. + # after someone force-pushes over the branch. Every later step would then be + # skipped and an earlier approval and auto-merge would survive, so withdraw + # here: everything when the commits are not Dependabot's, otherwise the + # App's own approvals and auto-merge (no eligibility verdict is possible). + # The job still fails, so the run shows the PR needs a human. + - name: Withdraw automation when Dependabot metadata cannot be read + if: ${{ !cancelled() && steps.metadata.outcome == 'failure' }} + run: | + # shellcheck source=/dev/null + . .dependabot-tooling/.github/scripts/dependabot/lib/gates.sh + # shellcheck source=/dev/null + . .dependabot-tooling/.github/scripts/dependabot/lib/gh-io.sh + + APP_LOGIN="${APP_SLUG:+${APP_SLUG}[bot]}" + rc=0 + GH_TOKEN="$READ_TOKEN" dependabot_provenance_check "$REPO" "$PR_NUMBER" "$APP_LOGIN" || rc=$? + if [[ "$rc" -eq 1 ]]; then + echo "::warning::Withdrawing automation on PR #$PR_NUMBER — $DEPENDABOT_GATE_REASON" + withdrawn=0 + dependabot_revoke_automation "$REPO" "$PR_NUMBER" "$APP_LOGIN" "$DEPENDABOT_GATE_REASON" || withdrawn=1 + else + echo "::warning::Withdrawing this App's automation on PR #$PR_NUMBER — its Dependabot metadata could not be read" + withdrawn=0 + dependabot_withdraw_app_automation "$REPO" "$PR_NUMBER" "$APP_LOGIN" "its Dependabot metadata could not be read" || withdrawn=1 + fi + if [[ "$withdrawn" -ne 0 ]]; then + echo "::error::Could not fully withdraw automation on PR #$PR_NUMBER — a human must check it" + fi + echo "::error::Dependabot metadata could not be read for PR #$PR_NUMBER; it needs a human review." + exit 1 + env: + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + APP_SLUG: ${{ steps.app-token.outputs.app-slug }} + READ_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ steps.app-token.outputs.token }} + # Eligibility can change with a new head. The merger trusts the auto-merge # request, so an update that now needs human review must not keep the # automation an earlier head earned. Auto-merge a human enabled is theirs. diff --git a/.github/workflows/dependabot-tests.yml b/.github/workflows/dependabot-tests.yml index 0525a2b5e..686a693ab 100644 --- a/.github/workflows/dependabot-tests.yml +++ b/.github/workflows/dependabot-tests.yml @@ -23,6 +23,7 @@ on: - '.github/scripts/dependabot/**' - 'test/workflows/dependabot/**' - 'standards/workflows/dependabot-*.yml' + - 'standards/canary-rings.json' push: branches: - main @@ -31,6 +32,7 @@ on: - '.github/scripts/dependabot/**' - 'test/workflows/dependabot/**' - 'standards/workflows/dependabot-*.yml' + - 'standards/canary-rings.json' permissions: contents: read diff --git a/standards/canary-rings.json b/standards/canary-rings.json index 4c40bb5c3..84df7e21c 100644 --- a/standards/canary-rings.json +++ b/standards/canary-rings.json @@ -322,6 +322,14 @@ "host": "combsmith/.github", "reusable": ".github/workflows/dependabot-automerge-reusable.yml", "run_workflow": "Dependabot auto-merge", + "agent_ref_paths": [ + ".github/scripts/dependabot/" + ], + "autocut": { + "watched_paths": [ + ".github/scripts/dependabot/" + ] + }, "rings": [ { "channel": "next", @@ -449,6 +457,14 @@ "host": "combsmith/.github", "reusable": ".github/workflows/dependabot-rebase-reusable.yml", "run_workflow": "Dependabot update and merge", + "agent_ref_paths": [ + ".github/scripts/dependabot/" + ], + "autocut": { + "watched_paths": [ + ".github/scripts/dependabot/" + ] + }, "rings": [ { "channel": "next", diff --git a/test/workflows/dependabot/automerge-provenance.bats b/test/workflows/dependabot/automerge-provenance.bats index 0c7429e3c..5e6c15cdd 100644 --- a/test/workflows/dependabot/automerge-provenance.bats +++ b/test/workflows/dependabot/automerge-provenance.bats @@ -77,6 +77,84 @@ _foreign_commits() { grep -F '.dependabot-tooling/.github/scripts/dependabot/lib/gh-io.sh' "$RUN_SCRIPT" } +# ── review round 3 ─────────────────────────────────────────────────────────── + +@test "automerge: a push between the two reads is not taken as a foreign commit" { + # The PR read says one commit; the commit list (read after a valid push) + # has two. Before the fix this revoked the newer head's automation. + _seed "[$(_dep_commit)]" + tt_fixture "repos/owner/repo/pulls/5/commits?per_page=100" "[$(tt_commit "$DEP_SHA" 'dependabot[bot]' web-flow true 1),$(_dep_commit)]" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" "[$(_app_approval_of 11 "$DEP_SHA")]" + _run_approve_step + [ "$status" -ne 0 ] + run _writes + [[ "$output" != *"dismissals"* ]] + [[ "$output" != *"--disable-auto"* ]] + [[ "$output" != *"event=APPROVE"* ]] +} + +@test "automerge: a head that moves between the reads is unreadable, and nothing is approved" { + _seed "[$(_dep_commit)]" + # The commit list read moves the head (a push lands during the reads). + printf '%s\n' "$(tt_pr_rest "$OTHER_SHA" 1)" > "${FIX_DIR}/moved_pr.json" + mv "${TT_BIN}/gh" "${TT_BIN}/gh-inner" + cat > "${TT_BIN}/gh" < "$script" + cd "$TT_WORK" || return 1 + REPO="owner/repo" PR_NUMBER=5 APP_SLUG="$APP_SLUG_FIXTURE" READ_TOKEN="read-token" GH_TOKEN="app-token" \ + run bash -eo pipefail "$script" +} + +@test "metadata failure: the token, tooling and withdrawal steps still run" { + local step + for step in "Check app secrets" "Generate app token" "Checkout Dependabot gate tooling"; do + run yq -r ".jobs.dependabot.steps[] | select(.name == \"$step\") | .if" "$REUSABLE" + [ "$output" = '${{ !cancelled() }}' ] || { echo "$step: $output"; return 1; } + done + run yq -r '.jobs.dependabot.steps[] | select(.name == "Withdraw automation when Dependabot metadata cannot be read") | .if' "$REUSABLE" + [ "$output" = "\${{ !cancelled() && steps.metadata.outcome == 'failure' }}" ] + # It comes after the steps it needs. + local w c + w="$(yq -r '.jobs.dependabot.steps | to_entries[] | select(.value.name == "Withdraw automation when Dependabot metadata cannot be read") | .key' "$REUSABLE")" + c="$(yq -r '.jobs.dependabot.steps | to_entries[] | select(.value.name == "Checkout Dependabot gate tooling") | .key' "$REUSABLE")" + [ -n "$w" ] && [ -n "$c" ] && [ "$w" -gt "$c" ] +} + +@test "metadata failure: a force-pushed foreign commit revokes approval and auto-merge, and the job fails" { + _seed "[$(tt_commit "$HEAD_SHA" mallory mallory true 1)]" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" "[$(_app_approval_of 11 "$DEP_SHA")]" + _run_metadata_failure_step + [ "$status" -ne 0 ] + [[ "$output" == *"needs a human review"* ]] + run _writes + [[ "$output" == *"pr merge 5 --repo owner/repo --disable-auto"* ]] + [[ "$output" == *"PUT repos/owner/repo/pulls/5/reviews/11/dismissals"* ]] +} + +@test "metadata failure: on Dependabot-only commits the App's own automation is withdrawn" { + _seed "[$(_dep_commit)]" + tt_fixture "repos/owner/repo/pulls/5/reviews?per_page=100" "[$(_app_approval_of 11 "$HEAD_SHA")]" + _run_metadata_failure_step + [ "$status" -ne 0 ] + [[ "$(_writes)" == *"reviews/11/dismissals"* ]] +} + @test "automerge: tooling checkout runs at the reusable's own commit, eligible or not" { run yq -r '.jobs.dependabot.steps[] | select(.name == "Checkout Dependabot gate tooling") | .with.ref' "$REUSABLE" [ "$output" = '${{ job.workflow_sha }}' ] @@ -84,7 +162,7 @@ _foreign_commits() { # earlier automation. for step in "Checkout Dependabot gate tooling" "Generate app token" "Check app secrets"; do run yq -r ".jobs.dependabot.steps[] | select(.name == \"$step\") | .if" "$REUSABLE" - [ "$output" = "null" ] + [ "$output" = '${{ !cancelled() }}' ] done } diff --git a/test/workflows/dependabot/canary-registry.bats b/test/workflows/dependabot/canary-registry.bats new file mode 100644 index 000000000..8a45644f4 --- /dev/null +++ b/test/workflows/dependabot/canary-registry.bats @@ -0,0 +1,21 @@ +#!/usr/bin/env bats +# The Dependabot reusables execute .github/scripts/dependabot/ at their pinned +# channel tag, so a change there alone must count for autocut and drift +# (canary-rollout.sh `_watched_paths` and `_autocut_signal_paths`). + +load 'helpers/setup' + +ORCH="${TT_REPO_ROOT}/scripts/canary-rollout.sh" +RINGS="${TT_REPO_ROOT}/standards/canary-rings.json" + +@test "registry: both Dependabot agents watch the gate scripts" { + local agent + for agent in dependabot-automerge dependabot-rebase; do + run env CANARY_RINGS="$RINGS" bash -c "source '$ORCH'; _watched_paths $agent" + [ "$status" -eq 0 ] + [[ "$output" == *".github/scripts/dependabot/"* ]] || { echo "$agent: $output"; return 1; } + run env CANARY_RINGS="$RINGS" bash -c "source '$ORCH'; _autocut_signal_paths $agent" + [ "$status" -eq 0 ] + [[ "$output" == *".github/scripts/dependabot/"* ]] || { echo "$agent: $output"; return 1; } + done +} diff --git a/test/workflows/dependabot/gates.bats b/test/workflows/dependabot/gates.bats index eb9b4ccd5..cb4cbfa4d 100644 --- a/test/workflows/dependabot/gates.bats +++ b/test/workflows/dependabot/gates.bats @@ -199,6 +199,47 @@ _ok_nodes() { [[ "$output" == *"1 check(s) not successful"*"gitleaks"* ]] } +# ── superseded check runs (#1399 review) ───────────────────────────────────── + +@test "checks: a cancelled run superseded by a later successful run of the same check passes" { + nodes="[$(tt_run 101 guard COMPLETED CANCELLED),$(tt_run 102 guard COMPLETED SUCCESS),$(tt_run 103 CodeQL COMPLETED SUCCESS)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 0 ] || { echo "$output"; return 1; } +} + +@test "checks: a later failing run of the same check still blocks" { + nodes="[$(tt_run 101 guard COMPLETED SUCCESS),$(tt_run 102 guard COMPLETED FAILURE)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: a later pending run of the same check still blocks" { + nodes="[$(tt_run 101 guard COMPLETED SUCCESS),$(tt_run 102 guard IN_PROGRESS '')]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: the newest run wins by databaseId, not by order in the response" { + nodes="[$(tt_run 102 guard COMPLETED SUCCESS),$(tt_run 101 guard COMPLETED CANCELLED)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 0 ] + nodes="[$(tt_run 102 guard COMPLETED FAILURE),$(tt_run 101 guard COMPLETED SUCCESS)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: the same job name in two workflows is two checks" { + nodes="[$(tt_run 101 'Lint and bats' COMPLETED FAILURE 1),$(tt_run 102 'Lint and bats' COMPLETED SUCCESS 2)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: runs without a databaseId are never collapsed" { + nodes="[$(tt_check_run guard COMPLETED CANCELLED),$(tt_check_run guard COMPLETED SUCCESS)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + # ── dependabot_provenance_gate (#1400) ────────────────────────────────────── _dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } @@ -284,17 +325,17 @@ _dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } [[ "$output" == *"no Dependabot commit"* ]] } -@test "provenance: an incomplete commit list (fewer than the PR's count) blocks" { +@test "provenance: an incomplete commit list (fewer than the PR's count) is unreadable, not a verdict" { commits="[$(_dep_commit "$HEAD_SHA")]" run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 251)" "$commits" "" - [ "$status" -eq 1 ] + [ "$status" -eq 2 ] [[ "$output" == *"cannot verify them all"* ]] } -@test "provenance: head not in the commit list blocks" { +@test "provenance: head not in the commit list is unreadable, not a verdict" { commits="[$(_dep_commit "$DEP_SHA")]" run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" - [ "$status" -eq 1 ] + [ "$status" -eq 2 ] } @test "provenance: head differing from the expected SHA blocks" { @@ -304,12 +345,18 @@ _dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } [[ "$output" == *"head moved"* ]] } -@test "provenance: empty or malformed input blocks" { +@test "provenance: empty or malformed input is unreadable, not a verdict" { run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 0)" '[]' "" - [ "$status" -eq 1 ] + [ "$status" -eq 2 ] run dependabot_provenance_gate 'nope' '[]' "" - [ "$status" -eq 1 ] + [ "$status" -eq 2 ] run dependabot_provenance_gate "$(tt_pr_rest)" 'nope' "" + [ "$status" -eq 2 ] +} + +@test "provenance: a foreign commit is still a verdict (1)" { + commits="[$(_dep_commit "$DEP_SHA"),$(tt_commit "$HEAD_SHA" mallory mallory true 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "" [ "$status" -eq 1 ] } diff --git a/test/workflows/dependabot/helpers/setup.bash b/test/workflows/dependabot/helpers/setup.bash index 9a0b1c00d..3cd32c3b2 100644 --- a/test/workflows/dependabot/helpers/setup.bash +++ b/test/workflows/dependabot/helpers/setup.bash @@ -150,6 +150,14 @@ tt_check_run() { checkSuite:{app:{slug:$a}}}' } +# tt_run ID NAME STATUS CONCLUSION [WORKFLOW_ID] [EVENT] — a CheckRun node with +# the fields used to spot superseded runs. +tt_run() { + jq -cn --argjson id "$1" --arg n "$2" --arg s "$3" --arg c "$4" --argjson w "${5:-1}" --arg e "${6:-pull_request}" \ + '{__typename:"CheckRun",databaseId:$id,name:$n,status:$s,conclusion:(if $c == "" then null else $c end), + checkSuite:{app:{slug:"github-actions"},workflowRun:{event:$e,workflow:{databaseId:$w}}}}' +} + # tt_status NAME STATE — one GraphQL StatusContext node. tt_status() { jq -cn --arg n "$1" --arg s "$2" '{__typename:"StatusContext",context:$n,state:$s}' From 539990adafb07b0f3a25003749a7e80cb943a59c Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 14:15:02 -0600 Subject: [PATCH 08/17] fix(dependabot): count a required check only as a GitHub Actions check run (#1399) pr-review at b64c2894: required checks were matched by name only, so a commit status or another app's check with the same name satisfied one. A required check now counts only as a CheckRun from the github-actions app; a run whose app cannot be read does not count. Also record the live pre-stable test the maintainer required for the accepted auto-merge window. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 2 +- .github/scripts/dependabot/lib/gates.sh | 11 ++++++--- standards/dependabot-policy.md | 13 ++++++++++ test/workflows/dependabot/gates.bats | 32 +++++++++++++++++++++++-- 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index b7f2e266b..dbf8b5f84 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -18,7 +18,7 @@ No network calls. Each gate prints a one-line reason and returns `0` (pass) or | Function | Passes only when | |----------|------------------| -| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Agent jobs (`DEPENDABOT_AGENT_JOB_RE`, GitHub Actions only, never a required check) are not counted. Every other check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present; the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). A check run superseded on the same commit (same app, workflow, event and name, lower databaseId) is not counted. | +| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Agent jobs (`DEPENDABOT_AGENT_JOB_RE`, GitHub Actions only, never a required check) are not counted. Every other check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present as a GitHub Actions check run (a commit status or another app's check of that name does not count); the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). A check run superseded on the same commit (same app, workflow, event and name, lower databaseId) is not counted. | | `dependabot_provenance_gate PR_JSON COMMITS_JSON [APP_LOGIN] [SHA]` | The full commit list was read, at least one commit is Dependabot's, and every commit has a verified signature and is either authored by `dependabot[bot]` and committed by `web-flow` or `dependabot[bot]`, or a merge commit by `APP_LOGIN` committed by `web-flow` (the App's update-branch merges). Anything else — e.g. a commit someone else pushed to the branch — blocks (#1400). Returns `2`, not `1`, when the reads cannot be trusted (no head, no commits, a commit list that does not match the PR, malformed input), so a push between two reads is never taken as a foreign commit. | | `dependabot_required_contexts RULES_JSON` | Prints the unique required status-check names from `GET /repos/{r}/rules/branches/{b}` (returns `1` on a non-array). | diff --git a/.github/scripts/dependabot/lib/gates.sh b/.github/scripts/dependabot/lib/gates.sh index 7ea10e62d..ebb1ae331 100644 --- a/.github/scripts/dependabot/lib/gates.sh +++ b/.github/scripts/dependabot/lib/gates.sh @@ -43,7 +43,8 @@ DEPENDABOT_AGENT_JOB_RE='^(dev-lead / .+|pr-auto-review / .+|pr-review / review| # Agent jobs (DEPENDABOT_AGENT_JOB_RE) are not counted. Passes only when every # other check run on the head commit COMPLETED as success, neutral or skipped, # every commit status is success, at least one such check was reported, and -# every required check name is present. Anything else — failure, cancelled, +# every required check name is present as a GitHub Actions check run (a +# commit status or another app's check of that name does not count). Anything else — failure, cancelled, # timed_out, action_required, stale, startup_failure, pending, missing, or more # contexts than one page — blocks. dependabot_checks_gate() { @@ -80,7 +81,11 @@ dependabot_checks_gate() { and (.checkSuite.app.slug? // "") == "github-actions" and (name_of | test($agent_re)) and (name_of as $n | $required | index($n) | not); - ([$nodes[] | name_of]) as $names + # A required check counts only as a CheckRun from GitHub Actions: a commit + # status, or a check from another app, with the same name must not + # satisfy it. A run whose app cannot be read does not count either. + ([$nodes[] | select(.__typename == "CheckRun" and (.checkSuite.app.slug? // "") == "github-actions") + | name_of]) as $names | ([$required[] | select(. as $r | $names | index($r) | not)]) as $missing | ([$nodes[] | select(agent_job | not)]) as $counted | (($nodes | length) - ($counted | length)) as $exempt @@ -97,7 +102,7 @@ dependabot_checks_gate() { elif ($bad | length) > 0 then "block\t\($bad | length) check(s) not successful on \($head[0:7]): \($bad | join(", "))" elif ($missing | length) > 0 then - "block\trequired check(s) not reported on \($head[0:7]): \($missing | join(", "))" + "block\trequired check(s) not reported by GitHub Actions on \($head[0:7]): \($missing | join(", "))" else "pass\tall \($counted | length) check(s) succeeded on \($head[0:7])\(if $exempt > 0 then " (\($exempt) agent job(s) not counted)" else "" end)" end diff --git a/standards/dependabot-policy.md b/standards/dependabot-policy.md index e7e5ae649..f1b09d5ca 100644 --- a/standards/dependabot-policy.md +++ b/standards/dependabot-policy.md @@ -184,6 +184,19 @@ Behavior: window, but this has not been tested end to end. The maintainer accepted this and kept native auto-merge. The direct merger is not exposed: it re-runs both gates and merges with `sha=`. +- **Required before `stable` (decided 2026-11-03):** the backstop above must be + tested live before `dependabot-automerge/v2-stable` or + `dependabot-rebase/v2-stable` moves to a release containing these gates. On a + sandbox repo with the org rulesets and the Dependabot App, take a Dependabot + PR the App has approved with auto-merge on, then push a non-Dependabot commit + to its branch twice: once with a normal token, and once with a token whose + pushes do not trigger workflows (the workflow `GITHUB_TOKEN`), so no + `synchronize` run can revoke. Confirm neither head can merge, natively or + through the merger. Record the result here; if either merges, stop the + promotion. +- **Required checks are bound to GitHub Actions:** a check the base branch's + rules require counts only as a check run from GitHub Actions. A commit status, + or a check from another app, with the same name does not satisfy it. ## Update and Merge Behind PRs Workflow diff --git a/test/workflows/dependabot/gates.bats b/test/workflows/dependabot/gates.bats index cb4cbfa4d..f9391f34d 100644 --- a/test/workflows/dependabot/gates.bats +++ b/test/workflows/dependabot/gates.bats @@ -116,8 +116,36 @@ _ok_nodes() { [[ "$output" == *"required check(s) not reported"*"SonarCloud"* ]] } -@test "checks: required checks satisfied by check runs and statuses pass" { - run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)")" '["CodeQL","CodeRabbit"]' +@test "checks: a required check is satisfied by a GitHub Actions check run" { + run dependabot_checks_gate "$(tt_pr_state "$(_ok_nodes)")" '["CodeQL"]' + [ "$status" -eq 0 ] +} + +# A required check counts only as a CheckRun from GitHub Actions: a commit status +# or another app's check with the same name must not satisfy it (#1399 review). +@test "checks: a same-name commit status does not satisfy a required check" { + nodes="[$(tt_check_run Lint COMPLETED SUCCESS),$(tt_status CodeQL SUCCESS)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '["CodeQL"]' + [ "$status" -eq 1 ] + [[ "$output" == *"required check(s) not reported by GitHub Actions"*"CodeQL"* ]] +} + +@test "checks: a same-name check from another app does not satisfy a required check" { + nodes="[$(tt_check_run Lint COMPLETED SUCCESS),$(tt_check_run CodeQL COMPLETED SUCCESS some-other-app)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '["CodeQL"]' + [ "$status" -eq 1 ] +} + +@test "checks: a required check run whose app cannot be read does not satisfy it" { + node=$(jq -cn '{__typename:"CheckRun",name:"CodeQL",status:"COMPLETED",conclusion:"SUCCESS"}') + nodes="[$(tt_check_run Lint COMPLETED SUCCESS),$node]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '["CodeQL"]' + [ "$status" -eq 1 ] +} + +@test "checks: the Actions run satisfies a required check beside a same-name run from another app" { + nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run CodeQL COMPLETED SUCCESS github-advanced-security)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '["CodeQL"]' [ "$status" -eq 0 ] } From fd7b407e9cfb95ba618f2c449d39de97eb53bef5 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 14:17:44 -0600 Subject: [PATCH 09/17] test(dependabot): pin that forged unsigned Dependabot-attributed commits are refused (#1400) Sandbox evidence (combsmith/software-factory-canary, 2026-11-03): commits made through the Contents and Git Data APIs with Dependabot's noreply email are attributed to dependabot[bot] but unsigned. Pin both shapes; dropping the verified-signature requirement fails the Git Data API case. Co-Authored-By: Claude Opus 5.5 --- test/workflows/dependabot/gates.bats | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/workflows/dependabot/gates.bats b/test/workflows/dependabot/gates.bats index f9391f34d..d3a727d29 100644 --- a/test/workflows/dependabot/gates.bats +++ b/test/workflows/dependabot/gates.bats @@ -388,6 +388,30 @@ _dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } [ "$status" -eq 1 ] } +# Sandbox evidence (combsmith/software-factory-canary, 2026-11-03): commits made +# through the Contents or Git Data API with Dependabot's noreply email are +# attributed to dependabot[bot] by GitHub, but are unsigned. Only the +# verified-signature requirement refuses them. +@test "provenance: forged Dependabot author via the Contents API (committer is the pusher, unsigned) blocks" { + commits="[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' don-petry false 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" + [ "$status" -eq 1 ] + [[ "$output" == *"not from Dependabot"*"unverified"* ]] +} + +@test "provenance: forged Dependabot author and committer via the Git Data API (unsigned) blocks" { + commits="[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' 'dependabot[bot]' false 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" + [ "$status" -eq 1 ] + [[ "$output" == *"not from Dependabot"*"unverified"* ]] +} + +@test "provenance: a forged unsigned commit on top of a genuine one blocks" { + commits="[$(tt_commit "$DEP_SHA" 'dependabot[bot]' web-flow true 1),$(tt_commit "$HEAD_SHA" 'dependabot[bot]' 'dependabot[bot]' false 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "" + [ "$status" -eq 1 ] +} + @test "provenance: a long commit list (beyond the argv size limit) is evaluated" { local i commits="" pad pad=$(printf 'x%.0s' $(seq 1 3000)) From 60cd8bb496490fc2180cf0e596d48948f7457099 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 14:31:19 -0600 Subject: [PATCH 10/17] fix(dependabot): bind required checks to the ruleset's app; scope both tokens (#1399, #1400) CodeRabbit review 5759000872: - a required check is satisfied only by a check run from the app the rule's integration_id names, or from GitHub Actions when it names none (the maintainer's fallback; true of every org rule today). The rollup query reads the app's databaseId; an unreadable app never satisfies a rule - the merger's GITHUB_TOKEN gets checks: read and statuses: read, and drops pull-requests to read (it writes nothing) - both App tokens request only what their writes need: automerge contents and pull-requests write; rebase those plus workflows write for update-branch and merges that touch workflow files Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 4 +- .github/scripts/dependabot/lib/gates.sh | 43 ++++++++++++------- .github/scripts/dependabot/lib/gh-io.sh | 2 +- .../dependabot-automerge-reusable.yml | 4 ++ .../workflows/dependabot-rebase-reusable.yml | 9 +++- standards/dependabot-policy.md | 8 ++-- .../dependabot/automerge-provenance.bats | 7 +++ test/workflows/dependabot/gates.bats | 35 ++++++++++++++- test/workflows/dependabot/helpers/setup.bash | 7 +++ .../dependabot/rebase-merge-gate.bats | 14 ++++++ 10 files changed, 108 insertions(+), 25 deletions(-) diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index dbf8b5f84..4a4d9e05e 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -18,9 +18,9 @@ No network calls. Each gate prints a one-line reason and returns `0` (pass) or | Function | Passes only when | |----------|------------------| -| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Agent jobs (`DEPENDABOT_AGENT_JOB_RE`, GitHub Actions only, never a required check) are not counted. Every other check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present as a GitHub Actions check run (a commit status or another app's check of that name does not count); the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). A check run superseded on the same commit (same app, workflow, event and name, lower databaseId) is not counted. | +| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Agent jobs (`DEPENDABOT_AGENT_JOB_RE`, GitHub Actions only, never a required check) are not counted. Every other check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present as a check run from the required app: the app the rule's `integration_id` names, or GitHub Actions when it names none (a commit status, or a run from any other app or with an unreadable app, does not count); the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). A check run superseded on the same commit (same app, workflow, event and name, lower databaseId) is not counted. | | `dependabot_provenance_gate PR_JSON COMMITS_JSON [APP_LOGIN] [SHA]` | The full commit list was read, at least one commit is Dependabot's, and every commit has a verified signature and is either authored by `dependabot[bot]` and committed by `web-flow` or `dependabot[bot]`, or a merge commit by `APP_LOGIN` committed by `web-flow` (the App's update-branch merges). Anything else — e.g. a commit someone else pushed to the branch — blocks (#1400). Returns `2`, not `1`, when the reads cannot be trusted (no head, no commits, a commit list that does not match the PR, malformed input), so a push between two reads is never taken as a foreign commit. | -| `dependabot_required_contexts RULES_JSON` | Prints the unique required status-check names from `GET /repos/{r}/rules/branches/{b}` (returns `1` on a non-array). | +| `dependabot_required_contexts RULES_JSON` | Prints the unique required checks from `GET /repos/{r}/rules/branches/{b}` as `{context, integration_id}` objects, with `integration_id` null when the rule names no app (returns `1` on a non-array). | ## `lib/gh-io.sh` — GitHub I/O diff --git a/.github/scripts/dependabot/lib/gates.sh b/.github/scripts/dependabot/lib/gates.sh index ebb1ae331..c79edc63f 100644 --- a/.github/scripts/dependabot/lib/gates.sh +++ b/.github/scripts/dependabot/lib/gates.sh @@ -32,8 +32,9 @@ DEPENDABOT_AGENT_JOB_RE='^(dev-lead / .+|pr-auto-review / .+|pr-review / review| # pageInfo{hasNextPage} nodes{CheckRun{databaseId name status # conclusion checkSuite{app{slug} workflowRun{event workflow{ # databaseId}}}} StatusContext{context state}}}}}}}. -# REQUIRED_JSON JSON array of check names the base branch's rules require -# (`[]` when none). +# REQUIRED_JSON JSON array of the checks the base branch's rules require: +# {context, integration_id} objects as dependabot_required_contexts +# prints them, or plain names (no integration_id). `[]` when none. # EXPECTED_SHA optional — block unless the PR head is this commit. # # A check run superseded on the same commit — an earlier run of the same check @@ -43,8 +44,10 @@ DEPENDABOT_AGENT_JOB_RE='^(dev-lead / .+|pr-auto-review / .+|pr-review / review| # Agent jobs (DEPENDABOT_AGENT_JOB_RE) are not counted. Passes only when every # other check run on the head commit COMPLETED as success, neutral or skipped, # every commit status is success, at least one such check was reported, and -# every required check name is present as a GitHub Actions check run (a -# commit status or another app's check of that name does not count). Anything else — failure, cancelled, +# every required check is present as a check run from the required app: the +# app the rule's integration_id names, or GitHub Actions when it names none. +# A commit status, or a run from any other app or with an unreadable app, +# does not count. Anything else — failure, cancelled, # timed_out, action_required, stale, startup_failure, pending, missing, or more # contexts than one page — blocks. dependabot_checks_gate() { @@ -80,13 +83,18 @@ dependabot_checks_gate() { def agent_job: .__typename == "CheckRun" and (.checkSuite.app.slug? // "") == "github-actions" and (name_of | test($agent_re)) - and (name_of as $n | $required | index($n) | not); - # A required check counts only as a CheckRun from GitHub Actions: a commit - # status, or a check from another app, with the same name must not - # satisfy it. A run whose app cannot be read does not count either. - ([$nodes[] | select(.__typename == "CheckRun" and (.checkSuite.app.slug? // "") == "github-actions") - | name_of]) as $names - | ([$required[] | select(. as $r | $names | index($r) | not)]) as $missing + and (name_of as $n | [$required[] | if type == "object" then .context else . end] | index($n) | not); + # A required check counts only as a CheckRun from the app the rule names + # (integration_id), or from GitHub Actions when it names none. A commit + # status, or a run from any other app or with an unreadable app, with the + # same name does not satisfy it. + def satisfies($r): + .__typename == "CheckRun" and name_of == $r.context + and (if ($r.integration_id | type) == "number" + then (.checkSuite.app.databaseId? // null) == $r.integration_id + else (.checkSuite.app.slug? // "") == "github-actions" end); + ([$required[] | if type == "object" then . else {context: ., integration_id: null} end]) as $req + | ([$req[] | . as $r | select([$nodes[] | satisfies($r)] | any | not) | .context]) as $missing | ([$nodes[] | select(agent_job | not)]) as $counted | (($nodes | length) - ($counted | length)) as $exempt | ([$counted[] | select(ok | not) | "\(name_of) (\(outcome))"]) as $bad @@ -102,7 +110,7 @@ dependabot_checks_gate() { elif ($bad | length) > 0 then "block\t\($bad | length) check(s) not successful on \($head[0:7]): \($bad | join(", "))" elif ($missing | length) > 0 then - "block\trequired check(s) not reported by GitHub Actions on \($head[0:7]): \($missing | join(", "))" + "block\trequired check(s) not reported by the required app on \($head[0:7]): \($missing | join(", "))" else "pass\tall \($counted | length) check(s) succeeded on \($head[0:7])\(if $exempt > 0 then " (\($exempt) agent job(s) not counted)" else "" end)" end @@ -205,15 +213,18 @@ dependabot_provenance_gate() { # dependabot_required_contexts RULES_JSON # RULES_JSON REST `GET /repos/{r}/rules/branches/{b}` — a flat array of # rules or the array of pages `--paginate --slurp` produces. -# Prints a JSON array of the unique required status-check names. Returns 1 -# (printing nothing) when the input is not an array. +# Prints a JSON array of the unique required checks as {context, +# integration_id} (null when the rule names no app). Returns 1 (printing +# nothing) when the input is not an array. dependabot_required_contexts() { local rules_json="$1" out out=$(jq -c ' if type != "array" then error("not an array") else . end | [flatten[] | select(type == "object" and .type == "required_status_checks") - | (.parameters.required_status_checks // [])[] | .context - | select(type == "string" and . != "")] | unique + | (.parameters.required_status_checks // [])[] + | select((.context | type) == "string" and .context != "") + | {context, integration_id: (if (.integration_id | type) == "number" then .integration_id else null end)}] + | unique ' <<<"$rules_json" 2>/dev/null) || return 1 [[ -n "$out" ]] || return 1 echo "$out" diff --git a/.github/scripts/dependabot/lib/gh-io.sh b/.github/scripts/dependabot/lib/gh-io.sh index 4a9c7b3b8..e6671c8c3 100644 --- a/.github/scripts/dependabot/lib/gh-io.sh +++ b/.github/scripts/dependabot/lib/gh-io.sh @@ -16,7 +16,7 @@ dependabot_fetch_pr_state() { local repo="$1" pr="$2" out # shellcheck disable=SC2016 # GraphQL variables, not shell expansions - local query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){headRefOid baseRefName mergeable autoMergeRequest{enabledAt} commits(last:1){nodes{commit{oid statusCheckRollup{contexts(first:100){pageInfo{hasNextPage} nodes{__typename ... on CheckRun{databaseId name status conclusion checkSuite{app{slug} workflowRun{event workflow{databaseId}}}} ... on StatusContext{context state}}}}}}}}}}' + local query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){headRefOid baseRefName mergeable autoMergeRequest{enabledAt} commits(last:1){nodes{commit{oid statusCheckRollup{contexts(first:100){pageInfo{hasNextPage} nodes{__typename ... on CheckRun{databaseId name status conclusion checkSuite{app{slug databaseId} workflowRun{event workflow{databaseId}}}} ... on StatusContext{context state}}}}}}}}}}' out=$(gh api graphql -f query="$query" -f owner="${repo%%/*}" \ -f name="${repo#*/}" -F number="$pr" 2>/dev/null) || return 1 diff --git a/.github/workflows/dependabot-automerge-reusable.yml b/.github/workflows/dependabot-automerge-reusable.yml index 595efe43f..4aa856d41 100644 --- a/.github/workflows/dependabot-automerge-reusable.yml +++ b/.github/workflows/dependabot-automerge-reusable.yml @@ -99,6 +99,10 @@ jobs: with: app-id: ${{ secrets.APP_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} + # Only what approving, dismissing approvals and turning auto-merge on + # or off need (auto-merge requires the right to merge: contents). + permission-pull-requests: write + permission-contents: write - name: Checkout Dependabot gate tooling if: ${{ !cancelled() }} diff --git a/.github/workflows/dependabot-rebase-reusable.yml b/.github/workflows/dependabot-rebase-reusable.yml index 5ee2535d9..f2a668f0d 100644 --- a/.github/workflows/dependabot-rebase-reusable.yml +++ b/.github/workflows/dependabot-rebase-reusable.yml @@ -69,7 +69,9 @@ jobs: runs-on: ubuntu-latest permissions: contents: read # check out the gate scripts from combsmith/.github - pull-requests: write # list Dependabot PRs and read their state with GITHUB_TOKEN + pull-requests: read # list Dependabot PRs and read their state with GITHUB_TOKEN + checks: read # the merge gate reads every check run on the head commit + statuses: read # ...and every commit status steps: - name: Checkout Dependabot gate tooling uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -97,6 +99,11 @@ jobs: with: app-id: ${{ secrets.APP_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} + # Only what update-branch and the merge need. workflows: write lets + # a Dependabot bump of a workflow file be merged or updated. + permission-contents: write + permission-pull-requests: write + permission-workflows: write - name: Update and merge Dependabot PRs env: diff --git a/standards/dependabot-policy.md b/standards/dependabot-policy.md index f1b09d5ca..3525c522b 100644 --- a/standards/dependabot-policy.md +++ b/standards/dependabot-policy.md @@ -194,9 +194,11 @@ Behavior: `synchronize` run can revoke. Confirm neither head can merge, natively or through the merger. Record the result here; if either merges, stop the promotion. -- **Required checks are bound to GitHub Actions:** a check the base branch's - rules require counts only as a check run from GitHub Actions. A commit status, - or a check from another app, with the same name does not satisfy it. +- **Required checks are bound to their app:** a check the base branch's rules + require counts only as a check run from the app the rule names + (`integration_id`), or from GitHub Actions when the rule names none, which is + the case for every rule in the org rulesets today. A commit status, or a run + from any other app, with the same name does not satisfy it. ## Update and Merge Behind PRs Workflow diff --git a/test/workflows/dependabot/automerge-provenance.bats b/test/workflows/dependabot/automerge-provenance.bats index 5e6c15cdd..e130cc31e 100644 --- a/test/workflows/dependabot/automerge-provenance.bats +++ b/test/workflows/dependabot/automerge-provenance.bats @@ -155,6 +155,13 @@ _run_metadata_failure_step() { [[ "$(_writes)" == *"reviews/11/dismissals"* ]] } +@test "automerge: the App token asks only for what approve, auto-merge and dismiss need" { + local step='.jobs.dependabot.steps[] | select(.id == "app-token") | .with' + [ "$(yq -r "$step | .\"permission-pull-requests\"" "$REUSABLE")" = "write" ] + [ "$(yq -r "$step | .\"permission-contents\"" "$REUSABLE")" = "write" ] + [ "$(yq -r "$step | keys | map(select(test(\"^permission-\"))) | length" "$REUSABLE")" = "2" ] +} + @test "automerge: tooling checkout runs at the reusable's own commit, eligible or not" { run yq -r '.jobs.dependabot.steps[] | select(.name == "Checkout Dependabot gate tooling") | .with.ref' "$REUSABLE" [ "$output" = '${{ job.workflow_sha }}' ] diff --git a/test/workflows/dependabot/gates.bats b/test/workflows/dependabot/gates.bats index d3a727d29..cc24fad30 100644 --- a/test/workflows/dependabot/gates.bats +++ b/test/workflows/dependabot/gates.bats @@ -127,7 +127,7 @@ _ok_nodes() { nodes="[$(tt_check_run Lint COMPLETED SUCCESS),$(tt_status CodeQL SUCCESS)]" run dependabot_checks_gate "$(tt_pr_state "$nodes")" '["CodeQL"]' [ "$status" -eq 1 ] - [[ "$output" == *"required check(s) not reported by GitHub Actions"*"CodeQL"* ]] + [[ "$output" == *"required check(s) not reported by the required app"*"CodeQL"* ]] } @test "checks: a same-name check from another app does not satisfy a required check" { @@ -143,6 +143,37 @@ _ok_nodes() { [ "$status" -eq 1 ] } +# A rule that names an app (integration_id) is satisfied only by that app's run; +# a rule that names none falls back to GitHub Actions only (#1399 review). +@test "checks: a required check with an integration_id is satisfied by that app's run" { + nodes="[$(tt_check_run Lint COMPLETED SUCCESS),$(tt_app_run CodeQL github-advanced-security 57789)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[{"context":"CodeQL","integration_id":57789}]' + [ "$status" -eq 0 ] || { echo "$output"; return 1; } +} + +@test "checks: a required check with an integration_id is not satisfied by another app, Actions included" { + nodes="[$(tt_check_run Lint COMPLETED SUCCESS),$(tt_app_run CodeQL github-actions 15368)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[{"context":"CodeQL","integration_id":57789}]' + [ "$status" -eq 1 ] + nodes="[$(tt_check_run Lint COMPLETED SUCCESS),$(tt_status CodeQL SUCCESS)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[{"context":"CodeQL","integration_id":57789}]' + [ "$status" -eq 1 ] +} + +@test "checks: a required check with an integration_id is not satisfied when the app id cannot be read" { + nodes="[$(tt_check_run Lint COMPLETED SUCCESS),$(tt_check_run CodeQL COMPLETED SUCCESS github-advanced-security)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[{"context":"CodeQL","integration_id":57789}]' + [ "$status" -eq 1 ] +} + +@test "checks: a required check with no integration_id falls back to GitHub Actions only" { + nodes="[$(tt_check_run Lint COMPLETED SUCCESS),$(tt_app_run CodeQL github-advanced-security 57789)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[{"context":"CodeQL","integration_id":null}]' + [ "$status" -eq 1 ] + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[{"context":"CodeQL"}]' + [ "$status" -eq 1 ] +} + @test "checks: the Actions run satisfies a required check beside a same-name run from another app" { nodes="[$(tt_check_run CodeQL COMPLETED SUCCESS),$(tt_check_run CodeQL COMPLETED SUCCESS github-advanced-security)]" run dependabot_checks_gate "$(tt_pr_state "$nodes")" '["CodeQL"]' @@ -429,7 +460,7 @@ _dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } rules='[[{"type":"required_status_checks","parameters":{"required_status_checks":[{"context":"CodeQL"},{"context":"SonarCloud"}]}},{"type":"pull_request","parameters":{}}],[{"type":"required_status_checks","parameters":{"required_status_checks":[{"context":"CodeQL","integration_id":1}]}}]]' run dependabot_required_contexts "$rules" [ "$status" -eq 0 ] - [ "$output" = '["CodeQL","SonarCloud"]' ] + [ "$output" = '[{"context":"CodeQL","integration_id":null},{"context":"CodeQL","integration_id":1},{"context":"SonarCloud","integration_id":null}]' ] } @test "required: no rules yields an empty list" { diff --git a/test/workflows/dependabot/helpers/setup.bash b/test/workflows/dependabot/helpers/setup.bash index 3cd32c3b2..5f4b3bbfe 100644 --- a/test/workflows/dependabot/helpers/setup.bash +++ b/test/workflows/dependabot/helpers/setup.bash @@ -143,6 +143,13 @@ STUB # ── fixture builders ───────────────────────────────────────────────────────── +# tt_app_run NAME APP_SLUG APP_ID — a successful CheckRun from a given app. +tt_app_run() { + jq -cn --arg n "$1" --arg a "$2" --argjson id "$3" \ + '{__typename:"CheckRun",name:$n,status:"COMPLETED",conclusion:"SUCCESS", + checkSuite:{app:{slug:$a,databaseId:$id}}}' +} + # tt_check_run NAME STATUS CONCLUSION [APP_SLUG] — one GraphQL CheckRun node. tt_check_run() { jq -cn --arg n "$1" --arg s "$2" --arg c "$3" --arg a "${4:-github-actions}" \ diff --git a/test/workflows/dependabot/rebase-merge-gate.bats b/test/workflows/dependabot/rebase-merge-gate.bats index 26696079b..e42b38027 100644 --- a/test/workflows/dependabot/rebase-merge-gate.bats +++ b/test/workflows/dependabot/rebase-merge-gate.bats @@ -44,6 +44,20 @@ _rules_requiring() { # NAME... printf '[{"type":"required_status_checks","parameters":{"strict_required_status_checks_policy":false,"required_status_checks":%s}}]' "$ctx" } +@test "rebase: GITHUB_TOKEN can read checks and statuses, and writes nothing" { + [ "$(yq -r '.jobs."update-and-merge".permissions.checks' "$REUSABLE")" = "read" ] + [ "$(yq -r '.jobs."update-and-merge".permissions.statuses' "$REUSABLE")" = "read" ] + [ "$(yq -r '.jobs."update-and-merge".permissions."pull-requests"' "$REUSABLE")" = "read" ] +} + +@test "rebase: the App token asks only for what update-branch and merge need" { + local step='.jobs."update-and-merge".steps[] | select(.id == "app-token") | .with' + [ "$(yq -r "$step | .\"permission-contents\"" "$REUSABLE")" = "write" ] + [ "$(yq -r "$step | .\"permission-pull-requests\"" "$REUSABLE")" = "write" ] + [ "$(yq -r "$step | .\"permission-workflows\"" "$REUSABLE")" = "write" ] + [ "$(yq -r "$step | keys | map(select(test(\"^permission-\"))) | length" "$REUSABLE")" = "3" ] +} + _merge_calls() { grep -F '/pulls/5/merge' "${FIX_DIR}/writes.log" 2>/dev/null || true; } @test "rebase: run-block exists and sources the gate libraries" { From 4362311be09d2a2cb97c3ff55235d802b6a855b6 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 15:40:14 -0600 Subject: [PATCH 11/17] fix(dependabot): do not request workflows permission the App lacks (#1399) The Dependabot App installation has no workflows permission, so requesting permission-workflows: write would make create-github-app-token fail. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/dependabot-rebase-reusable.yml | 5 ++--- test/workflows/dependabot/rebase-merge-gate.bats | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/dependabot-rebase-reusable.yml b/.github/workflows/dependabot-rebase-reusable.yml index f2a668f0d..9e60951ef 100644 --- a/.github/workflows/dependabot-rebase-reusable.yml +++ b/.github/workflows/dependabot-rebase-reusable.yml @@ -99,11 +99,10 @@ jobs: with: app-id: ${{ secrets.APP_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} - # Only what update-branch and the merge need. workflows: write lets - # a Dependabot bump of a workflow file be merged or updated. + # Only what update-branch and the merge need. The App has no + # workflows permission, and requesting one it lacks fails the token. permission-contents: write permission-pull-requests: write - permission-workflows: write - name: Update and merge Dependabot PRs env: diff --git a/test/workflows/dependabot/rebase-merge-gate.bats b/test/workflows/dependabot/rebase-merge-gate.bats index e42b38027..7ba5c5b5f 100644 --- a/test/workflows/dependabot/rebase-merge-gate.bats +++ b/test/workflows/dependabot/rebase-merge-gate.bats @@ -54,8 +54,7 @@ _rules_requiring() { # NAME... local step='.jobs."update-and-merge".steps[] | select(.id == "app-token") | .with' [ "$(yq -r "$step | .\"permission-contents\"" "$REUSABLE")" = "write" ] [ "$(yq -r "$step | .\"permission-pull-requests\"" "$REUSABLE")" = "write" ] - [ "$(yq -r "$step | .\"permission-workflows\"" "$REUSABLE")" = "write" ] - [ "$(yq -r "$step | keys | map(select(test(\"^permission-\"))) | length" "$REUSABLE")" = "3" ] + [ "$(yq -r "$step | keys | map(select(test(\"^permission-\"))) | length" "$REUSABLE")" = "2" ] } _merge_calls() { grep -F '/pulls/5/merge' "${FIX_DIR}/writes.log" 2>/dev/null || true; } From 831dcc7f60a693270bcabf7e437f60d2077df12c Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:17:40 -0600 Subject: [PATCH 12/17] fix(dependabot): read the merge gate with the App token, not GITHUB_TOKEN (#1399) The rebase reusable asked GITHUB_TOKEN for checks: read and statuses: read, which neither caller stub grants, so the job would fail at startup fleet-wide once promoted. The merge gate now reads check runs, statuses, rules and commits with the App installation token, minted with checks: read and statuses: read on top of contents/pull-requests write; GITHUB_TOKEN is back to contents: read and pull-requests: read, which the callers grant. Until the App has checks: read and statuses: read the token step fails and a follow-up step names the missing permissions; the run block also refuses to start without an App token. A new caller-permissions.bats pins that every permission each Dependabot reusable requests is granted by this repo's stubs and the standards templates. Co-Authored-By: Claude Opus 5.5 --- .../workflows/dependabot-rebase-reusable.yml | 34 +++++++--- standards/dependabot-policy.md | 21 ++++-- .../dependabot/caller-permissions.bats | 66 +++++++++++++++++++ .../dependabot/rebase-merge-gate.bats | 35 ++++++++-- 4 files changed, 134 insertions(+), 22 deletions(-) create mode 100644 test/workflows/dependabot/caller-permissions.bats diff --git a/.github/workflows/dependabot-rebase-reusable.yml b/.github/workflows/dependabot-rebase-reusable.yml index 9e60951ef..b9bf63c3c 100644 --- a/.github/workflows/dependabot-rebase-reusable.yml +++ b/.github/workflows/dependabot-rebase-reusable.yml @@ -48,7 +48,10 @@ # branch keeps the PR out of the automated merge. # # Required org/repo secrets (passed explicitly by caller via secrets: mapping): -# APP_ID — GitHub App ID with contents:write and pull-requests:write +# APP_ID — GitHub App ID with contents:write, pull-requests:write, +# checks:read and statuses:read (the merge gate reads every +# check run and commit status with the App token, so the +# callers' GITHUB_TOKEN grants need not change) # APP_PRIVATE_KEY — GitHub App private key name: Dependabot update and merge (Reusable) @@ -56,7 +59,7 @@ on: workflow_call: secrets: APP_ID: - description: "GitHub App ID with contents:write and pull-requests:write" + description: "GitHub App ID with contents:write, pull-requests:write, checks:read and statuses:read" required: true APP_PRIVATE_KEY: description: "GitHub App private key" @@ -69,9 +72,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read # check out the gate scripts from combsmith/.github - pull-requests: read # list Dependabot PRs and read their state with GITHUB_TOKEN - checks: read # the merge gate reads every check run on the head commit - statuses: read # ...and every commit status + pull-requests: read # list Dependabot PRs with GITHUB_TOKEN; the merge gate reads with the App token steps: - name: Checkout Dependabot gate tooling uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -99,10 +100,20 @@ jobs: with: app-id: ${{ secrets.APP_ID }} private-key: ${{ secrets.APP_PRIVATE_KEY }} - # Only what update-branch and the merge need. The App has no - # workflows permission, and requesting one it lacks fails the token. + # Only what update-branch, the merge and the merge gate need: the + # gate reads every check run and commit status with this token, as + # callers grant GITHUB_TOKEN no checks or statuses scope. Requesting + # a permission the App lacks fails this step (no workflows here). permission-contents: write permission-pull-requests: write + permission-checks: read + permission-statuses: read + + - name: Explain a failed app token + if: ${{ failure() && steps.app-token.outcome == 'failure' }} + run: | + echo "::error::Could not mint the App token with contents:write, pull-requests:write, checks:read and statuses:read. If the App lacks checks:read or statuses:read, grant both (App settings → Permissions → Repository) and accept the installation update; the merge gate cannot read check runs or commit statuses without them. See standards/dependabot-policy.md" + exit 1 - name: Update and merge Dependabot PRs env: @@ -119,6 +130,10 @@ jobs: # Branch updates this App makes (update-branch merge commits) are the # only non-Dependabot commits the merge gate accepts. APP_LOGIN="${APP_SLUG:+${APP_SLUG}[bot]}" + if [[ -z "$APP_TOKEN" ]]; then + echo "::error::No App token: the merge gate cannot read check runs or commit statuses without it." + exit 1 + fi # Find open Dependabot PRs PRS=$(gh pr list --repo "$REPO" --author "app/dependabot" \ @@ -182,8 +197,9 @@ jobs: # head commit successful (fail closed on failure, cancelled, # timed_out, action_required, stale, startup_failure, pending or a # missing required check), and every commit from Dependabot or a - # branch update by this App (#1399, #1400). - if ! dependabot_merge_gate "$REPO" "$PR_NUMBER" "$APP_LOGIN"; then + # branch update by this App (#1399, #1400). It reads with the App + # token, which has checks:read and statuses:read. + if ! GH_TOKEN="$APP_TOKEN" dependabot_merge_gate "$REPO" "$PR_NUMBER" "$APP_LOGIN"; then echo " Skipping — $DEPENDABOT_GATE_REASON" continue fi diff --git a/standards/dependabot-policy.md b/standards/dependabot-policy.md index 3525c522b..fe74541c7 100644 --- a/standards/dependabot-policy.md +++ b/standards/dependabot-policy.md @@ -199,6 +199,12 @@ Behavior: (`integration_id`), or from GitHub Actions when the rule names none, which is the case for every rule in the org rulesets today. A commit status, or a run from any other app, with the same name does not satisfy it. +- **Required before `dependabot-rebase` is promoted:** the Dependabot App must + have `checks: read` and `statuses: read`. The merger reads every check run and + commit status with the App token, so caller stubs keep granting only + `contents: write` and `pull-requests: write`. Without them the token cannot be + minted and the job stops with an error naming the missing permissions; no PR + is merged. ## Update and Merge Behind PRs Workflow @@ -374,12 +380,15 @@ The workflow fails if any known vulnerability is found, blocking the PR from mer > [CODEOWNERS Standard](codeowners-standard.md). The earlier approach of > using `gh api .../merge` as a bypass was fragile and has been superseded. 4. Add `workflows/dependency-audit.yml` to `.github/workflows/`. -5. **GitHub App permissions** — the `dependabot-automerge-petry` GitHub App requires - the `workflows` permission in addition to `contents: write` and `pull_requests: write`. - Without it, the rebase workflow cannot call `update-branch` when `main` contains - workflow file changes. Grant: GitHub App settings → Permissions → Repository → - Workflows → Read & Write, then accept the updated permission in each repo's - installed-app settings. +5. **GitHub App permissions** — the Dependabot GitHub App requires + `contents: write`, `pull_requests: write`, `checks: read` and `statuses: read`. + The rebase workflow's merge gate reads check runs and commit statuses with the + App token; without `checks: read` and `statuses: read` it stops with an error. + Grant: GitHub App settings → Permissions → Repository, then accept the updated + permissions in the org's installed-app settings. The rebase workflow does not + request `workflows`, so `update-branch` cannot bring in workflow file changes + from `main`; it falls back to a direct merge when strict up-to-date checks + are off. 6. **GitHub App secrets** — `APP_ID` and `APP_PRIVATE_KEY` are managed at the **organization level** (`gh secret set --org combsmith --visibility all`), diff --git a/test/workflows/dependabot/caller-permissions.bats b/test/workflows/dependabot/caller-permissions.bats new file mode 100644 index 000000000..d08b9b181 --- /dev/null +++ b/test/workflows/dependabot/caller-permissions.bats @@ -0,0 +1,66 @@ +#!/usr/bin/env bats +# A reusable workflow's jobs cannot get more GITHUB_TOKEN permissions than the +# calling job grants; asking for more fails the run at startup. Pin that every +# permission the Dependabot reusables request is granted by each caller: this +# repo's own stubs and the standards templates copied to every org repo +# (#1399 review). + +load 'helpers/setup' + +WF="${TT_REPO_ROOT}/.github/workflows" +STD="${TT_REPO_ROOT}/standards/workflows" + +setup() { + tt_setup +} + +teardown() { + tt_teardown +} + +# _assert_caller_grants CALLER — every permission each job of the reusable the +# caller's job uses requests is granted by that job (write covers read). +_assert_caller_grants() { + local caller="$1" job uses reusable granted requested scope level have + for job in $(yq -r '.jobs | keys | .[]' "$caller"); do + uses=$(yq -r ".jobs.\"${job}\".uses // \"\"" "$caller") + [[ "$uses" == *dependabot-*-reusable.yml* ]] || continue + reusable="${WF}/$(basename "${uses%%@*}")" + [ -f "$reusable" ] || { echo "$caller: $uses not found"; return 1; } + granted=$(yq -o=json -I0 ".jobs.\"${job}\".permissions // {}" "$caller") + requested=$(yq -o=json -I0 '[.jobs[].permissions // {} | to_entries[]]' "$reusable") + while IFS=$'\t' read -r scope level; do + [[ -n "$scope" ]] || continue + have=$(jq -r --arg s "$scope" '.[$s] // "none"' <<<"$granted") + if [[ "$level" == "write" && "$have" != "write" ]] \ + || [[ "$level" == "read" && "$have" != "read" && "$have" != "write" ]]; then + echo "$caller grants ${scope}: ${have}; $(basename "$reusable") requests ${scope}: ${level}" + return 1 + fi + done < <(jq -r '.[] | [.key, .value] | @tsv' <<<"$requested") + done +} + +@test "callers: this repo's dependabot-rebase stub grants what the reusable requests" { + _assert_caller_grants "${WF}/dependabot-rebase.yml" +} + +@test "callers: the dependabot-rebase template grants what the reusable requests" { + _assert_caller_grants "${STD}/dependabot-rebase.yml" +} + +@test "callers: this repo's dependabot-automerge stub grants what the reusable requests" { + _assert_caller_grants "${WF}/dependabot-automerge.yml" +} + +@test "callers: the dependabot-automerge template grants what the reusable requests" { + _assert_caller_grants "${STD}/dependabot-automerge.yml" +} + +@test "callers: the check catches a reusable that asks for more than its caller grants" { + local fake="${TT_TMP}/caller.yml" + printf 'jobs:\n r:\n permissions:\n contents: read\n uses: ./.github/workflows/dependabot-rebase-reusable.yml\n' > "$fake" + run _assert_caller_grants "$fake" + [ "$status" -eq 1 ] + [[ "$output" == *"requests pull-requests: read"* ]] +} diff --git a/test/workflows/dependabot/rebase-merge-gate.bats b/test/workflows/dependabot/rebase-merge-gate.bats index 7ba5c5b5f..896c4802a 100644 --- a/test/workflows/dependabot/rebase-merge-gate.bats +++ b/test/workflows/dependabot/rebase-merge-gate.bats @@ -44,17 +44,36 @@ _rules_requiring() { # NAME... printf '[{"type":"required_status_checks","parameters":{"strict_required_status_checks_policy":false,"required_status_checks":%s}}]' "$ctx" } -@test "rebase: GITHUB_TOKEN can read checks and statuses, and writes nothing" { - [ "$(yq -r '.jobs."update-and-merge".permissions.checks' "$REUSABLE")" = "read" ] - [ "$(yq -r '.jobs."update-and-merge".permissions.statuses' "$REUSABLE")" = "read" ] +@test "rebase: GITHUB_TOKEN asks for no checks or statuses scope, and writes nothing" { + [ "$(yq -r '.jobs."update-and-merge".permissions | keys | sort | join(",")' "$REUSABLE")" = "contents,pull-requests" ] + [ "$(yq -r '.jobs."update-and-merge".permissions.contents' "$REUSABLE")" = "read" ] [ "$(yq -r '.jobs."update-and-merge".permissions."pull-requests"' "$REUSABLE")" = "read" ] } -@test "rebase: the App token asks only for what update-branch and merge need" { +@test "rebase: the App token asks only for what update-branch, merge and the gate need" { local step='.jobs."update-and-merge".steps[] | select(.id == "app-token") | .with' [ "$(yq -r "$step | .\"permission-contents\"" "$REUSABLE")" = "write" ] [ "$(yq -r "$step | .\"permission-pull-requests\"" "$REUSABLE")" = "write" ] - [ "$(yq -r "$step | keys | map(select(test(\"^permission-\"))) | length" "$REUSABLE")" = "2" ] + [ "$(yq -r "$step | .\"permission-checks\"" "$REUSABLE")" = "read" ] + [ "$(yq -r "$step | .\"permission-statuses\"" "$REUSABLE")" = "read" ] + [ "$(yq -r "$step | keys | map(select(test(\"^permission-\"))) | length" "$REUSABLE")" = "4" ] +} + +@test "rebase: a token that cannot be minted stops the job with the missing App permissions named" { + local step='.jobs."update-and-merge".steps[] | select(.name == "Explain a failed app token")' + [ "$(yq -r "$step | .if" "$REUSABLE")" = "\${{ failure() && steps.app-token.outcome == 'failure' }}" ] + run yq -r "$step | .run" "$REUSABLE" + [[ "$output" == *"checks:read and statuses:read"* ]] + [[ "$output" == *"exit 1"* ]] +} + +@test "rebase: no App token stops the run before any read" { + cd "$TT_WORK" || return 1 + GH_TOKEN="read-token" APP_TOKEN="" APP_SLUG="$APP_SLUG_FIXTURE" \ + REPO="owner/repo" run bash -eo pipefail "$RUN_SCRIPT" + [ "$status" -eq 1 ] + [[ "$output" == *"No App token"* ]] + [ ! -s "${FIX_DIR}/calls.log" ] } _merge_calls() { grep -F '/pulls/5/merge' "${FIX_DIR}/writes.log" 2>/dev/null || true; } @@ -187,11 +206,13 @@ _merge_calls() { grep -F '/pulls/5/merge' "${FIX_DIR}/writes.log" 2>/dev/null || [[ "$output" == *"Warning: failed to merge PR #5"* ]] } -@test "rebase: gate reads use the job token, only the merge uses the App token" { +@test "rebase: the merge gate reads with the App token (checks, statuses, rules, commits)" { tt_seed_pr "$(tt_pr_state "$(_green_nodes)")" "$(_dependabot_only)" _run_merge_step [ "$status" -eq 0 ] run grep -E 'graphql|/rules/|/commits' "${FIX_DIR}/calls.log" [ "$status" -eq 0 ] - ! printf '%s\n' "$output" | grep -v '^TOKEN=read-token ' + ! printf '%s\n' "$output" | grep -v '^TOKEN=app-token ' + # Listing the PRs and comparing branches stay on GITHUB_TOKEN. + grep -E '^TOKEN=read-token .*pr list' "${FIX_DIR}/calls.log" } From cc8abca31946143e14f4137f73dd2fcba335d177 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:28:29 -0600 Subject: [PATCH 13/17] style(dependabot): wrap the failed-token error under the 200-column lint limit (#1399) Co-Authored-By: Claude Opus 5.5 --- .github/workflows/dependabot-rebase-reusable.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dependabot-rebase-reusable.yml b/.github/workflows/dependabot-rebase-reusable.yml index b9bf63c3c..f1f1a96de 100644 --- a/.github/workflows/dependabot-rebase-reusable.yml +++ b/.github/workflows/dependabot-rebase-reusable.yml @@ -112,7 +112,11 @@ jobs: - name: Explain a failed app token if: ${{ failure() && steps.app-token.outcome == 'failure' }} run: | - echo "::error::Could not mint the App token with contents:write, pull-requests:write, checks:read and statuses:read. If the App lacks checks:read or statuses:read, grant both (App settings → Permissions → Repository) and accept the installation update; the merge gate cannot read check runs or commit statuses without them. See standards/dependabot-policy.md" + msg="Could not mint the App token with contents:write, pull-requests:write, checks:read and statuses:read." + msg+=" If the App lacks checks:read or statuses:read, grant both (App settings → Permissions → Repository)" + msg+=" and accept the installation update; the merge gate cannot read check runs or commit statuses without them." + msg+=" See standards/dependabot-policy.md" + echo "::error::${msg}" exit 1 - name: Update and merge Dependabot PRs From 64497f8d480797e702fb636dec3bc57d308ff87f Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:35:23 -0600 Subject: [PATCH 14/17] test(dependabot): make the token-split assertions fail when they should (#1399) A negated pipeline does not trip bats' set -e, so a read with the wrong token passed whenever another command followed it. Assert on the output instead. Co-Authored-By: Claude Opus 5.5 --- test/workflows/dependabot/automerge-provenance.bats | 2 +- test/workflows/dependabot/rebase-merge-gate.bats | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/workflows/dependabot/automerge-provenance.bats b/test/workflows/dependabot/automerge-provenance.bats index e130cc31e..3fed995a9 100644 --- a/test/workflows/dependabot/automerge-provenance.bats +++ b/test/workflows/dependabot/automerge-provenance.bats @@ -379,5 +379,5 @@ HOOK [ "$status" -eq 0 ] run grep -E 'api repos/owner/repo/pulls/5($| |/commits)' "${FIX_DIR}/calls.log" [ "$status" -eq 0 ] - ! printf '%s\n' "$output" | grep -v '^TOKEN=read-token ' + [ -z "$(printf '%s\n' "$output" | grep -v '^TOKEN=read-token ')" ] } diff --git a/test/workflows/dependabot/rebase-merge-gate.bats b/test/workflows/dependabot/rebase-merge-gate.bats index 896c4802a..186d862cf 100644 --- a/test/workflows/dependabot/rebase-merge-gate.bats +++ b/test/workflows/dependabot/rebase-merge-gate.bats @@ -212,7 +212,7 @@ _merge_calls() { grep -F '/pulls/5/merge' "${FIX_DIR}/writes.log" 2>/dev/null || [ "$status" -eq 0 ] run grep -E 'graphql|/rules/|/commits' "${FIX_DIR}/calls.log" [ "$status" -eq 0 ] - ! printf '%s\n' "$output" | grep -v '^TOKEN=app-token ' + [ -z "$(printf '%s\n' "$output" | grep -v '^TOKEN=app-token ')" ] # Listing the PRs and comparing branches stay on GITHUB_TOKEN. grep -E '^TOKEN=read-token .*pr list' "${FIX_DIR}/calls.log" } From 5cc84ac701eec0e7617499cf26102706bc01a20e Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:30:30 -0600 Subject: [PATCH 15/17] fix(dependabot): collapse superseded check runs only when unambiguous (#1399) The collapse grouped runs by (app, workflow, event, name) with missing parts read as "", so two different checks with an unreadable workflow, event or app, or another app's checks with no workflow run, were merged and only the newest counted, dropping a failure. A tie on databaseId or a newer SKIPPED/NEUTRAL run could also hide an older failure. Older runs are now dropped only when the whole key is readable and the newest run is unique and SUCCESS; otherwise every run of the check counts, so any failure among them holds the merge. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 2 +- .github/scripts/dependabot/lib/gates.sh | 38 +++++++++++++++------ test/workflows/dependabot/gates.bats | 45 +++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index 4a4d9e05e..88357e9e4 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -18,7 +18,7 @@ No network calls. Each gate prints a one-line reason and returns `0` (pass) or | Function | Passes only when | |----------|------------------| -| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Agent jobs (`DEPENDABOT_AGENT_JOB_RE`, GitHub Actions only, never a required check) are not counted. Every other check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present as a check run from the required app: the app the rule's `integration_id` names, or GitHub Actions when it names none (a commit status, or a run from any other app or with an unreadable app, does not count); the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). A check run superseded on the same commit (same app, workflow, event and name, lower databaseId) is not counted. | +| `dependabot_checks_gate PR_JSON REQUIRED_JSON [SHA]` | Agent jobs (`DEPENDABOT_AGENT_JOB_RE`, GitHub Actions only, never a required check) are not counted. Every other check run on the head commit `COMPLETED` as `success`, `neutral` or `skipped`; every commit status is `success`; at least one check reported; every check the base branch's rules require is present as a check run from the required app: the app the rule's `integration_id` names, or GitHub Actions when it names none (a commit status, or a run from any other app or with an unreadable app, does not count); the rollup is for the head commit (and the head is `SHA` when given). Failure, cancelled, timed_out, action_required, stale, startup_failure, pending, missing, or more than one page of checks all block (#1399). A check run superseded on the same commit (same app, workflow, event and name, lower databaseId) is not counted, but only when that whole key can be read and the newest run is unique and `success`; otherwise every run of the check counts, so any failure among them blocks. | | `dependabot_provenance_gate PR_JSON COMMITS_JSON [APP_LOGIN] [SHA]` | The full commit list was read, at least one commit is Dependabot's, and every commit has a verified signature and is either authored by `dependabot[bot]` and committed by `web-flow` or `dependabot[bot]`, or a merge commit by `APP_LOGIN` committed by `web-flow` (the App's update-branch merges). Anything else — e.g. a commit someone else pushed to the branch — blocks (#1400). Returns `2`, not `1`, when the reads cannot be trusted (no head, no commits, a commit list that does not match the PR, malformed input), so a push between two reads is never taken as a foreign commit. | | `dependabot_required_contexts RULES_JSON` | Prints the unique required checks from `GET /repos/{r}/rules/branches/{b}` as `{context, integration_id}` objects, with `integration_id` null when the rule names no app (returns `1` on a non-array). | diff --git a/.github/scripts/dependabot/lib/gates.sh b/.github/scripts/dependabot/lib/gates.sh index c79edc63f..383ac4b32 100644 --- a/.github/scripts/dependabot/lib/gates.sh +++ b/.github/scripts/dependabot/lib/gates.sh @@ -39,8 +39,11 @@ DEPENDABOT_AGENT_JOB_RE='^(dev-lead / .+|pr-auto-review / .+|pr-review / review| # # A check run superseded on the same commit — an earlier run of the same check # from the same app, workflow and event, e.g. one cancelled by a concurrency -# group or replaced by a re-run — is not counted; only the newest (highest -# databaseId) is. Runs without a databaseId are never collapsed. +# group or replaced by a re-run — is not counted when the newest (highest +# databaseId) run is unique and succeeded. Otherwise every run of the check +# counts: runs without a databaseId, or whose app, workflow or event cannot +# be read, are never collapsed, nor are runs whose newest was skipped, +# neutral, failing or pending, or shares its databaseId. # Agent jobs (DEPENDABOT_AGENT_JOB_RE) are not counted. Passes only when every # other check run on the head commit COMPLETED as success, neutral or skipped, # every commit status is success, at least one such check was reported, and @@ -59,14 +62,29 @@ dependabot_checks_gate() { | ($pr.headRefOid // "") as $head | ($pr.commits.nodes[0].commit // {}) as $commit | ($commit.statusCheckRollup.contexts // null) as $ctx - | def latest_runs: - # Newest run per (app, workflow, event, name); anything that cannot be - # keyed or ordered is kept as is, so nothing is dropped by accident. - ([.[] | select(.__typename == "CheckRun" and ((.databaseId | type) == "number"))] - | group_by([(.checkSuite.app.slug? // ""), (.checkSuite.workflowRun.workflow.databaseId? // ""), - (.checkSuite.workflowRun.event? // ""), (.name // "")]) - | map(max_by(.databaseId))) as $newest - | [.[] | select(.__typename != "CheckRun" or ((.databaseId | type) != "number"))] + $newest; + | def run_key: + [(.checkSuite.app.slug? // null), (.checkSuite.workflowRun.workflow.databaseId? // null), + (.checkSuite.workflowRun.event? // null), (.name // null)]; + def collapsible: .__typename == "CheckRun" and ((.databaseId | type) == "number") + and (run_key | (.[0] | type) == "string" and .[0] != "" + and (.[1] | type) == "number" + and (.[2] | type) == "string" and .[2] != "" + and (.[3] | type) == "string" and .[3] != ""); + def latest_runs: + # Older runs of a check are dropped only when the newest run of the + # same (app, workflow, event, name) is unambiguous and succeeded. A run + # whose key cannot be read fully, a newest run that was skipped, + # neutral, failing or pending, or a tie on the newest databaseId keeps + # every run of the group, so any failure among them still holds. + ([.[] | select(collapsible)] | group_by(run_key) + | map((map(.databaseId) | max) as $top + | [.[] | select(.databaseId == $top)] as $newest + | if ($newest | length) == 1 + and (($newest[0].status // "" | ascii_upcase) == "COMPLETED") + and (($newest[0].conclusion // "" | ascii_upcase) == "SUCCESS") + then $newest else . end) + | add // []) as $kept + | [.[] | select(collapsible | not)] + $kept; ($ctx.nodes // [] | latest_runs) as $nodes | def name_of: if .__typename == "StatusContext" then (.context // "") else (.name // "") end; diff --git a/test/workflows/dependabot/gates.bats b/test/workflows/dependabot/gates.bats index cc24fad30..acaa0fd24 100644 --- a/test/workflows/dependabot/gates.bats +++ b/test/workflows/dependabot/gates.bats @@ -299,6 +299,51 @@ _ok_nodes() { [ "$status" -eq 1 ] } +# A run is collapsed only when its whole key can be read: two runs whose app, +# workflow or event is missing could be different checks, so both count +# (#1421 review: the collapse must fail closed). +_run_with() { # ID NAME CONCLUSION JQ_EDIT + tt_run "$1" "$2" COMPLETED "$3" | jq -c "$4" +} + +@test "checks: runs whose workflow cannot be read are never collapsed" { + nodes="[$(_run_with 101 build FAILURE '.checkSuite.workflowRun.workflow = null'),$(_run_with 102 build SUCCESS '.checkSuite.workflowRun.workflow = null')]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] + [[ "$output" == *"build (failure)"* ]] +} + +@test "checks: runs with no workflow run (another app's checks) are never collapsed" { + nodes="[$(_run_with 101 scan FAILURE '.checkSuite = {app:{slug:"other-ci"}}'),$(_run_with 102 scan SUCCESS '.checkSuite = {app:{slug:"other-ci"}}')]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: runs whose event or app cannot be read are never collapsed" { + nodes="[$(_run_with 101 build FAILURE '.checkSuite.workflowRun.event = null'),$(_run_with 102 build SUCCESS '.checkSuite.workflowRun.event = null')]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] + nodes="[$(_run_with 101 build FAILURE '.checkSuite.app = null'),$(_run_with 102 build SUCCESS '.checkSuite.app = null')]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: a newer run that was skipped does not hide an older failure" { + nodes="[$(tt_run 101 build COMPLETED FAILURE),$(tt_run 102 build COMPLETED SKIPPED)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] + [[ "$output" == *"build (failure)"* ]] + nodes="[$(tt_run 101 build COMPLETED FAILURE),$(tt_run 102 build COMPLETED NEUTRAL)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + +@test "checks: two runs sharing the newest databaseId both count" { + nodes="[$(tt_run 102 build COMPLETED FAILURE),$(tt_run 102 build COMPLETED SUCCESS)]" + run dependabot_checks_gate "$(tt_pr_state "$nodes")" '[]' + [ "$status" -eq 1 ] +} + # ── dependabot_provenance_gate (#1400) ────────────────────────────────────── _dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } From a7757092ade6745ffd32f115e8ef6087e0974e0f Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:40:35 -0600 Subject: [PATCH 16/17] test(dependabot): pin that forged GITHUB_TOKEN commits are refused (#1400) Sandbox run 41754805617 on software-factory-canary: a Contents API commit with Dependabot's email as author comes out as author dependabot[bot], committer github-actions[bot], unsigned; without the override GitHub signs it but attributes it to github-actions[bot]. Pin both shapes. Co-Authored-By: Claude Opus 5.5 --- test/workflows/dependabot/gates.bats | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/test/workflows/dependabot/gates.bats b/test/workflows/dependabot/gates.bats index acaa0fd24..1fde7d3f2 100644 --- a/test/workflows/dependabot/gates.bats +++ b/test/workflows/dependabot/gates.bats @@ -467,7 +467,9 @@ _dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } # Sandbox evidence (combsmith/software-factory-canary, 2026-11-03): commits made # through the Contents or Git Data API with Dependabot's noreply email are # attributed to dependabot[bot] by GitHub, but are unsigned. Only the -# verified-signature requirement refuses them. +# verified-signature requirement refuses them. The same holds for a workflow +# GITHUB_TOKEN (run 41754805617): GitHub signs its commits only when the author +# is not overridden, and then attributes them to github-actions[bot]. @test "provenance: forged Dependabot author via the Contents API (committer is the pusher, unsigned) blocks" { commits="[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' don-petry false 1)]" run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" @@ -482,6 +484,20 @@ _dep_commit() { tt_commit "$1" 'dependabot[bot]' web-flow true 1; } [[ "$output" == *"not from Dependabot"*"unverified"* ]] } +@test "provenance: forged Dependabot author via the Contents API with GITHUB_TOKEN (committer github-actions[bot], unsigned) blocks" { + commits="[$(tt_commit "$HEAD_SHA" 'dependabot[bot]' 'github-actions[bot]' false 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" + [ "$status" -eq 1 ] + [[ "$output" == *"not from Dependabot"*"unverified"* ]] +} + +@test "provenance: a signed GITHUB_TOKEN commit (author github-actions[bot], committer web-flow) blocks" { + commits="[$(tt_commit "$HEAD_SHA" 'github-actions[bot]' web-flow true 1)]" + run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 1)" "$commits" "" + [ "$status" -eq 1 ] + [[ "$output" == *"not from Dependabot"* ]] +} + @test "provenance: a forged unsigned commit on top of a genuine one blocks" { commits="[$(tt_commit "$DEP_SHA" 'dependabot[bot]' web-flow true 1),$(tt_commit "$HEAD_SHA" 'dependabot[bot]' 'dependabot[bot]' false 1)]" run dependabot_provenance_gate "$(tt_pr_rest "$HEAD_SHA" 2)" "$commits" "" From 1f5466439576b0cd26f59c8c49faf911e0bd3a14 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:45:35 -0600 Subject: [PATCH 17/17] docs(dependabot): match the caller stub example to the template; note the failed-read path (#1399) Co-Authored-By: Claude Opus 5.5 --- .github/scripts/dependabot/README.md | 5 ++++- standards/dependabot-policy.md | 9 ++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/scripts/dependabot/README.md b/.github/scripts/dependabot/README.md index 88357e9e4..55a0353b8 100644 --- a/.github/scripts/dependabot/README.md +++ b/.github/scripts/dependabot/README.md @@ -43,7 +43,10 @@ applies only when it is enabled. The re-read and the `synchronize` run together withdraw that state on every interleaving, but not in the same instant, so a foreign push has a short window before its run revokes. The merger is not exposed to it: it re-runs both gates and merges with `sha=`. If the `synchronize` -run does not run at all, the state stays until something re-evaluates the PR. The +run does not run at all, or fails because the provenance read returned `2` +(the PR state could not be read, so there is no verdict to act on), the state +stays until something re-evaluates the PR; a failed read fails the run, so it +shows up as a red check. The required approval of the last push should block a merge in that window, but this has not been tested end to end. The maintainer accepted this risk on 2026-11-02 (see `standards/dependabot-policy.md`). diff --git a/standards/dependabot-policy.md b/standards/dependabot-policy.md index fe74541c7..4e6bd3536 100644 --- a/standards/dependabot-policy.md +++ b/standards/dependabot-policy.md @@ -296,14 +296,17 @@ Gate logic: [`.github/scripts/dependabot/`](../.github/scripts/dependabot/README ### Caller Stub Format The repo-level `dependabot-rebase.yml` is a thin caller stub. It must use -**explicit secrets** (not `secrets: inherit`) and **write permissions**: +**explicit secrets** (not `secrets: inherit`) and **write permissions**. Copy +[`workflows/dependabot-rebase.yml`](workflows/dependabot-rebase.yml) verbatim; +its job is: ```yaml jobs: dependabot-rebase: permissions: - pull-requests: write # call update-branch API on behind PRs and merge when ready - uses: combsmith/.github/.github/workflows/dependabot-rebase-reusable.yml@b51e2edf830ea085be0277bcf3174c7b3ec8f958 # v1 + contents: write # the reusable checks out its gate tooling (contents: read) + pull-requests: write # the reusable lists Dependabot PRs (pull-requests: read) + uses: combsmith/.github/.github/workflows/dependabot-rebase-reusable.yml@dependabot-rebase/v2-stable secrets: APP_ID: ${{ secrets.APP_ID }} APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }}