Skip to content

Harden actions/checkout credential persistence across all workflows #318

Description

@doublegate

Problem

actions/checkout defaults to persist-credentials: true, which writes the workflow's GITHUB_TOKEN into .git/config as an http.https://github.com/.extraheader entry. Any code the job subsequently executes from the checkout — Cargo build scripts, proc macros, test binaries, Gradle build scripts, shell scripts under scripts/, the MkDocs build — can read that file.

On a pull request the checked-out tree is, by definition, code that has not yet been reviewed. Every CI job in this repo compiles and runs it.

Raised by CodeRabbit on #317. persist-credentials: false was applied there to the new libretro-cross job only (.github/workflows/ci.yml:347-349); this issue tracks the repo-wide sweep.

Why it matters here

  • For fork PRs GitHub forces a read-only token, which caps the damage. For same-repo branch PRs — how this repo actually operates — and for Dependabot PRs, the job's full declared permissions: apply.
  • .github/workflows/web.yml declares pages: write + id-token: write at the workflow level (L52-55). That block applies to the build job too, which runs on pull requests and executes trunk, cargo doc, pip install, mkdocs build, and a repo shell script. That is the highest-value token in the repo sitting in the job with the widest untrusted-code surface.
  • ci.yml:154 selects the LIGHT vs FULL matrix from github.head_ref. For a fork PR that value is attacker-chosen, so a fork branch named release/* reaches the test-roms and bench jobs as well.

Audit result

19 actions/checkout usages. .github/actions/rust-setup/action.yml performs no checkout of its own (it documents this at L5-6), so hardening at each call site is complete coverage.

There is exactly one Git network operation in the entire .github/ tree (release-auto.yml:88). No workflow pushes commits, tags, or branches. There are no submodules. GitHub Pages uses the OIDC flow (configure-pages@v6 / upload-pages-artifact@v5 / deploy-pages@v5), not a gh-pages branch push, and the deploy job has no checkout at all. gh release create (release-auto.yml:159) and softprops/action-gh-release@v3 (release.yml:150) authenticate via GH_TOKEN / GITHUB_TOKEN against the REST API and are unaffected. fastlane match (ios.yml:128) clones a different repository with its own MATCH_GIT_* credentials.

File Line Job Downstream Classification
web.yml 78 build trunk / cargo doc / pip / mkdocs; pages:write + id-token:write SAFE — harden (highest priority)
ci.yml 221 test cargo test --workspace (runs test binaries) SAFE — harden
android.yml 107 gradle-bundle ./gradlew (arbitrary JVM build scripts) SAFE — harden
ci.yml 170 lint 6x cargo clippy / cargo doc SAFE — harden
security.yml 83 clippy-security cargo clippy incl. vendored rcheevos C FFI SAFE — harden
android.yml 42 cross-build cargo ndk build, cargo run ... uniffi-bindgen SAFE — harden
ci.yml 290 wasm 3x cargo clippy --target wasm32 SAFE — harden
ci.yml 272 no_std cargo build --target thumbv7em-none-eabihf SAFE — harden
ci.yml 404 bench scripts/bench_regression_check.sh SAFE — harden
ci.yml 248 test-roms cargo test --release --features test-roms SAFE — harden
security.yml 55 audit prebuilt cargo-audit; no compilation SAFE — harden (low value)
security.yml 70 deny prebuilt cargo-deny; no compilation SAFE — harden (low value)
release.yml 89 build cargo build --release; release upload is API-based SAFE — harden (not PR-reachable; contents: write)
pgo.yml 76 pgo scripts/pgo/run.sh, cargo pgo SAFE — harden (not PR-reachable)
pgo.yml 195 bolt same SAFE — harden (not PR-reachable)
ios.yml 45 testflight xcframework build; fastlane match uses its own creds SAFE — harden (not PR-reachable)
ci.yml 89 changes dorny/paths-filter@v4 only; has fetch-depth: 0 SAFE — harden, verify (see below)
ci.yml 347 libretro-cross cargo check --release -p rustynes-libretro ALREADY HARDENED (#317)
release-auto.yml 67 prepare reads Cargo.toml/CHANGELOG; git ls-remote origin at L88 LEAVE AS-IS — see below

Proposed fix

Add to each site listed as "harden":

      - uses: actions/checkout@v7
        with:
          persist-credentials: false

ci.yml:89 already has a with: block; merge the key in rather than replacing it:

      - uses: actions/checkout@v7
        with:
          fetch-depth: 0
          persist-credentials: false

release-auto.yml:67 is deliberately excluded. It is unreachable from untrusted input (workflow_run, further gated to event == 'push' at L61), executes no repository code, and is the only job with a real Git network operation. An inline comment should record why, so a future blanket sweep does not silently break the release path.

Converting that git ls-remote to gh api repos/$GITHUB_REPOSITORY/git/ref/tags/$tag — which would also fix the latent bug where >/dev/null 2>&1 makes a transient failure indistinguishable from "tag absent", causing the workflow to attempt a release for a tag that already exists — is a worthwhile but separate change.

Verification

  • CI green on the PR covers 10 of the changed sites directly.
  • web.yml build runs on PRs; deploy is correctly skipped by L178.
  • android.yml is paths-gated — trigger it via workflow_dispatch post-merge.
  • Confirm a docs-only push to main still skips the heavy jobs, validating that dorny/paths-filter on the push path is unaffected (ci.yml:89). Its failure mode is benign — it would report "everything changed" — but it would cost the docs-only skip that ci.yml:84-88 exists to preserve.
  • release.yml is only exercised at a tag or workflow_dispatch; smoke-run it against an existing tag before the next cut, as it is the one changed job with contents: write.

Out of scope

  • zizmor unpinned-uses (no action pinned to a commit SHA). Different risk class, and SHA pinning is a maintenance commitment that requires enabling the github-actions Dependabot ecosystem to avoid trading a supply-chain risk for a stale-action risk.
    • One sub-item is worth doing on its own and soon: .github/actions/rust-setup/action.yml:41 uses dtolnay/rust-toolchain@master — a branch ref, which advances on every upstream commit, unlike the @vN tags used everywhere else. That composite is consumed by 12 of the 19 checkouts, including release.yml (contents: write) and web.yml (pages: write + id-token: write), and it is the action that installs the compiler. Upstream publishes a v1 tag, so @v1 or a SHA pin is a one-line fix.
  • Converting release-auto.yml's tag-existence check from git ls-remote to gh api.
  • Any change to workflow permissions: blocks.

Related findings (not part of this issue, recorded so they are not lost)

  • ci.yml:154 derives the LIGHT/FULL matrix decision from github.head_ref, which is attacker-chosen on fork PRs; a fork branch named release/* promotes the run to FULL. The comment at L128-134 implies those jobs are less reachable than they are.
  • android.yml's gradle-bundle job runs arbitrary Gradle build scripts under continue-on-error: true (L104), so a side-effect-only compromise would not turn the check red.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions