Skip to content

Harden GitHub Actions workflows - #269

Open
hirthbrian wants to merge 1 commit into
TheOnlyZac:mainfrom
hirthbrian:fix/pull_request_target
Open

Harden GitHub Actions workflows#269
hirthbrian wants to merge 1 commit into
TheOnlyZac:mainfrom
hirthbrian:fix/pull_request_target

Conversation

@hirthbrian

Copy link
Copy Markdown

Both workflows

  • Add a top-level permissions: contents: read. On a public repo this grants nothing an anonymous clone doesn't already have, and it removes the default write-scoped token from a job that runs contributor-authored scripts.
  • persist-credentials: false on checkout, so no token is left behind in .git/config for build scripts to pick up.
  • Gate the PR job behind an environment: pr-build, so a maintainer has to approve each run. Approval is tied to the head SHA, so every push to a PR is re-approved rather than approved once.
  • Add a per-PR concurrency group with cancel-in-progress: true, so pushing repeatedly to a PR queues one pending approval instead of a stack of them.
  • Pass allow-unsafe-pr-checkout: true — an explicit acknowledgement that this checkout is untrusted PR code under pull_request_target, rather than relying on the action's default.

build.yml specifically

  • Split the single build job into build-main (push, trusted — the code is already on main) and build-pr (pull_request_target, untrusted). Only build-pr carries the environment gate and concurrency group; pushes to main still build without waiting on approval.
  • Bump actions/checkout@v2@v4.
  • The ref: ${{ github.event.pull_request.head.sha || github.sha }} fallback goes away — each job now has one event and one unambiguous ref.

Inline comments explaining the trusted/untrusted split are in both files, so the reasoning survives the next edit to these workflows.

Required repo setup before merging

The environment gate is inert without configuration. In Settings → Environments, create an environment named pr-build and add Required reviewers. Without a protection rule, environment: pr-build resolves and the job runs immediately, so the workflows would be no worse than today but the approval step would do nothing.

Notes

  • This does not switch the workflows away from pull_request_target. Moving to pull_request would drop the need for the approval gate, but PR builds would lose secrets.FILE_URL and couldn't fetch the game executable. Keeping pull_request_target + manual approval preserves the current developer experience.
  • doxygen.yml, frogress.yml, and progress.yml are untouched — none of them use pull_request_target.

Limit workflow permissions and separate trusted/untrusted builds. Both workflows now set contents: read and update actions/checkout to v4 with persist-credentials:false. build.yml splits into build-main (push/trusted) and build-pr (pull_request_target/untrusted) with environment pr-build, concurrency groups, and allow-unsafe-pr-checkout for PR checkouts. build-main fetches a secret file URL and runs scripts/build.sh. Added comments explaining rationale: reduce risk from contributor-authored build scripts and protect repository secrets by scoping permissions and requiring maintainer approval for PR runs.
@hirthbrian hirthbrian closed this Aug 7, 2026
@hirthbrian hirthbrian reopened this Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant