diff --git a/.github/workflows/renovate-hash-sync.yml b/.github/workflows/renovate-hash-sync.yml index 3658ccf..0208c19 100644 --- a/.github/workflows/renovate-hash-sync.yml +++ b/.github/workflows/renovate-hash-sync.yml @@ -70,22 +70,44 @@ # the first two; xow-firmware pins opaque Microsoft Update .cab GUIDs, # not a version). See docs/renovate.md. # -# TRIGGER: only for PRs opened by Renovate itself (github.actor check below), -# only on branches in THIS repo (never a fork -- pushing back requires -# write access the default token does not get on a fork-originated PR), and -# only when a file this workflow actually knows how to fix has changed. -# Idempotent by design: it always recomputes from the PR's current file -# content and only commits if the recomputed hash differs from what's -# already there, so re-runs (e.g. a second Renovate push to the same PR) -# are harmless no-ops once the hash is already correct. +# TRIGGER: automatically, only for PRs opened by Renovate itself (github.actor +# check below), only on branches in THIS repo (never a fork -- pushing back +# requires write access the default token does not get on a fork-originated +# PR), and only when a file this workflow actually knows how to fix has +# changed. Idempotent by design: it always recomputes from the branch's current +# file content and only commits if the recomputed hash differs from what's +# already there, so re-runs (e.g. a second Renovate push to the same PR) are +# harmless no-ops once the hash is already correct. # -# UNVERIFIED: like renovate.json itself, this workflow has never been -# exercised against a real Renovate PR (Renovate is installed and live -# since 2026-07-14, but no custom manager has produced a real PR yet -- -# see docs/renovate.md's "Unverified / what to check on first run"). -# Treat the exact regexes/URLs below -- including the libchdr loop entry -# and the bespoke lzma-sdk and sdcard-payload steps -- as reviewed-by-hand, -# not proven. +# ...and manually, via workflow_dispatch with a `branch` input. That escape +# hatch exists because none of the automatic machinery can re-drive this +# workflow after a bug in it is fixed: +# +# * the paths: filter above does not list this file, so pushing a fix to it +# triggers nothing; +# * the actor gate skips the job when a human pushes the fix to the branch; +# * and a re-run replays the workflow definition from the commit the original +# run was created against, so it re-executes the OLD code. +# +# All three held simultaneously on run 29669946883, which ran the same broken +# kernel-URL code three times across two separate fixes. When dispatching, +# note that the ref you dispatch FROM (which supplies this file) and the +# `branch` you dispatch AT (which gets checked out and pushed to) are +# deliberately different things. +# +# PARTIALLY VERIFIED (updated 2026-07-19): the kernel step HAS now been +# exercised against a real Renovate PR (#41, kernel 6.18.38 -> 6.18.39) and +# works end to end. That first run also found two bugs this header used to +# understate: an unanchored defconfig grep that built a URL containing a +# newline (#42), and the fact that a fetch failure here is only a ::warning:: +# -- so the job reported SUCCESS three times while silently leaving +# linux.hash stale. Read every warn-and-skip path below as "this can go green +# without doing anything". +# +# Still UNPROVEN: the generic github-package loop (including the libchdr +# entry) and the bespoke lzma-sdk and sdcard-payload steps have never run +# against a real PR. Treat those regexes/URLs as reviewed-by-hand, not +# proven -- see docs/renovate.md's "Unverified / what to check on first run". # ################################################################################ @@ -111,8 +133,31 @@ on: - "package/lzma-sdk/lzma-sdk.mk" - "scripts/fetch-sdcard-payload.sh" + # Manual escape hatch. Needed because NOTHING else can re-drive this workflow + # after a fix lands: the pull_request trigger's `paths:` filter does not list + # this file (so pushing a fix to it triggers nothing), the job is gated on + # actor == renovate[bot] (so a maintainer's own push is skipped), and a + # re-run replays the workflow definition from the commit the original run was + # created against -- which is how run 29669946883 executed the same broken + # code three times across two fixes. + # + # IMPORTANT: dispatch from a branch whose copy of THIS FILE is the one you + # want to run (normally the default branch), and name the branch you want + # FIXED in the `branch` input. Those are two different things, and conflating + # them is the trap above: GitHub runs the workflow definition from the ref you + # dispatch on, while this workflow edits and pushes to `inputs.branch`. + workflow_dispatch: + inputs: + branch: + description: "Plain branch name to refresh companion hashes on (e.g. renovate/kernel-longterm-6.18-6.x). NOT the branch you are dispatching from, and NOT a ref -- refs/heads/x and heads/x are rejected." + required: true + type: string + concurrency: - group: renovate-hash-sync-${{ github.event.pull_request.number }} + # pull_request runs key on the PR number; dispatch runs key on the target + # branch. Without the fallback every manual run would share one empty-suffix + # group and cancel the previous one. + group: renovate-hash-sync-${{ github.event.pull_request.number || inputs.branch }} cancel-in-progress: true permissions: @@ -120,24 +165,92 @@ permissions: jobs: hash-sync: - name: Refresh companion hashes for this PR's version bump(s) + name: Refresh companion hashes for this branch's version bump(s) runs-on: ubuntu-26.04 - # Only Renovate's own PRs, and never a fork (pushing back needs write - # access to the head branch, which a fork PR's default token never has). + # Automatic runs: only Renovate's own PRs, and never a fork (pushing back + # needs write access to the head branch, which a fork PR's default token + # never has). Manual runs: workflow_dispatch is already restricted to + # collaborators with write access, which is the same bar this gate enforces + # for the automatic path -- so it does not widen who can drive this. if: > - github.actor == 'renovate[bot]' && - github.event.pull_request.head.repo.full_name == github.repository + github.event_name == 'workflow_dispatch' || + (github.actor == 'renovate[bot]' && + github.event.pull_request.head.repo.full_name == github.repository) timeout-minutes: 10 + # Resolved once, here, rather than repeated at each use site. Passing it to + # the push step as a shell variable (not an inline ${{ }} expansion) also + # closes the script-injection vector actionlint flags for + # github.event.pull_request.head.ref -- a branch name is attacker-chosen + # text on a PR from an untrusted contributor. + env: + TARGET_BRANCH: ${{ github.event_name == 'workflow_dispatch' && inputs.branch || github.event.pull_request.head.ref }} + steps: + # A manual run pushes commits to whatever branch it is pointed at, so + # refuse the default branch outright: that would land an auto-generated + # hash commit on master with no review. The automatic path cannot hit + # this (a PR head branch is never the base), but a mistyped input can. + - name: Validate the target branch, and refuse the default branch + env: + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: | + set -euo pipefail + + if [ -z "$TARGET_BRANCH" ]; then + echo "::error::target branch resolved to empty -- nothing to check out." >&2 + exit 1 + fi + + # Syntactic validation first: rejects empty, embedded spaces, "..", + # a leading "-", and the other things git considers malformed. Runs + # before checkout, which is fine -- check-ref-format needs no repo. + if ! git check-ref-format --branch "$TARGET_BRANCH" >/dev/null 2>&1; then + echo "::error::'$TARGET_BRANCH' is not a valid git branch name." >&2 + exit 1 + fi + + # REJECT ref-namespace forms rather than normalising them away. git + # resolves several spellings to the same branch, so a plain string + # comparison against the default branch is only sound once these are + # excluded. Verified with `git push --dry-run`, all three reach master: + # + # master -> master (caught by the compare below) + # refs/heads/master -> master (bypasses a naive compare) + # heads/master -> master (bypasses it too) + # + # Rejecting is deliberate, not laziness: TARGET_BRANCH is consumed by + # BOTH actions/checkout and the final `git push`, so the value that + # gets validated here must be the exact value those steps use. + # Normalising would mean re-exporting through $GITHUB_ENV and relying + # on it overriding the job-level env: for later steps -- a subtlety + # not worth betting a force-push to the default branch on. + case "$TARGET_BRANCH" in + refs/*|heads/*|remotes/*|tags/*) + echo "::error::pass a plain branch name, not a ref: '$TARGET_BRANCH'." >&2 + echo "::error::git resolves refs/heads/ and heads/ to the same branch as , which would slip past the default-branch check below." >&2 + exit 1 + ;; + esac + + if [ "$TARGET_BRANCH" = "$DEFAULT_BRANCH" ]; then + echo "::error::refusing to run against the default branch ('$TARGET_BRANCH') -- this workflow commits and pushes hash fixes, which must go through a PR." >&2 + exit 1 + fi + + echo "target branch: $TARGET_BRANCH" + # Same pin as every other workflow in this repo (build.yml et al.) -- # kept in sync so Renovate's own github-actions manager (renovate.json) # tracks exactly one actions/checkout dependency, not two drifting # copies. - - name: Checkout PR head branch + - name: Checkout the target branch uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: - ref: ${{ github.event.pull_request.head.ref }} + # The PR head branch, or the dispatch input -- NOT the ref this + # workflow definition came from. On a manual run those differ by + # design (see the workflow_dispatch note above). + ref: ${{ env.TARGET_BRANCH }} fetch-depth: 1 # --- 1. Github-sourced packages (drivers/firmware + libchdr) ----------- @@ -445,4 +558,9 @@ jobs: comment. That value must come from Buildroot's GPG-signed .sign manifest and is a manual step. COMMITMSG - git push origin "HEAD:${{ github.event.pull_request.head.ref }}" + # Shell variable, NOT a workflow-expression interpolation: a branch + # name is attacker-controlled text on a PR from an untrusted + # contributor, and expanding it into the script body before bash sees + # it is a command-injection vector (actionlint flags exactly this). + # "--" stops a branch named like an option from being parsed as one. + git push origin -- "HEAD:$TARGET_BRANCH" diff --git a/docs/renovate.md b/docs/renovate.md index 4a31443..a85b4aa 100644 --- a/docs/renovate.md +++ b/docs/renovate.md @@ -108,8 +108,50 @@ the one case where it deliberately does nothing. **Triggers on**: `pull_request` (opened/synchronize), only when `github.actor == 'renovate[bot]'` and the PR's head repo is this repo (never a fork — a fork PR's default token cannot push back to it anyway). It always -recomputes from the PR's current file content and only commits if the result -actually differs, so re-runs on an already-correct PR are harmless no-ops. +recomputes from the branch's current file content and only commits if the +result actually differs, so re-runs on an already-correct PR are harmless +no-ops. + +**...and manually**, via `workflow_dispatch` with a required `branch` input. + +This escape hatch is not a convenience — without it there is **no way at all** +to re-drive this workflow after fixing a bug in it. Three things block every +other route, and all three held at once on +[run 29669946883](https://github.com/mcfbytes/Buildroot_MiSTer/actions/runs/29669946883), +which executed the same broken kernel-URL code three times across two separate +fixes: + +| route | why it fails | +|---|---| +| push the fix to the branch | the `paths:` filter doesn't list the workflow file, so nothing triggers | +| a maintainer merges the fix in | the `renovate[bot]` actor gate skips the job | +| re-run the failed run | a re-run replays the workflow definition from the commit the *original* run was created against — so it re-executes the old code | + +Only Renovate itself moving the branch head, or this manual trigger, gets new +code to run. + +> **The ref you dispatch *from* and the branch you dispatch *at* are different +> things, deliberately.** GitHub runs the workflow definition from the ref you +> dispatch on, while this workflow checks out and pushes to `inputs.branch`. +> So dispatch **from the default branch** (which has the fixed workflow) and +> put the branch you want *repaired* — e.g. +> `renovate/kernel-longterm-6.18-6.x` — in the input. Dispatching *on* the +> stale branch would just run its stale copy again, which is the whole trap. + +Manual runs refuse to target the default branch: this workflow commits and +pushes, and an auto-generated hash commit must go through a PR. `workflow_dispatch` +is already restricted to collaborators with write access, so this does not +widen who can drive the workflow. + +The input must be a **plain branch name**, not a ref. git resolves several +spellings to the same branch — `master`, `refs/heads/master` and `heads/master` +all push to `master` (verified with `git push --dry-run`) — so a naive string +comparison against the default branch is only sound once the ref forms are +excluded. They are rejected outright rather than normalised, because the same +value is consumed by both `actions/checkout` and the final `git push`: what gets +validated has to be exactly what those steps use. The name is also run through +`git check-ref-format --branch`, which rejects embedded spaces, `..`, a leading +`-`, and the rest. **What it fixes automatically, and why each case is safe:**