diff --git a/.github/workflows/dependabot-auto-merge.yml b/.github/workflows/dependabot-auto-merge.yml index a7a204d..46b7cfa 100644 --- a/.github/workflows/dependabot-auto-merge.yml +++ b/.github/workflows/dependabot-auto-merge.yml @@ -1,31 +1,19 @@ name: Dependabot Auto-Merge -# Reusable workflow: safely auto-merges Dependabot PRs after CI passes. -# Scope is narrow: +# Reusable workflow: merges every Dependabot PR once all its checks pass. +# Scope (v3, smartwatermelon/dev-env#229, #203): # - Only runs when the PR author is dependabot[bot], checked via # github.event.pull_request.user.login (not github.actor, which # zizmor flags as spoofable in other trigger contexts: it reflects # the last actor to touch the ref, not who opened the PR). -# - Patch/minor: always auto-merged. -# - Major: only auto-merged when EVERY listed dependency belongs to a -# trusted namespace (dependabot/, actions/, plus each caller's own -# org — see the `trusted_namespaces` input below). One -# untrusted dep in a grouped update blocks the whole group. -# - Major, opt-in (allow_major): one dep, merged only after ALL -# checks finish green. See README. +# - Every PR merges only after all checks pass; a blocked PR is labeled. See README. +# # - Uses pull_request_target so the BASE-branch workflow runs, not # the PR branch's — a PR modifying this file cannot bypass itself. # - Never executes PR code; the only actions taken are `gh pr review` # and `gh pr merge`, both GitHub-side API calls. -# - Computes patch-vs-major from previous-version/new-version itself -# rather than trusting steps.metadata.outputs.update-type. On -# 2026-04-28, fetch-metadata@v2 mislabeled a 2.0.1 -> 3.0.0 -# reusable-workflow bump as semver-patch; trust math, not labels. -# This shortcut only applies to single-dependency PRs — those -# version fields describe one package, so a grouped PR (more than -# one entry in dependency-names) always goes through the -# namespace-allowlist path instead, even if it "looks like" a -# patch/minor bump. +# +# - Labels use `gh pr edit` and `gh label create`, also API-only. # # DO NOT add `actions/checkout` to this workflow. It uses # `pull_request_target`, which runs with base-branch secrets available. @@ -52,11 +40,9 @@ name: Dependabot Auto-Merge # # `gh pr review --approve` satisfies branch-protection rules that # require review; it's a no-op on repos where no approving review is -# required. `gh pr merge --auto` waits only on REQUIRED checks; a red -# non-required build does not stop it. -# Both `gh` calls degrade to a visible ::warning:: rather than a hard -# failure — see #87 (a hard failure on either call left an -# unactionable red check with no visible cause). +# required. +# +# Every `gh` call degrades to a ::warning:: or a label, never a red run (#87). # # Usage in a caller workflow: # @@ -66,10 +52,7 @@ name: Dependabot Auto-Merge # # jobs: # dependabot-auto-merge: -# uses: smartwatermelon/github-workflows/.github/workflows/dependabot-auto-merge.yml@dependabot-auto-merge-v2 -# with: -# trusted_namespaces: 'smartwatermelon' # dependabot/actions always trusted; add your org(s) -# allow_major: true # optional opt-in; default false +# uses: smartwatermelon/github-workflows/.github/workflows/dependabot-auto-merge.yml@dependabot-auto-merge-v3 # # Do NOT add a `secrets:` block to the caller (see above). # @@ -80,38 +63,24 @@ name: Dependabot Auto-Merge # second, unrelated file cannot safely "start its own v1" in the same # repo-wide tag namespace. # -# v2 (this version): ported fleet-wide hardening that had drifted -# ahead of v1 in per-repo copies — user.login actor check, -# SHA-pinned fetch-metadata, self-computed major/minor decision with -# a trusted-namespace allowlist for major bumps — and generalized the -# allowlist's org name into a `trusted_namespaces` input so -# non-smartwatermelon callers (e.g. nightowlstudiollc) aren't stuck -# with another org's namespace hardcoded in. See smartwatermelon/dev-env#20. +# v3 (this version): every PR merges through the all-checks gate, and blocked PRs are labeled. on: workflow_call: inputs: trusted_namespaces: description: >- - Comma-separated dependency-name namespace prefixes (without - trailing slash) that are auto-mergeable on a MAJOR version - bump. dependabot and actions are always trusted; add your - own org here, e.g. 'smartwatermelon' or 'nightowlstudiollc'. - Only matches 'namespace/name'-style dependency names (GitHub - Actions org/repo, npm scoped packages like '@scope/name'). - Unscoped package-manager deps (npm 'lodash', PyPI 'requests', - etc. with no slash) never match and always fall through to - manual review on a major bump, regardless of this allowlist. + Ignored since v3: every dependency goes through the same gate. + Accepted for one release so v2 callers do not fail with + "unexpected input"; removed in v4. type: string required: false default: '' allow_major: description: >- - Opt in to auto-merging a single-dependency MAJOR bump outside - trusted_namespaces. It merges only after every check on the - PR head has finished green (not just required checks). See - the README. Grouped PRs are never - affected. Default false keeps the pre-input behavior. + Ignored since v3: majors always merge through the gate. + Accepted for one release so v2 callers do not fail with + "unexpected input"; removed in v4. type: boolean required: false default: false @@ -127,8 +96,8 @@ jobs: # ref, which a human pushing to an existing Dependabot PR can spoof). if: github.event.pull_request.user.login == 'dependabot[bot]' runs-on: ubuntu-latest - # Bounds the opt-in wait (settle + 40 min deadline). - timeout-minutes: 50 + # Bounds the gate's wait (settle + 60 min deadline) with room to label. + timeout-minutes: 70 permissions: contents: write pull-requests: write @@ -166,7 +135,7 @@ jobs: id: metadata uses: dependabot/fetch-metadata@25dd0e34f4fe68f24cc83900b1fe3fe149efef98 # v3 - - name: Decide if PR is auto-mergeable + - name: Decide if PR can be evaluated id: policy env: PREV_VERSION: ${{ steps.metadata.outputs.previous-version }} @@ -179,123 +148,93 @@ jobs: extract_major() { printf '%s' "$1" | sed -E 's@^v?([0-9]+).*@\1@' | grep -E '^[0-9]+$' || echo "" } - # fetch-metadata's previous-version/new-version describe a single - # dependency; for a grouped Dependabot PR (dependency-names has - # more than one entry) they aren't guaranteed to represent every - # package in the group. Trusting them as a group-wide patch/minor - # shortcut could let an untrusted-namespace major bump ride along - # inside a group whose primary package looks like a patch. Force - # any multi-dependency PR through the namespace-allowlist path - # instead of the version-shortcut path. dep_count=$(printf '%s' "${DEP_NAMES:-}" \ | tr ',' '\n' \ | sed -E 's@^[[:space:]]+|[[:space:]]+$@@g' \ | grep -c -v '^$' || true) - if [ "$dep_count" -le 1 ]; then - prev_major=$(extract_major "$PREV_VERSION") - new_major=$(extract_major "$NEW_VERSION") - if [ -n "$prev_major" ] && [ -n "$new_major" ] && [ "$prev_major" = "$new_major" ]; then - echo "decision=merge" >> "$GITHUB_OUTPUT" - echo "::notice::Patch/minor bump within major v$prev_major; auto-merging" - exit 0 - fi - fi - # Fail closed if dependency-names is missing or whitespace-only — - # we cannot apply the trusted-namespace allowlist without it. - # Checking dep_count (already trimmed/normalized above) rather - # than the raw DEP_NAMES string catches a whitespace-only value - # (e.g. " "), which would otherwise pass this guard, then have - # the namespace-allowlist path below trim it to an empty - # remainder and incorrectly decide "merge". + # Fail closed if dependency-names is missing or whitespace-only: + # without it the run cannot say what the PR updates. if [ "$dep_count" -eq 0 ]; then - echo "decision=skip" >> "$GITHUB_OUTPUT" - echo "::notice::Empty dependency-names ($PREV_VERSION -> $NEW_VERSION); leaving for manual review" + echo "decision=blocked" >> "$GITHUB_OUTPUT" + echo "reason=cannot-evaluate" >> "$GITHUB_OUTPUT" + echo "::notice::Not merged: empty dependency-names ($PREV_VERSION -> $NEW_VERSION)" exit 0 fi - # Namespaces are validated against a strict charset (alnum, -, _) - # before being spliced into the ERE below — dependency-name - # namespaces on GitHub/npm/etc. never contain regex metacharacters, - # so rejecting anything else closes the injection surface (e.g. a - # caller passing '.*' could otherwise widen the match to - # everything and auto-merge untrusted major bumps). - namespace_pattern="^(dependabot|actions" - if [ -n "${TRUSTED_NAMESPACES:-}" ]; then - while IFS= read -r ns; do - [ -z "$ns" ] && continue - if ! printf '%s' "$ns" | grep -qE '^[A-Za-z0-9_-]+$'; then - echo "::error::trusted_namespaces entry '$ns' has chars outside [A-Za-z0-9_-]; refusing allowlist." - exit 1 - fi - namespace_pattern="${namespace_pattern}|${ns}" - done <<< "$(printf '%s' "$TRUSTED_NAMESPACES" | tr ',' '\n' | sed -E 's@^[[:space:]]+|[[:space:]]+$@@g')" + if [ -n "${TRUSTED_NAMESPACES:-}" ] || [ "${ALLOW_MAJOR:-false}" = "true" ]; then + echo "::notice::trusted_namespaces and allow_major are ignored since v3; remove them from the caller" fi - namespace_pattern="${namespace_pattern})/" - remainder=$(printf '%s' "$DEP_NAMES" \ - | tr ',' '\n' \ - | sed -E 's@^[[:space:]]+|[[:space:]]+$@@g' \ - | grep -v '^$' \ - | grep -vE "$namespace_pattern" \ - || true) - if [ -z "$remainder" ]; then - echo "decision=merge" >> "$GITHUB_OUTPUT" - echo "::notice::$PREV_VERSION -> $NEW_VERSION, deps in trusted namespace; auto-merging" - elif [ "${ALLOW_MAJOR:-false}" = "true" ] && [ "$dep_count" -eq 1 ] \ - && [ -n "${prev_major:-}" ] && [ -n "${new_major:-}" ] \ - && [ "$prev_major" != "$new_major" ]; then - # Opt-in major path (dev-env#120); the gate step waits for ALL checks. - echo "decision=major-gate" >> "$GITHUB_OUTPUT" - echo "::notice::Major v$prev_major -> v$new_major ($remainder); allow_major is on, waiting for all checks" + # For the log only: every outcome below goes through the gate. + prev_major=$(extract_major "$PREV_VERSION") + new_major=$(extract_major "$NEW_VERSION") + if [ "$dep_count" -gt 1 ]; then + kind="group of $dep_count" + elif [ -z "$prev_major" ] || [ -z "$new_major" ]; then + kind="unparseable version change" + elif [ "$prev_major" = "$new_major" ]; then + kind="patch/minor within v$prev_major" else - echo "decision=skip" >> "$GITHUB_OUTPUT" - echo "::notice::Non-allowlisted deps present: $remainder" + kind="major v$prev_major -> v$new_major" fi + echo "decision=gate" >> "$GITHUB_OUTPUT" + echo "::notice::$kind ($DEP_NAMES: $PREV_VERSION -> $NEW_VERSION); merging after every check passes" - - name: Approve and enable auto-merge - if: steps.policy.outputs.decision == 'merge' - env: - PR_URL: ${{ github.event.pull_request.html_url }} - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - gh pr review --approve "$PR_URL" \ - || echo "::warning::Approval failed (check can_approve_pull_request_reviews setting); continuing." - gh pr merge --auto --squash --delete-branch "$PR_URL" \ - || echo "::warning::Auto-merge enable failed — check branch protection / auto-merge repo setting." - - # Every exit is gate=pass or a ::notice:: skip, never red (#87). - - name: Wait for all checks (major, opt-in) + # Every exit is gate=pass|blocked|superseded, never red (#87). + - name: Wait for all checks id: gate - if: steps.policy.outputs.decision == 'major-gate' + if: steps.policy.outputs.decision == 'gate' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_URL: ${{ github.event.pull_request.html_url }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} + REPO: ${{ github.repository }} + BASE_REF: ${{ github.event.pull_request.base.ref }} RUN_ID: ${{ github.run_id }} SELF_WORKFLOW: ${{ github.workflow }} SETTLE_SECONDS: '120' POLL_SECONDS: '30' - WAIT_SECONDS: '2400' + WAIT_SECONDS: '3600' run: | set -euo pipefail - skip() { - echo "gate=skip" >> "$GITHUB_OUTPUT" - echo "::notice::Major bump left for manual review: $1" + stop() { # stop GATE REASON MESSAGE + echo "gate=$1" >> "$GITHUB_OUTPUT" + if [ -n "$2" ]; then echo "reason=$2" >> "$GITHUB_OUTPUT"; fi + echo "::notice::Not merged: $3" exit 0 } + # Names of failing checks on the base branch head, one per line. + main_failing() { + local runs statuses + runs=$(gh api "repos/${REPO}/commits/${BASE_REF}/check-runs?per_page=100" --jq '.check_runs[] + | select(.conclusion == "failure" or .conclusion == "cancelled" or .conclusion == "timed_out") + | .name') || return 1 + statuses=$(gh api "repos/${REPO}/commits/${BASE_REF}/status" --jq '.statuses[] + | select(.state == "failure" or .state == "error") | .context') || return 1 + printf '%s\n%s\n' "$runs" "$statuses" + } deadline=$(( $(date +%s) + WAIT_SECONDS )) # Netlify checks register late: settle, then need two identical finished polls. sleep "$SETTLE_SECONDS" last="" + others="" while :; do if [ "$(date +%s)" -ge "$deadline" ]; then - skip "checks still running after $(( WAIT_SECONDS / 60 )) min" + if [ "$others" = "0" ]; then + stop blocked no-checks "only this workflow's checks ran; add a required check such as standards-check" + fi + stop blocked timeout "checks still running after $(( WAIT_SECONDS / 60 )) min" fi head=$(gh pr view "$PR_URL" --json headRefOid --jq .headRefOid 2>/dev/null || true) + if [ -z "$head" ]; then + # Nothing re-runs for this head, so an unreadable head must be labeled. + stop blocked cannot-evaluate "could not read the PR head" + fi if [ "$head" != "$HEAD_SHA" ]; then - skip "PR head is now '${head:-unreadable}', not $HEAD_SHA; the run for the new head decides" + stop superseded "" "PR head is now '$head', not $HEAD_SHA; the run for the new head decides" fi # Exits nonzero while pending/failing; judge the JSON. json=$(gh pr checks "$PR_URL" --json name,workflow,bucket,link 2>/dev/null || true) # bad_names is last: read collapses empty tab fields. Unknown buckets count as pending. + # Names are joined with \u001f because job names can contain ", ". if ! summary=$(printf '%s' "$json" | jq -er \ --arg run "/actions/runs/${RUN_ID}/" --arg wf "$SELF_WORKFLOW" ' if type != "array" then error("not an array") else . end @@ -311,20 +250,33 @@ jobs: ($bad | length), ($o | map(select(.bucket == "pass")) | length), ($o | map("\(.name)=\(.bucket)") | sort | join("|")), - ($bad | map(.name) | join(", ")) ] + ($bad | map(.name) | join("\u001f")) ] | @tsv'); then - skip "could not read the PR's checks" + stop blocked cannot-evaluate "could not read the PR's checks" fi IFS=$'\t' read -r self others pending bad passed sig bad_names <<< "$summary" if [ "$self" -eq 0 ]; then # No checks: read; seeing itself proves the list is not a permission blank. - skip "this run cannot see its own check run (no checks: read on a private repo?)" + stop blocked cannot-evaluate "this run cannot see its own check run (no checks: read on a private repo?)" fi if [ "$bad" -gt 0 ]; then - skip "failing or cancelled checks: $bad_names" + # main-red: every failing check also fails on the base branch, + # so the fix is on main, not in this PR. Unreadable -> red-check. + reason=red-check + if main_list=$(main_failing); then + reason=main-red + while IFS= read -r name; do + [ -z "$name" ] && continue + grep -qxF -- "$name" <<< "$main_list" || reason=red-check + done < <(printf '%s\n' "$bad_names" | tr '\037' '\n') + fi + stop blocked "$reason" "failing or cancelled checks ($reason): ${bad_names//$'\037'/, }" fi - if [ "$pending" -eq 0 ] && [ "$passed" -ge 1 ]; then + if [ "$pending" -eq 0 ] && [ "$others" -ge 1 ]; then if [ "$sig" = "$last" ]; then + if [ "$passed" -eq 0 ]; then + stop blocked no-checks "all $others other checks were skipped; nothing proved this head works" + fi echo "gate=pass" >> "$GITHUB_OUTPUT" echo "::notice::All $others other checks finished with no failure on $HEAD_SHA" exit 0 @@ -336,15 +288,57 @@ jobs: sleep "$POLL_SECONDS" done - - name: Approve and merge (major, opt-in) + - name: Approve and merge + id: merge if: steps.gate.outputs.gate == 'pass' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_URL: ${{ github.event.pull_request.html_url }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | + set -uo pipefail gh pr review --approve "$PR_URL" \ || echo "::warning::Approval failed (check can_approve_pull_request_reviews setting); continuing." + # Clear labels an earlier run left; this head passed. + stale=$(gh pr view "$PR_URL" --json labels --jq '.labels[].name' 2>/dev/null \ + | grep '^dependabot-blocked' | paste -sd, - || true) + if [ -n "$stale" ]; then + gh pr edit "$PR_URL" --remove-label "$stale" >/dev/null \ + || echo "::warning::Could not remove labels: $stale" + fi # Not --auto: it survives a rebase and then waits on required checks only. - gh pr merge --squash --delete-branch --match-head-commit "$HEAD_SHA" "$PR_URL" \ - || echo "::warning::Merge failed — head moved, approval missing, or branch protection blocked it." + if ! gh pr merge --squash --delete-branch --match-head-commit "$HEAD_SHA" "$PR_URL"; then + echo "reason=merge-failed" >> "$GITHUB_OUTPUT" + echo "::warning::Merge failed — head moved, approval missing, or branch protection blocked it." + fi + + - name: Label blocked PR + if: >- + steps.policy.outputs.decision == 'blocked' + || steps.gate.outputs.gate == 'blocked' + || steps.merge.outputs.reason == 'merge-failed' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + PR_URL: ${{ github.event.pull_request.html_url }} + REASON: ${{ steps.merge.outputs.reason || steps.gate.outputs.reason || steps.policy.outputs.reason }} + run: | + set -uo pipefail + case "${REASON:-}" in + red-check|main-red|timeout|no-checks|cannot-evaluate|merge-failed) ;; + *) REASON=cannot-evaluate ;; + esac + want="dependabot-blocked:${REASON}" + for label in dependabot-blocked "$want"; do + gh label create "$label" --color D93F0B --force \ + --description "Dependabot auto-merge did not merge this PR; see the workflow run" >/dev/null \ + || echo "::warning::Could not create label $label" + done + # Drop a reason label left by an earlier run for another reason. + stale=$(gh pr view "$PR_URL" --json labels --jq '.labels[].name' 2>/dev/null \ + | grep '^dependabot-blocked:' | grep -vxF "$want" | paste -sd, - || true) + args=(--add-label "dependabot-blocked,${want}") + if [ -n "$stale" ]; then args+=(--remove-label "$stale"); fi + gh pr edit "$PR_URL" "${args[@]}" >/dev/null \ + || echo "::warning::Could not label the PR ${want}" + echo "::notice::Labeled ${want}" diff --git a/README.md b/README.md index e608919..32b3850 100644 --- a/README.md +++ b/README.md @@ -129,39 +129,61 @@ cannot safely start its own bare `v1` while one already exists. ## `dependabot-auto-merge` -Reusable workflow: approves and auto-merges Dependabot PRs for patch and -minor version updates once the **required** checks pass. Major-version -bumps are left open for manual review, unless the dependency is in a -trusted namespace or the caller opts in with `allow_major: true`. +Reusable workflow: merges every Dependabot PR, patch, minor, major or +grouped, once **every** check on the PR head has finished green. No human +approves Dependabot updates (smartwatermelon/dev-env#229, decided +2026-10-25). The risk of a compromised upstream release is accepted; the +gate covers the other risk, a bump that breaks the build. -"Required" means branch protection's list, not every check. `gh pr merge ---auto` waits only on required checks, and most fleet repos require only -`standards-check`. On the patch/minor path, a red `build` that is not -required does not stop the merge. - -### Opt-in: `allow_major` - -`allow_major: true` (default `false`) lets a **single-dependency** major -bump merge without a human, under a stricter gate than the patch/minor -path (smartwatermelon/dev-env#120): +### The gate (v3) - The job waits until **every** check on the PR head SHA has finished, required or not, except this workflow's own. Any failed or cancelled - check leaves the PR for manual review. SKIPPED and NEUTRAL do not block. + check blocks the merge. SKIPPED and NEUTRAL do not block. "Required" + is not enough: most fleet repos require only `standards-check`, and a + red `build` that is not required would otherwise merge. - It then merges directly with `--match-head-commit`, not `--auto`, so a later Dependabot rebase cannot inherit an armed auto-merge. -- It gives up with a `::notice::` (never a red check) after about 40 - minutes, when the head moves, or when it cannot see its own check run. - The job's token has no `checks` permission; on a private repo the check - list may come back blank, and the self-visibility test turns that into - "manual review" instead of a false "all green". -- Grouped PRs never take this path. -- Pair it with a Dependabot `cooldown` so a major is some days old before - its PR opens. `semver-major-days` works only for ecosystems that support - SemVer (npm, pip, uv, and others); `github-actions` honors only - `default-days`. **Security updates ignore `cooldown`**, so with - `allow_major: true` a security-driven major can merge on day 0 once all - checks are green. +- It waits up to 60 minutes. When the head moves, it stops and the run + for the new head decides. The job's token has no `checks` permission. + On a private repo the check list may come back blank. The + self-visibility test turns that into a block, not a false "all green". +- The major/minor/group classification is logged, never used to decide. + +### Blocked PRs are labeled + +When the gate does not merge a PR, it adds `dependabot-blocked` and one +reason label. It also removes any reason label left by an earlier run. Nothing +fails red (#87). The Dependabot digest in `smartwatermelon/dev-env` reads +these labels. + +| Label | Meaning | +| --- | --- | +| `dependabot-blocked:red-check` | A check failed on this PR and is not failing on the base branch. | +| `dependabot-blocked:main-red` | Every failing check also fails on the base branch head. Fix `main`; the PR is probably fine. | +| `dependabot-blocked:timeout` | Checks were still running after 60 minutes. | +| `dependabot-blocked:no-checks` | No other check ran, or every other check was skipped. Add a required check such as `standards-check`. | +| `dependabot-blocked:cannot-evaluate` | Empty `dependency-names`, an unreadable PR head or check list, or the run cannot see its own check run. | +| `dependabot-blocked:merge-failed` | The gate passed but `gh pr merge` failed (head moved, approval missing, or branch protection). | + +A PR that later merges has its `dependabot-blocked` labels removed first. +Creating and adding labels needs only `pull-requests: write`, which the +caller already grants. + +### Cooldown + +Keep a short Dependabot `cooldown` (1 to 3 days) on version updates. It +is the main guard against a compromised release. Such a release is +usually pulled within hours to days. **Security updates ignore `cooldown`**, so a +security fix still merges on day 0 once all checks are green. +`semver-major-days` works only for ecosystems that support SemVer (npm, +pip, uv, and others); `github-actions` honors only `default-days`. + +### Removed inputs + +`trusted_namespaces` and `allow_major` are ignored since v3. The run +prints a notice when a caller passes them. They are still declared so v2 +callers do not fail with "unexpected input". They are removed in v4. ### Setup @@ -185,9 +207,8 @@ permissions: jobs: # This caller stub is intentionally a thin delegate, not a full - # implementation. The version-comparison logic, the dependabot/actions - # always-trusted namespace defaults, and the actor-spoofing guard all - # live in the canonical workflow (smartwatermelon/github-workflows, + # implementation. The all-checks gate, the blocked labels, and the + # actor-spoofing guard all live in the canonical workflow (smartwatermelon/github-workflows, # this file) rather than here — a local diff that only shows this # stub shrinking will not show that logic; it moved, not disappeared. # See smartwatermelon/dev-env#20 for why: copy-pasting this workflow @@ -196,19 +217,16 @@ jobs: # workflows here use a repo-wide semver tag convention (see # "Versioning" below), not per-action SHA pinning. dependabot-auto-merge: - uses: smartwatermelon/github-workflows/.github/workflows/dependabot-auto-merge.yml@dependabot-auto-merge-v2 - with: - trusted_namespaces: 'smartwatermelon' # adjust to your own org; dependabot/actions are always trusted - # allow_major: true # optional; see "Opt-in: allow_major" above + uses: smartwatermelon/github-workflows/.github/workflows/dependabot-auto-merge.yml@dependabot-auto-merge-v3 ``` Also add `can_approve_pull_request_reviews: true` to the repo's Actions workflow permissions (Settings → Actions → General → Workflow -permissions), or run the fleet-wide fix in this repo's -`README.md`/issue #87 history. Without it, `gh pr review --approve` -fails; the workflow degrades to a visible `::warning::` instead of a -silent stall, but auto-merge still won't proceed if the repo's branch -protection requires an approval. +permissions). The fleet-wide fix is in this repo's `README.md`/issue #87 +history. Without it, `gh pr review --approve` fails, and the workflow +prints a visible `::warning::`. If branch protection requires an +approval, the merge then fails and the PR is labeled +`dependabot-blocked:merge-failed`. **Do NOT add `secrets: inherit` to the caller.** This workflow needs no secrets beyond the `GITHUB_TOKEN` it mints internally and declares no @@ -229,11 +247,12 @@ reach for `secrets: inherit`. ### Security invariant: no `actions/checkout` This workflow uses `pull_request_target`, which runs with base-branch -secrets available — the trigger implicated in several real supply-chain -incidents (Ultralytics, nx, tj-actions) when combined with a checkout -of PR-controlled code. This workflow is safe today because it never -executes PR code: the only actions are API calls (`dependabot/fetch-metadata`, -`gh pr review`, `gh pr merge`). **`actions/checkout` must never be added +secrets available. That trigger is implicated in several real +supply-chain incidents (Ultralytics, nx, tj-actions) when combined with a +checkout of PR-controlled code. This workflow is safe today because it +never executes PR code. Its only actions are API calls +(`dependabot/fetch-metadata`, `gh pr review`, `gh pr merge`, +`gh pr edit`, `gh label create`). **`actions/checkout` must never be added to this file.** The guardrail below enforces this (closes #64): - `self-review.yml`'s `guard-no-checkout` job greps this repo's own @@ -259,15 +278,14 @@ so the namespace collision this prefix avoids is permanent.) ### Rollout discipline -New tags of this workflow (and behavior-changing bumps) are pointed at -from 2-3 low-traffic pilot repos first, pinned to the specific new tag -(not a floating major). Let at least one real Dependabot PR flow -through each pilot and confirm correct patch/minor-only behavior before -repointing any floating tag fleet-wide. This workflow approves and -merges PRs unattended with no fallback reviewer behind it — a -bug here ships to every repo pinned to the affected tag at once, so it -gets the same pilot-then-fleet discipline as the org-level rollout -work. +New tags of this workflow, and behavior-changing bumps, go first to 2-3 +low-traffic pilot repos. Each pilot pins the specific new tag, not a +floating major. Let at least one real Dependabot PR flow through each +pilot. Confirm the expected merge or label before moving callers to the +new tag. This workflow approves and merges PRs unattended, with no +fallback reviewer behind it. A bug here ships to every repo pinned to +the affected tag at once. So it gets the same pilot-then-fleet +discipline as the org-level rollout work. --- diff --git a/tests/stub-dependabot/gh b/tests/stub-dependabot/gh index c9f8ecc..810b120 100755 --- a/tests/stub-dependabot/gh +++ b/tests/stub-dependabot/gh @@ -1,12 +1,24 @@ #!/usr/bin/env bash -# Stub gh: pr view prints head; pr checks prints checks..json -# (last repeats) and exits 8 while pending, like gh. +# Stub gh: pr view prints head (or labels); pr checks prints checks..json +# (last repeats) and exits 8 while pending, like gh. api prints the base +# branch's failing names from main-runs / main-statuses (already --jq-filtered). +# A file fail.[-] in STUB_DIR (fail.api, fail.pr-merge) makes it exit 1. set -euo pipefail : "${STUB_DIR:?}" printf '%s\n' "$*" >>"${STUB_DIR}/calls.log" -case "$1 $2" in +key="$1" +[[ "$1" == api ]] || key="$1-${2:-}" +if [[ -f "${STUB_DIR}/fail.${key}" ]]; then + echo "stub gh: forced failure of $*" >&2 + exit 1 +fi +case "$1 ${2:-}" in "pr view") - cat "${STUB_DIR}/head" + if [[ "$*" == *labels* ]]; then + cat "${STUB_DIR}/labels" 2>/dev/null || true + else + cat "${STUB_DIR}/head" + fi ;; "pr checks") n=$(($(cat "${STUB_DIR}/checks.count" 2>/dev/null || echo 0) + 1)) @@ -17,6 +29,13 @@ case "$1 $2" in cat "${f}" if jq -e 'any(.[]; .bucket == "pending")' "${f}" >/dev/null 2>&1; then exit 8; fi ;; + "api "*/check-runs*) + cat "${STUB_DIR}/main-runs" 2>/dev/null || true + ;; + "api "*/status) + cat "${STUB_DIR}/main-statuses" 2>/dev/null || true + ;; + "pr review" | "pr merge" | "pr edit" | "label create") ;; *) echo "stub gh: unhandled $*" >&2 exit 99 diff --git a/tests/test-dependabot-auto-merge.sh b/tests/test-dependabot-auto-merge.sh index e634f11..7ba40e0 100755 --- a/tests/test-dependabot-auto-merge.sh +++ b/tests/test-dependabot-auto-merge.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -# Tests dependabot-auto-merge.yml's policy and gate run: blocks, -# extracted with yq, against tests/stub-dependabot/gh. +# Tests dependabot-auto-merge.yml's policy, gate, merge and label run: +# blocks, extracted with yq, against tests/stub-dependabot/gh. set -euo pipefail here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" wf="${here}/../.github/workflows/dependabot-auto-merge.yml" @@ -15,9 +15,13 @@ _bad() { echo " FAIL $1"; fail=$((fail + 1)); } _step() { # _step NAME: the run: block of the named step yq ".jobs.auto-merge.steps[] | select(.name == \"$1\") | .run" "${wf}" } -policy_sh="$(_step 'Decide if PR is auto-mergeable')" -gate_sh="$(_step 'Wait for all checks (major, opt-in)')" -[[ -n "${policy_sh}" && -n "${gate_sh}" ]] || { echo "could not extract run blocks"; exit 1; } +policy_sh="$(_step 'Decide if PR can be evaluated')" +gate_sh="$(_step 'Wait for all checks')" +merge_sh="$(_step 'Approve and merge')" +label_sh="$(_step 'Label blocked PR')" +for s in "${policy_sh}" "${gate_sh}" "${merge_sh}" "${label_sh}"; do + [[ -n "${s}" && "${s}" != "null" ]] || { echo "could not extract run blocks"; exit 1; } +done _fixture() { STUB_DIR="$(mktemp -d)"; export STUB_DIR @@ -25,35 +29,42 @@ _fixture() { : >"${STUB_DIR}/calls.log" : >"${STUB_DIR}/out" } +_out() { sed -n "s/^$1=//p" "${STUB_DIR}/out"; } +_called() { grep -qF -- "$1" "${STUB_DIR}/calls.log"; } # --- policy step ------------------------------------------------------- -# policy PREV NEW DEPS ALLOW [TRUSTED] -> sets decision +# policy PREV NEW DEPS [ALLOW] [TRUSTED] -> sets decision, reason, out policy() { _fixture rc=0 - PREV_VERSION="$1" NEW_VERSION="$2" DEP_NAMES="$3" ALLOW_MAJOR="$4" \ + out="$(PREV_VERSION="$1" NEW_VERSION="$2" DEP_NAMES="$3" ALLOW_MAJOR="${4-false}" \ TRUSTED_NAMESPACES="${5-}" GITHUB_OUTPUT="${STUB_DIR}/out" \ - bash -c "${policy_sh}" >/dev/null 2>&1 || rc=$? - decision="$(sed -n 's/^decision=//p' "${STUB_DIR}/out")" + bash -c "${policy_sh}" 2>&1)" || rc=$? + decision="$(_out decision)"; reason="$(_out reason)" } -_expect_policy() { # label expected - if [[ "${rc}" -eq 0 && "${decision}" == "$2" ]]; then _ok "$1"; else _bad "$1: rc=${rc} decision='${decision}'"; fi +_expect_policy() { # label expected-decision [notice-substring] + if [[ "${rc}" -eq 0 && "${decision}" == "$2" ]] && grep -qF -- "${3-}" <<<"${out}"; then + _ok "$1" + else + _bad "$1: rc=${rc} decision='${decision}' out=${out}" + fi } -policy 5.9.3 6.0.3 typescript false -_expect_policy "single untrusted major, allow_major off -> skip (unchanged)" skip -policy 5.9.3 6.0.3 typescript true -_expect_policy "single untrusted major, allow_major on -> major-gate" major-gate -policy 5.9.3 6.0.3 "typescript, eslint" true -_expect_policy "grouped PR with allow_major on -> skip (groups unchanged)" skip -policy 5.9.3 5.10.0 typescript true -_expect_policy "single minor -> merge (unchanged)" merge -policy abc def typescript true -_expect_policy "unparseable versions with allow_major on -> skip" skip +policy 5.9.3 6.0.3 typescript +_expect_policy "single untrusted major -> gate (was skip in v2)" gate "major v5 -> v6" +policy 5.9.3 6.0.3 "typescript, eslint" +_expect_policy "grouped PR -> gate (was skip in v2)" gate "group of 2" +policy 5.9.3 5.10.0 typescript +_expect_policy "single minor -> gate, not --auto" gate "patch/minor within v5" +policy abc def typescript +_expect_policy "unparseable versions -> gate" gate "unparseable version change" policy 1.0.0 2.0.0 smartwatermelon/thing true smartwatermelon -_expect_policy "trusted single major keeps the --auto path -> merge" merge -policy 1.0.0 2.0.0 "" true -_expect_policy "empty dependency-names with allow_major on -> skip" skip +_expect_policy "v2 inputs are accepted and reported as ignored" gate "ignored since v3" +policy 1.0.0 2.0.0 "" +_expect_policy "empty dependency-names -> blocked" blocked "empty dependency-names" +if [[ "${reason}" == "cannot-evaluate" ]]; then _ok "empty deps reason is cannot-evaluate"; else _bad "empty deps reason='${reason}'"; fi +policy 1.0.0 2.0.0 " " +_expect_policy "whitespace-only dependency-names -> blocked" blocked # --- gate step --------------------------------------------------------- sha="0123456789abcdef0123456789abcdef01234567" @@ -82,15 +93,17 @@ gate() { rc=0 out="$(PATH="${here}/stub-dependabot:${PATH}" GITHUB_OUTPUT="${STUB_DIR}/out" \ PR_URL=https://github.com/acme/site/pull/7 HEAD_SHA="${sha}" RUN_ID="${run_id}" \ + REPO=acme/site BASE_REF=main \ SELF_WORKFLOW="Dependabot Auto-Merge" SETTLE_SECONDS=0 POLL_SECONDS=0.1 \ WAIT_SECONDS="${WAIT:-3}" bash -c "${gate_sh}" 2>&1)" || rc=$? - result="$(sed -n 's/^gate=//p' "${STUB_DIR}/out")" + result="$(_out gate)"; reason="$(_out reason)" } -_expect_gate() { # label expected [notice-substring] - if [[ "${rc}" -eq 0 && "${result}" == "$2" ]] && grep -qF -- "${3-}" <<<"${out}"; then +_expect_gate() { # label expected [reason] [notice-substring] + if [[ "${rc}" -eq 0 && "${result}" == "$2" && "${reason}" == "${3-}" ]] \ + && grep -qF -- "${4-}" <<<"${out}"; then _ok "$1" else - _bad "$1: rc=${rc} gate='${result}' out=${out}" + _bad "$1: rc=${rc} gate='${result}' reason='${reason}' out=${out}" fi } @@ -99,57 +112,155 @@ _fixture; echo "${sha}" >"${STUB_DIR}/head" _checks 1 self "build|pending" _checks 2 self "build|pass" "Pages changed|skipping" gate -_expect_gate "pending then green (NEUTRAL/skipped allowed) -> pass" pass "All 2 other checks" +_expect_gate "pending then green (NEUTRAL/skipped allowed) -> pass" pass "" "All 2 other checks" polls="$(<"${STUB_DIR}/checks.count")" if [[ "${polls}" -ge 3 ]]; then _ok "needs two identical finished polls"; else _bad "passed after ${polls} polls"; fi -# 2. a failing non-required check blocks, and names it +# 2. a failing non-required check blocks, and names it; main is green _fixture; echo "${sha}" >"${STUB_DIR}/head" _checks 1 self "build|fail" "standards|pass" gate -_expect_gate "failing build -> skip" skip "failing or cancelled checks: build" +_expect_gate "failing build, green main -> red-check" blocked red-check "failing or cancelled checks (red-check): build" # 3. cancelled counts as bad _fixture; echo "${sha}" >"${STUB_DIR}/head" _checks 1 self "build|cancel" gate -_expect_gate "cancelled check -> skip" skip "build" +_expect_gate "cancelled check -> red-check" blocked red-check "build" # 4. cannot see its own check run -> fail closed even though the rest is green _fixture; echo "${sha}" >"${STUB_DIR}/head" _checks 1 "build|pass" gate -_expect_gate "own check run not visible -> skip" skip "cannot see its own check run" +_expect_gate "own check run not visible -> cannot-evaluate" blocked cannot-evaluate "cannot see its own check run" # 5. still pending at the deadline _fixture; echo "${sha}" >"${STUB_DIR}/head" _checks 1 self "build|pending" WAIT=1 gate -_expect_gate "pending past the deadline -> skip" skip "checks still running" +_expect_gate "pending past the deadline -> timeout" blocked timeout "checks still running" # 6. only this workflow's own checks present: never passes on nothing _fixture; echo "${sha}" >"${STUB_DIR}/head" _checks 1 self "dependabot-auto-merge / other|pass|Dependabot Auto-Merge" WAIT=1 gate -_expect_gate "no other checks -> skip at deadline, not pass" skip "checks still running" +_expect_gate "no other checks -> no-checks at deadline, not pass" blocked no-checks "add a required check" -# 7. head moved -> the new run decides +# 7. head moved -> the new run decides, no label reason _fixture; echo "fedcba9876543210fedcba9876543210fedcba98" >"${STUB_DIR}/head" _checks 1 self "build|pass" gate -_expect_gate "head moved -> skip" skip "the run for the new head decides" +_expect_gate "head moved -> superseded" superseded "" "the run for the new head decides" -# 8. unreadable checks output -> skip, not pass +# 8. unreadable checks output -> blocked, not pass _fixture; echo "${sha}" >"${STUB_DIR}/head" _raw_checks 1 "not json" gate -_expect_gate "unreadable checks -> skip" skip "could not read" +_expect_gate "unreadable checks -> cannot-evaluate" blocked cannot-evaluate "could not read" # 9. an unknown bucket counts as pending, not as pass _fixture; echo "${sha}" >"${STUB_DIR}/head" _checks 1 self "build|pass" "odd|weird" WAIT=1 gate -_expect_gate "unknown bucket never passes" skip "checks still running" +_expect_gate "unknown bucket never passes" blocked timeout "checks still running" + +# 10. every failing check also fails on main -> main-red (check-runs and statuses) +_fixture; echo "${sha}" >"${STUB_DIR}/head" +_checks 1 self "build (20.x, ubuntu)|fail" "netlify/site/deploy-preview|fail" "standards|pass" +printf '%s\n' "build (20.x, ubuntu)" "lint" >"${STUB_DIR}/main-runs" +printf '%s\n' "netlify/site/deploy-preview" >"${STUB_DIR}/main-statuses" +gate +_expect_gate "failures all red on main too -> main-red" blocked main-red "build (20.x, ubuntu), netlify/site/deploy-preview" +if _called "commits/main/check-runs"; then _ok "main-red reads the base branch"; else _bad "base branch not read"; fi + +# 11. one failure is only on the PR -> red-check +_fixture; echo "${sha}" >"${STUB_DIR}/head" +_checks 1 self "build|fail" "test|fail" +printf '%s\n' "build" >"${STUB_DIR}/main-runs" +gate +_expect_gate "a failure not on main -> red-check" blocked red-check "build, test" + +# 12. base branch unreadable -> red-check, never main-red +_fixture; echo "${sha}" >"${STUB_DIR}/head" +_checks 1 self "build|fail" +printf '%s\n' "build" >"${STUB_DIR}/main-runs" +: >"${STUB_DIR}/fail.api" +gate +_expect_gate "base branch unreadable -> red-check" blocked red-check "build" + +# 13. unreadable head -> labeled, never an unlabeled superseded +_fixture; : >"${STUB_DIR}/head" +_checks 1 self "build|pass" +gate +_expect_gate "unreadable head -> cannot-evaluate" blocked cannot-evaluate "could not read the PR head" + +# 14. every other check skipped -> no-checks once settled, not timeout +_fixture; echo "${sha}" >"${STUB_DIR}/head" +_checks 1 self "build|skipping" "deploy|skipping" +WAIT=30 gate +_expect_gate "all other checks skipped -> no-checks" blocked no-checks "all 2 other checks were skipped" + +# --- merge step -------------------------------------------------------- +merge() { + rc=0 + out="$(PATH="${here}/stub-dependabot:${PATH}" GITHUB_OUTPUT="${STUB_DIR}/out" \ + PR_URL=https://github.com/acme/site/pull/7 HEAD_SHA="${sha}" \ + bash -c "${merge_sh}" 2>&1)" || rc=$? + reason="$(_out reason)" +} +_fixture +printf '%s\n' "dependencies" "dependabot-blocked" "dependabot-blocked:timeout" >"${STUB_DIR}/labels" +merge +if [[ "${rc}" -eq 0 && -z "${reason}" ]] \ + && _called "pr merge --squash --delete-branch --match-head-commit ${sha}" \ + && ! _called "--auto"; then + _ok "merge pins the head commit, never --auto" +else + _bad "merge call: rc=${rc} reason='${reason}' calls=$(<"${STUB_DIR}/calls.log")" +fi +if _called "--remove-label dependabot-blocked,dependabot-blocked:timeout"; then + _ok "merge clears blocked labels from earlier runs" +else + _bad "stale labels kept: $(<"${STUB_DIR}/calls.log")" +fi + +_fixture; : >"${STUB_DIR}/fail.pr-merge" +merge +if [[ "${rc}" -eq 0 && "${reason}" == "merge-failed" ]]; then _ok "merge failure -> merge-failed, never red"; else _bad "merge failure: rc=${rc} reason='${reason}'"; fi +if _called "pr edit"; then _bad "unexpected pr edit"; else _ok "no label edit when none are stale"; fi + +# --- label step -------------------------------------------------------- +label() { # label REASON + rc=0 + out="$(PATH="${here}/stub-dependabot:${PATH}" GH_REPO=acme/site \ + PR_URL=https://github.com/acme/site/pull/7 REASON="$1" \ + bash -c "${label_sh}" 2>&1)" || rc=$? +} +_fixture +printf '%s\n' "dependabot-blocked" "dependabot-blocked:timeout" >"${STUB_DIR}/labels" +label red-check +if [[ "${rc}" -eq 0 ]] && _called "label create dependabot-blocked:red-check" \ + && _called "pr edit https://github.com/acme/site/pull/7 --add-label dependabot-blocked,dependabot-blocked:red-check --remove-label dependabot-blocked:timeout"; then + _ok "labels the reason and drops the stale reason" +else + _bad "label: rc=${rc} calls=$(<"${STUB_DIR}/calls.log")" +fi + +_fixture +label 'bogus;reason' +if _called "--add-label dependabot-blocked,dependabot-blocked:cannot-evaluate"; then + _ok "unknown reason -> cannot-evaluate" +else + _bad "unknown reason: $(<"${STUB_DIR}/calls.log")" +fi + +_fixture; : >"${STUB_DIR}/fail.pr-edit"; : >"${STUB_DIR}/fail.label-create" +label timeout +if [[ "${rc}" -eq 0 ]] && grep -qF "::warning::Could not label" <<<"${out}"; then + _ok "label failures warn, never red" +else + _bad "label failure: rc=${rc} out=${out}" +fi echo "${pass} passed, ${fail} failed" [[ "${fail}" -eq 0 ]]