security(ci): fail-closed release tag check + pin the toolchain action

.github/actions/rust-setup/action.yml

@@ -37,8 +37,23 @@ inputs:
 runs:
   using: composite
   steps:
+    # Pinned to a commit SHA, not `@master`. Every other action in this repo is
+    # referenced by a `@vN` tag, which is not expected to move; `@master` is a
+    # BRANCH that advances on every upstream commit, so each run silently
+    # resolved to whatever HEAD happened to be — no review window, no signal
+    # that anything changed. That matters disproportionately here: this
+    # composite is consumed by 12 of the repo's 19 checkouts, including
+    # `release.yml` (`contents: write`, builds the shipped binaries) and
+    # `web.yml` (`pages: write` + `id-token: write`), and it is the action that
+    # installs the compiler — a position where a compromise is very hard to
+    # detect from the build output.
+    #
+    # The trailing `# v1` is the convention Dependabot's `github-actions`
+    # ecosystem (already enabled in `.github/dependabot.yml`) reads to keep the
+    # pin current, so this does not trade a supply-chain risk for a stale-action
+    # one. Verified: `refs/tags/v1` -> e97e2d8c at time of pinning.
     - name: Install Rust toolchain
-      uses: dtolnay/rust-toolchain@master
+      uses: dtolnay/rust-toolchain@e97e2d8cc328f1b50210efc529dca0028893a2d9 # v1
       with:
         toolchain: ${{ inputs.toolchain }}
         targets: ${{ inputs.targets }}

.github/workflows/release-auto.yml

@@ -69,39 +69,22 @@ jobs:
           # Build the release from the exact commit CI went green on. A shallow
           # checkout suffices: we only read plain text files (Cargo.toml,
           # CHANGELOG.md, and the optional `.github/release-notes/vX.Y.Z.md`
-          # override), and the tag existence check uses `git ls-remote` (no
-          # local tag history needed).
+          # override), and the tag existence check is an API call, not a Git
+          # operation.
           ref: ${{ github.event.workflow_run.head_sha }}
-          #
-          # DELIBERATELY the one checkout in this repo that KEEPS persisted Git
-          # credentials (every other one sets `persist-credentials: false`; see
-          # issue #318). The `git ls-remote --tags origin` below is the only
-          # operation in this repo that needs THIS checkout's `origin`
-          # credential. (It is not the only Git-over-network call in `.github/`
-          # — `fastlane match` in `ios.yml` clones the signing repository — but
-          # that one authenticates with its own `MATCH_GIT_*` secrets against a
-          # different remote, so it is unaffected by this setting.) This job is
-          # also unreachable from untrusted input — `workflow_run`, further
-          # gated to `event == 'push'` above — and executes no repository code
-          # at all: it only reads the text files named above. So hardening buys
-          # nothing here while risking a release path that is only exercised at
-          # a version cut.
-          #
-          # If a future sweep wants uniformity, first replace the `ls-remote`
-          # with `gh api repos/$GITHUB_REPOSITORY/git/ref/tags/$tag` (which uses
-          # GH_TOKEN and is unaffected by credential persistence), THEN drop the
-          # credentials. That swap also fixes a latent bug: the `>/dev/null 2>&1`
-          # below makes a transient network/auth failure indistinguishable from
-          # "tag does not exist", so a blip sends the workflow down the
-          # should_release=true path for a version that is already tagged.
-          # `gh release create` then either fails outright (if a release already
-          # exists for that tag) or publishes a release against the pre-existing
-          # tag — noisy or wrong, but not silently duplicated. Fail-closed
-          # handling belongs with that change, not here.
+          # Completes the #318 sweep — this was the last checkout in the repo
+          # still persisting credentials, held back only because the tag check
+          # used `git ls-remote origin`. That check is now `gh api` (see the
+          # step below), which authenticates with GH_TOKEN and needs nothing
+          # from `.git/config`, so the exception no longer has a reason to
+          # exist and all 19 checkouts are uniform.
+          persist-credentials: false
 
       - name: Decide whether a new version needs releasing
         id: decide
         shell: bash
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
           set -euo pipefail
           # Workspace version from [workspace.package] in Cargo.toml.
