From fe020c6d7ba0563ecb99e88435c3b61aec6afb0e Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Mon, 20 Jul 2026 22:02:51 -0400 Subject: [PATCH 1/2] security(ci): drop persisted Git credentials from compiling checkouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #318. actions/checkout defaults to persist-credentials: true, which writes the workflow GITHUB_TOKEN into .git/config as an http.https://github.com/.extraheader entry. Any code the job subsequently executes from the checkout can read it: Cargo build scripts, proc macros, test binaries, Gradle build scripts, scripts/*.sh, the MkDocs build. On a pull request that tree is by definition unreviewed code, and every CI job in this repo compiles and runs it. Raised by CodeRabbit on #317, where the fix was applied to the one new job; this is the repo-wide sweep. Applied to 18 of the 19 checkouts under .github/. Audited rather than applied blanket. Three findings shaped the result: 1. .github/actions/rust-setup performs no checkout of its own (it says so at L5-6 and is used by 12 call sites), so hardening at each call site is complete coverage — there is no hidden checkout to miss. 2. No job actually needed the persisted credential. There is exactly ONE Git network operation in the whole .github/ tree. Nothing pushes commits, tags, or branches; there are no submodules; GitHub Pages uses the OIDC flow (configure-pages/upload-pages-artifact/deploy-pages) rather than a gh-pages branch push, and its deploy job has no checkout at all. The usual false positives were each checked and dismissed: `gh release create` and softprops/action-gh-release authenticate via GH_TOKEN against the REST API, and `fastlane match` clones a DIFFERENT repository using its own MATCH_GIT_* credentials. persist-credentials only affects git network operations relying on the stored credential — not the gh CLI, not the API. 3. The highest-exposure site is not a ci.yml job. web.yml declares pages: write + id-token: write at the WORKFLOW level, so that block also covers its `build` job — which is pull-request-reachable and runs trunk, cargo doc, pip install, and mkdocs build. That is the most valuable token in the repo sitting in the job with the widest untrusted-code surface. The single deliberate exception is release-auto.yml's `prepare`, which keeps its credentials and now carries an inline comment explaining why. It holds the tree's only real Git network operation (git ls-remote --tags origin), it is unreachable from untrusted input (workflow_run, further gated to event == 'push'), and it executes no repository code at all — it reads two text files. Hardening it would buy nothing while risking a release path that is only exercised at a version cut. The comment also records the safe migration order for anyone later wanting uniformity: convert the ls-remote to `gh api .../git/ref/tags/$tag` FIRST, then drop the credentials. That swap additionally fixes a latent bug, since the current `>/dev/null 2>&1` makes a transient failure indistinguishable from "tag does not exist" and would publish a duplicate release. ci.yml's `changes` job is the one place the change touches the git layer at all, since dorny/paths-filter reads local git on push events. It needs no authentication (fetch-depth: 0 means the base commit is already local, and the pull_request path uses the GitHub API), so this is expected to be inert; an inline comment records the reasoning and the benign failure mode. Deliberately NOT included: zizmor's unpinned-uses finding. SHA-pinning every action is a different risk class and a maintenance commitment that needs the github-actions Dependabot ecosystem enabled first, or it trades a supply-chain risk for a stale-action risk. One sub-item is worth doing soon on its own: .github/actions/rust-setup pins dtolnay/rust-toolchain@master — a branch ref that advances on every upstream commit, unlike the @vN tags used everywhere else — and that composite feeds 12 of the 19 checkouts including release.yml (contents: write) and web.yml (pages: write). Tracked in #318. Purely additive CI configuration: no source, build output, or emulation behavior changes. --- .github/workflows/android.yml | 4 ++++ .github/workflows/ci.yml | 22 ++++++++++++++++++++++ .github/workflows/ios.yml | 2 ++ .github/workflows/pgo.yml | 4 ++++ .github/workflows/release-auto.yml | 16 ++++++++++++++++ .github/workflows/release.yml | 2 ++ .github/workflows/security.yml | 6 ++++++ .github/workflows/web.yml | 2 ++ CHANGELOG.md | 29 +++++++++++++++++++++++++++++ 9 files changed, 87 insertions(+) diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index c602cb07..ff490e9c 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -40,6 +40,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + persist-credentials: false # The project toolchain (rust-toolchain.toml = 1.96) plus the two shipped # Android targets (arm64 ships; x86_64 is the emulator/CI ABI). @@ -105,6 +107,8 @@ jobs: needs: cross-build steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - name: Install Android Rust targets run: rustup target add aarch64-linux-android x86_64-linux-android - uses: Swatinem/rust-cache@v2 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a0072b6f..65ef911e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -86,9 +86,19 @@ jobs: # the base commit. (On `pull_request` it uses the GitHub API and needs no # history.) Without this the push-event filter falls back to "everything # changed" — safe, but it loses the docs-only skip on direct main pushes. + # + # This is the one job in the credential-hardening sweep (#318) where the + # change touches the git layer at all: paths-filter reads local git on + # `push` events. It needs no authentication — `fetch-depth: 0` means the + # base commit is already local, and the `pull_request` path goes through + # the GitHub API with its own token — so dropping the persisted credential + # is expected to be inert. Failure mode if that is ever wrong is benign + # (the filter reports "everything changed" and the full suite runs), but + # it would silently cost the docs-only skip the comment above describes. - uses: actions/checkout@v7 with: fetch-depth: 0 + persist-credentials: false - uses: dorny/paths-filter@v4 id: filter with: @@ -168,6 +178,8 @@ jobs: RUSTDOCFLAGS: -D warnings steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - uses: ./.github/actions/rust-setup with: toolchain: 1.96.0 @@ -219,6 +231,8 @@ jobs: os: ${{ fromJSON(needs.setup.outputs.os) }} steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - uses: ./.github/actions/rust-setup with: cache-key: test @@ -246,6 +260,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - uses: ./.github/actions/rust-setup with: cache-key: test-roms @@ -270,6 +286,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - uses: ./.github/actions/rust-setup with: targets: thumbv7em-none-eabihf @@ -288,6 +306,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - uses: ./.github/actions/rust-setup with: targets: wasm32-unknown-unknown @@ -402,6 +422,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - uses: ./.github/actions/rust-setup with: apt: "false" diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index 84ee0a28..d5a68e54 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -43,6 +43,8 @@ jobs: if: ${{ github.event_name != 'schedule' || vars.IOS_SIGNING_READY == 'true' }} steps: - uses: actions/checkout@v7 + with: + persist-credentials: false # v2.0.7 "Trim" — App Store submission floor. From 2026-04-28 every App Store # Connect upload must be built with the iOS 26 SDK (Xcode 26). Select the newest diff --git a/.github/workflows/pgo.yml b/.github/workflows/pgo.yml index 3eb3cf45..13e4da52 100644 --- a/.github/workflows/pgo.yml +++ b/.github/workflows/pgo.yml @@ -74,6 +74,8 @@ jobs: speedup_pct: ${{ steps.gate.outputs.speedup_pct }} steps: - uses: actions/checkout@v7 + with: + persist-credentials: false # Pinned to the project toolchain (rust-toolchain.toml = 1.96.0), matching # the other jobs. PGO needs the llvm-tools-preview component (profdata @@ -193,6 +195,8 @@ jobs: contents: read steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - uses: ./.github/actions/rust-setup with: toolchain: 1.96.0 diff --git a/.github/workflows/release-auto.yml b/.github/workflows/release-auto.yml index 9eeb411f..2d344d8c 100644 --- a/.github/workflows/release-auto.yml +++ b/.github/workflows/release-auto.yml @@ -70,6 +70,22 @@ jobs: # checkout suffices: we read Cargo.toml + CHANGELOG.md, and the tag # existence check uses `git ls-remote` (no local tag history needed). 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 real + # Git network operation in the whole `.github/` tree, and this job is + # unreachable from untrusted input — `workflow_run`, further gated to + # `event == 'push'` above — and executes no repository code at all: it + # reads two text files. 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 would publish a duplicate release. - name: Decide whether a new version needs releasing id: decide diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 28cc7695..809301dc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -87,6 +87,8 @@ jobs: bin_name: rustynes.exe steps: - uses: actions/checkout@v7 + with: + persist-credentials: false # Shared toolchain + Linux deps (no-op on macOS/Windows) + cargo cache. # The per-target cache key keeps each platform's artifacts separate. diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 1abd0edb..a152767b 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -53,6 +53,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + persist-credentials: false # Install the PREBUILT binary, never `cargo install` (build-from-source): # the repo pins rustc 1.96 (rust-toolchain.toml), but the current # cargo-audit release needs >= 1.88 to COMPILE. The prebuilt binary RUNS @@ -68,6 +70,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + persist-credentials: false # Prebuilt binary for the same reason as the audit job: the latest # cargo-deny needs rustc >= 1.88 to build from source, while the repo is # pinned to 1.96. Policy lives in `deny.toml`. @@ -81,6 +85,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - uses: ./.github/actions/rust-setup with: toolchain: 1.96.0 diff --git a/.github/workflows/web.yml b/.github/workflows/web.yml index b8671105..b7215c44 100644 --- a/.github/workflows/web.yml +++ b/.github/workflows/web.yml @@ -76,6 +76,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + persist-credentials: false # Shared toolchain + cargo cache. `apt: true` because the rustdoc build # compiles the whole workspace (incl. rustynes-frontend) for the host, diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f066a3f..9a497777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,35 @@ cycle-accurate core later replaced. `get_fastforwarding`-gated audio-push skip during RetroArch's fast-forward/rollback-netplay catch-up path. +### Security + +- **`persist-credentials: false` on every CI checkout that compiles or runs + repository code (closes #318).** `actions/checkout` defaults to writing the + workflow `GITHUB_TOKEN` into `.git/config`, where any code the job then + executes from the checkout — Cargo build scripts, proc macros, test + binaries, Gradle build scripts, `scripts/*.sh`, the MkDocs build — can read + it. On a pull request that tree is by definition unreviewed code, and every + CI job compiles and runs it. Applied to **18 of the 19** checkouts in + `.github/`. Highest-exposure site was not a `ci.yml` job but `web.yml`'s + `build`, whose workflow-level `permissions:` grant `pages: write` + + `id-token: write` and which is PR-reachable while running `trunk`, + `cargo doc`, `pip install`, and `mkdocs build`. + Audited rather than applied blanket: `.github/actions/rust-setup` performs + no checkout of its own, so call-site hardening is complete coverage; and + **no** job actually needed the persisted credential — nothing pushes + commits, tags, or branches, there are no submodules, GitHub Pages uses the + OIDC flow (not a `gh-pages` push), and `gh release create` / + `softprops/action-gh-release` / `fastlane match` all authenticate by API + token or their own separate repository credentials. The single deliberate + exception is `release-auto.yml`'s `prepare`, the only job in the tree with a + real Git network operation (`git ls-remote --tags origin`); it is + unreachable from untrusted input and executes no repository code, so + hardening would buy nothing while risking a release path exercised only at a + version cut. Its checkout now carries an inline comment recording that, plus + the safe migration order for anyone wanting uniformity later. Purely + additive CI configuration — no source, build output, or emulation behavior + changes. + ### Changed - **Dependency version-bump consolidation (closes Dependabot #313–#315).** From 88e95af37f54139cdbd4ae349fdeedbf4a5c9c43 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Mon, 20 Jul 2026 22:11:52 -0400 Subject: [PATCH 2/2] docs(ci): tighten the credential-exception wording (review feedback) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three accuracy corrections from Copilot and CodeRabbit on #319, all adopted; none change behaviour. 1. "reads two text files" / "Cargo.toml + CHANGELOG.md" undercounted: the job also reads the optional .github/release-notes/vX.Y.Z.md override (L119). Both the pre-existing comment and mine now describe the set without pinning an exact count, so the security rationale cannot go stale the next time a file is added. 2. "the only real Git network operation in the whole .github/ tree" was too broad. fastlane match in ios.yml also clones over the network — it just uses its own MATCH_GIT_* secrets against a different remote, so it is unaffected by persist-credentials. Reworded to the claim that actually justifies the exception: the only operation needing THIS checkout's origin credential. The CHANGELOG had the mirrored problem, asserting no job needed the credential and then naming an exception; it now says every *hardened* job was confirmed not to need it. 3. The CHANGELOG claimed "every CI job compiles and runs" the checked-out tree. The audit and deny jobs do neither — they install prebuilt binaries and parse Cargo.lock — and overstating a security claim in release notes is exactly what should not happen. Also corrected a claim in my own comment: a masked ls-remote failure was described as publishing "a duplicate release". Per gh's documented behaviour it would instead fail outright when a release already exists for the tag, or publish against the pre-existing tag when one does not — noisy or wrong, but not silently duplicated. Declined in this PR (tracked in #318): converting the ls-remote to a fail-closed gh api call. It is a real latent bug, but it changes the release path, cannot be verified without a version cut, and does not belong in a credential-hardening change. The comment records the safe migration order. --- .github/workflows/release-auto.yml | 30 +++++++++++++++++++++--------- CHANGELOG.md | 21 ++++++++++++--------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/.github/workflows/release-auto.yml b/.github/workflows/release-auto.yml index 2d344d8c..276bf0ff 100644 --- a/.github/workflows/release-auto.yml +++ b/.github/workflows/release-auto.yml @@ -67,25 +67,37 @@ jobs: - uses: actions/checkout@v7 with: # Build the release from the exact commit CI went green on. A shallow - # checkout suffices: we read Cargo.toml + CHANGELOG.md, and the tag - # existence check uses `git ls-remote` (no local tag history needed). + # 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). 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 real - # Git network operation in the whole `.github/` tree, and this job is - # unreachable from untrusted input — `workflow_run`, further gated to - # `event == 'push'` above — and executes no repository code at all: it - # reads two text files. So hardening buys nothing here while risking a - # release path that is only exercised at a version cut. + # 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 would publish a duplicate release. + # "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. - name: Decide whether a new version needs releasing id: decide diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a497777..e7f36d91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,21 +43,24 @@ cycle-accurate core later replaced. workflow `GITHUB_TOKEN` into `.git/config`, where any code the job then executes from the checkout — Cargo build scripts, proc macros, test binaries, Gradle build scripts, `scripts/*.sh`, the MkDocs build — can read - it. On a pull request that tree is by definition unreviewed code, and every - CI job compiles and runs it. Applied to **18 of the 19** checkouts in + it. On a pull request that tree is by definition unreviewed code, and nearly + every CI job compiles or runs it — the exceptions being the `audit` / `deny` + jobs, which install prebuilt binaries and only parse `Cargo.lock`. Applied to + **18 of the 19** checkouts in `.github/`. Highest-exposure site was not a `ci.yml` job but `web.yml`'s `build`, whose workflow-level `permissions:` grant `pages: write` + `id-token: write` and which is PR-reachable while running `trunk`, `cargo doc`, `pip install`, and `mkdocs build`. Audited rather than applied blanket: `.github/actions/rust-setup` performs no checkout of its own, so call-site hardening is complete coverage; and - **no** job actually needed the persisted credential — nothing pushes - commits, tags, or branches, there are no submodules, GitHub Pages uses the - OIDC flow (not a `gh-pages` push), and `gh release create` / - `softprops/action-gh-release` / `fastlane match` all authenticate by API - token or their own separate repository credentials. The single deliberate - exception is `release-auto.yml`'s `prepare`, the only job in the tree with a - real Git network operation (`git ls-remote --tags origin`); it is + **every hardened job was confirmed not to need the checkout credential** — + nothing pushes commits, tags, or branches, there are no submodules, GitHub + Pages uses the OIDC flow (not a `gh-pages` push), and `gh release create` / + `softprops/action-gh-release` authenticate by API token while + `fastlane match` clones a different remote with its own `MATCH_GIT_*` + credentials. The single deliberate + exception is `release-auto.yml`'s `prepare`, the one job whose work needs + that credential (`git ls-remote --tags origin`); it is unreachable from untrusted input and executes no repository code, so hardening would buy nothing while risking a release path exercised only at a version cut. Its checkout now carries an inline comment recording that, plus