Skip to content

ci: build exact PR head commit, not the PR/base merge#192

Merged
pk910 merged 1 commit into
masterfrom
pk910/fix-build-workflow
Jun 11, 2026
Merged

ci: build exact PR head commit, not the PR/base merge#192
pk910 merged 1 commit into
masterfrom
pk910/fix-build-workflow

Conversation

@pk910

@pk910 pk910 commented Jun 11, 2026

Copy link
Copy Markdown
Member

Problem

build-dev.yml triggers on pull_request but stopped pinning the checkout ref after the [codex] fix security alerts hardening. On a pull_request event, actions/checkout with no ref checks out refs/pull/<N>/merge — the PR branch merged with the base branch — so PR builds were producing branch-merged-with-base instead of the exact pushed commit. For branches that have diverged from the base this ships wrong/broken artifacts (it caused a production incident on a devnet build).

Fix (additive — nothing removed, hardening preserved)

  • _shared-build.yaml: add an optional ref input and pin every actions/checkout to ${{ inputs.ref }}. Empty default keeps push-triggered callers (build-master/build-main and build-release) working unchanged.
  • build-dev.yml: compute commit_ref from github.event.pull_request.head.sha (read via env to avoid script injection) and pass it to both build jobs.

The repo's existing branch_name → docker_tag slash normalization is untouched — commit_ref is a separate raw git SHA that must not be normalized.

Why it's safe

This workflow runs on plain pull_request (not pull_request_target), so fork PRs get a read-only token and no secrets — building untrusted head code is harmless. Docker publishing (which needs secrets) is gated to same-repo PRs. A DO NOT REMOVE banner on the ref input documents this so a future automated security pass doesn't strip it again.

Regression introduced by

The checkout ref pinning was removed by #183 ([codex] fix security alerts). This PR restores it without reverting the security hardening from that PR.

The dev build workflow lost its checkout ref pinning when the security
hardening moved it from pull_request_target to pull_request. On a
pull_request event, actions/checkout with no ref defaults to
refs/pull/<N>/merge -- the PR branch merged with the base branch -- so
PR builds shipped branch+base code instead of the exact pushed commit.
For branches diverged from the base this produces wrong/broken
artifacts (it caused a production incident on a devnet build).

Restore the ref pinning while keeping the hardening:

- _shared-build.yaml: add an optional `ref` input and pin every
  actions/checkout to it. The empty default makes checkout fall back to
  the event default, so push-triggered callers (build-master/-main and
  build-release) that don't pass a ref keep working unchanged.
- build-dev.yml: compute commit_ref from the PR head sha (read via env
  to avoid script injection) and pass it to both build jobs.

Safe under pull_request: fork PRs run with a read-only token and no
secrets, and docker publishing is gated to same-repo PRs -- so pinning
the head sha reintroduces no risk. A DO-NOT-REMOVE banner on the `ref`
input documents this so a future automated security pass won't strip it.
@pk910 pk910 merged commit 2231b3e into master Jun 11, 2026
6 checks passed
@pk910 pk910 deleted the pk910/fix-build-workflow branch June 11, 2026 19:44
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.

2 participants