@@ -113,13 +96,46 @@ jobs:
           tag="v${version}"
           echo "tag=${tag}" >> "$GITHUB_OUTPUT"
           echo "version=${version}" >> "$GITHUB_OUTPUT"
-          if git ls-remote --exit-code --tags origin "refs/tags/${tag}" >/dev/null 2>&1; then
-            echo "Tag ${tag} already exists - nothing to release."
-            echo "should_release=false" >> "$GITHUB_OUTPUT"
-          else
-            echo "Version ${version} has no ${tag} tag yet - will release."
-            echo "should_release=true" >> "$GITHUB_OUTPUT"
-          fi
+
+          # Tag-existence check, FAIL-CLOSED. The previous implementation was
+          #   git ls-remote --exit-code --tags origin "refs/tags/$tag" >/dev/null 2>&1
+          # which collapsed three distinct outcomes into two: tag present, tag
+          # absent, and *lookup failed* all became a simple true/false, with any
+          # non-zero exit read as "absent". A transient network or auth blip
+          # therefore pushed an already-released version down the
+          # should_release=true path. This decides the entire release, so
+          # guessing is the one thing it must not do.
+          #
+          # `git/matching-refs` is used rather than `git/ref/tags/$tag` because
+          # it answers "absent" with HTTP 200 and an empty array instead of a
+          # 404 — so a genuine miss never looks like an error, and no error-body
+          # parsing is needed to tell them apart. It matches by PREFIX, so
+          # `tags/v2.2.1` would also return `v2.2.10`; the jq filter compares the
+          # full ref for exactness.
+          #
+          # Everything here fails closed under `set -euo pipefail`: a gh/API
+          # failure, malformed JSON, or an unexpected count aborts the job
+          # rather than resolving to a release decision. It also needs no Git
+          # credentials, which is what let the checkout above join the rest of
+          # the #318 sweep.
+          refs_json="$(gh api "repos/${GITHUB_REPOSITORY}/git/matching-refs/tags/${tag}")"
+          count="$(printf '%s\n' "$refs_json" \
+            | jq --arg r "refs/tags/${tag}" '[.[] | select(.ref == $r)] | length')"
+          case "$count" in
+            0)
+              echo "Version ${version} has no ${tag} tag yet - will release."
+              echo "should_release=true" >> "$GITHUB_OUTPUT"
+              ;;
+            1)
+              echo "Tag ${tag} already exists - nothing to release."
+              echo "should_release=false" >> "$GITHUB_OUTPUT"
+              ;;
+            *)
+              echo "::error::Unexpected match count (${count}) for refs/tags/${tag} - refusing to guess."
+              printf '%s\n' "$refs_json" >&2
+              exit 1
+              ;;
+          esac
 
       - name: Resolve release notes + title
         if: steps.decide.outputs.should_release == 'true'

AGENTS.md

@@ -189,6 +189,10 @@ These cross-cutting decisions span multiple files. Reading individual chip docs
 - **GitHub Wiki Initialization:** When assisting with GitHub Wiki deployments for the first time, instruct the user to click "Create the first page" in the GitHub UI to provision the `.wiki.git` repository. If the Wiki is cloned locally inside the main repository, ensure its folder (e.g., `RustyNES.wiki/`) is added to `.gitignore`.
 - **Symlinked Agent Configs:** Ensure symlinked agent files (like `GEMINI.md` -> `AGENTS.md`) are explicitly removed from `.gitignore` so they are correctly tracked by version control.
 - **Libretro buildbot CI (issue #311) shipped in PR #312 (2026-07-19/20, `b49dd1e0`) — issue #311 stays OPEN by design.** `.gitlab-ci.yml` + a `[lib] name = "rustynes"` naming-collision fix + `[workspace] default-members` + RA memory-maps + an FDS load-path fix/disk-control + native Game Genie cheats all shipped. TAS and Netplay needed no new libretro-side wiring — RetroArch's own rollback/movie machinery already rides the existing `on_serialize`/`on_unserialize` hooks. Companion upstream PRs `libretro/docs#1164` and `libretro/libretro-super#2021` are open but not yet merged by the libretro team; the remaining step (mirroring the repo onto `git.libretro.com` + enabling the buildbot) is entirely on their side. **Never close #311** with a "Closes #311" commit-body keyword — it already auto-closed prematurely once this way and had to be reopened; only close it once the upstream team confirms the buildbot is live.
+- **GENERAL RULE (learned twice, the hard way): never put a GitHub magic closing keyword — `Closes #N` / `Fixes #N` / `Resolves #N` — in a commit or PR body when the issue tracks ANY work the change does not finish.** GitHub closes the issue the instant the PR merges, marks it `stateReason: COMPLETED`, and the tracker then reports finished work that was never done. This has now fired twice: #311 (PR #312), and again on #318, which auto-closed one second after PR #319 merged (`02:42:15Z` merge → `02:42:16Z` close) even though #319's own body and review replies explicitly deferred two of its items. Write "addresses #N" / "partially addresses #N" in prose instead, and close the issue by hand only once its last item actually lands. When an issue *looks* completed, verify against the tree (`git log`, grep the code) rather than trusting the CLOSED/COMPLETED state — a keyword-closed issue is indistinguishable from a genuinely finished one in the UI.
+- **CI security hardening (PRs #319 + #320, 2026-07-21, merged `85ee20db` / `a69200ef`).** `persist-credentials: false` on **all 19** `actions/checkout` sites (18 in #319; the last one, `release-auto.yml`, once its tag check stopped needing Git credentials — see the next bullet), because build scripts / proc macros / test binaries / Gradle scripts / MkDocs all execute unreviewed PR code that could read the token out of `.git/config`. Facts worth not re-deriving: `.github/actions/rust-setup` performs **no checkout of its own** (so call-site hardening is complete coverage); `persist-credentials` does **not** affect the `gh` CLI or API calls, only git network ops using the stored credential — which is why `gh release create`, `softprops/action-gh-release`, and `fastlane match` (a *different* repo, own `MATCH_GIT_*` secrets) all look like they need it and don't; and the highest-exposure job is **`web.yml`'s `build`**, not any `ci.yml` job, because `web.yml` declares `pages: write` + `id-token: write` at *workflow* level. There are now **no exceptions**: `release-auto.yml`'s `prepare` was the last holdout (it needed `git ls-remote origin` for the tag check), and that check is now a `gh api` call, so its checkout joined the sweep.
+- **The release tag-existence check is FAIL-CLOSED — keep it that way.** `release-auto.yml`'s `decide` step queries `gh api repos/$GITHUB_REPOSITORY/git/matching-refs/tags/<tag>`, NOT `git/ref/tags/<tag>`: `matching-refs` answers "absent" with HTTP 200 + an empty array, so a genuine miss can never be confused with a lookup failure and no error-body parsing is needed. It matches by **prefix**, so the exact ref is compared in `jq` — this is load-bearing, not defensive: `v2.2` prefix-matches two real tags (`v2.2.0`, `v2.2.1`) while exact-matching none. Under `shell: bash` + `set -euo pipefail` both a `gh` failure and a non-array body abort the job (verified: exit 1 and exit 5 respectively). The old `git ls-remote ... >/dev/null 2>&1` read *any* non-zero exit as "tag absent", so a blip would try to re-release a shipped version. **Never reintroduce a form where a failed lookup is indistinguishable from "absent."** (Note when testing shell behavior locally: this harness's shell is zsh, whose `set -e` semantics for `var="$(cmd)"` differ from bash's — test with `bash -c` or you will get a false result.)
+- **`pre-commit run --all-files` REWRITES vendored/immutable trees — use `--files <changed files>` or a single named hook instead.** `trailing-whitespace` / `end-of-file-fixer` / `mixed-line-ending` *modify* files and, before PR #320, had no `exclude` at all: one `--all-files` run silently reformatted **41 files** across the vendored TriCNES C#, vendored rcheevos C, `ref-docs/`, an upstream font licence, and upstream test-ROM READMEs — destroying exactly the byte-identical-to-upstream property those trees exist for. `.markdownlintignore` covered them for markdownlint only. #320 added a shared `exclude` anchor across the three rewriting hooks, scoped deliberately **narrower** than `.markdownlintignore`: only content we did not author. Frozen-but-ours trees (`docs/archive/`, `to-dos/plans/`, `docs/monetization/`) stay in scope, since the invariant is "don't rewrite what we didn't write". If it happens anyway, revert **only** the unintended paths (never a blanket `git checkout`, and never including your own edits).
 - **The libretro buildbot is a THIRD CI system with its own rules — and the pinned toolchain fights it.** The first real run (pipeline #91899, 2026-07-20) passed 1 of 10 jobs. `rust-toolchain.toml`'s `channel = "1.96.0"` makes rustup install a *fresh* toolchain inside libretro's build image, bypassing the image's pre-provisioned cross targets, so 8 jobs died on `E0463: can't find crate for core`; each job in `.gitlab-ci.yml` now runs `rustup target add ${RUST_TARGET}` (NOT added to `rust-toolchain.toml`'s `targets` — that would cost every contributor and GH Actions job ~8 extra `rust-std` downloads). The Apple jobs must use `!reference` rather than `extends` for that, because GitLab's `extends` REPLACES array keys and would silently drop the templates' `SDKROOT`/`STRIP`/`CC`/`CXX` exports. tvOS is special twice over: its template hardcodes `cargo +nightly build -Zbuild-std`, which bypasses our channel pin onto the image's stale nightly (below our MSRV) *and* omits `panic_abort` that `[profile.release] panic = "abort"` needs — handled by refreshing nightly + `CARGO_PROFILE_RELEASE_PANIC=unwind` in that job (`CARGO_UNSTABLE_BUILD_STD` does NOT work: the CLI `-Z` flag wins). **A green GitHub Actions run does not imply a green buildbot** — the new `libretro-cross` CI job (one triple for each buildbot ABI family a Linux runner can model — MinGW-Windows and Android/NDK; the Apple families are deliberately excluded, as bindgen needs a real per-target sysroot and there is no Apple SDK on a Linux runner) is the early-warning gate; before touching anything libretro-related, cross-check `cargo check --release -p rustynes-libretro --target <triple>` locally.
 - **`rust-libretro 0.3.2` is unmaintained (no commit since 2023-02) and has a MinGW bug we work around.** It casts a keycode with `cfg(target_family = "windows")`, but C enum signedness follows the *ABI*: only **MSVC** gives plain enums `int` — under **MinGW** (`x86_64-pc-windows-gnu`, what the buildbot builds) bindgen emits `c_uint` and the crate fails `E0308`. `.cargo/config.toml`'s `[env] BINDGEN_EXTRA_CLANG_ARGS_x86_64_pc_windows_gnu = "--target=x86_64-pc-windows-msvc"` fixes it; the generated-bindings diff is 28 lines, all enum signedness. Don't "clean up" that env var without rebuilding for `x86_64-pc-windows-gnu`.
 - **CodeRabbit is now a 3rd automated PR review bot** (`.coderabbit.yaml`, added 2026-07-20 in PR #316), alongside gemini-code-assist and copilot-pull-request-reviewer — same reply-and-resolve-every-thread ceremony applies before any merge. Configured `profile: assertive` (not the "chill" default) and a `tools{}`/`path_instructions`/custom-checks set audited against this repo's actual file footprint, not guessed. `tone_instructions` has a hard 250-character schema limit that fails validation silently on the CodeRabbit side — after editing `.coderabbit.yaml`, verify with a `@coderabbitai configuration` PR comment and confirm every changed field shows `Source: Repository YAML (base)`.