[release-1.10] ci: simplify /build-images to always build PR HEAD

[rm3l]

Manual cherry-pick of #3076

[rhdh-qodo-merge]

PR Summary by Qodo

CI: make /build-images always build the current PR HEAD
⚙️ Configuration changes 📝 Documentation 🕐 10-20 Minutes

Description

• Simplify the /build-images slash command to build the PR branch HEAD by default.
• Remove the requirement to specify an approved commit SHA in the trigger comment.
• Update the PR template instructions to match the new slash-command behavior.

Diagram

graph TD
  M(("Maintainer")) --> IC["Comment: /build-images"] --> WF["GH Actions: PR image build"] --> API{{"GitHub API: pulls.get"}} --> SHA["Use PR HEAD SHA"] --> CO["Checkout @ SHA"] --> BUILD["Build & push images"] --> QUAY[("Quay registry")] --> PRC["Comment image links"]

  subgraph Legend
    direction LR
    _actor(("Actor")) ~~~ _wf["Workflow"] ~~~ _api{{"External API"}} ~~~ _db[("Registry")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep explicit SHA approval (`/build-images `)

• ➕ Maintainer explicitly approves the exact commit that will be built (stronger safety against post-review pushes/force-pushes).
• ➕ Makes it harder to accidentally build unreviewed code if the PR changes after approval.
• ➖ More friction for maintainers/contributors; requires copying SHAs.
• ➖ Higher chance of user error (wrong/old SHA).

2. Build HEAD but enforce 'no-new-commits-since-comment' check

• ➕ Keeps the simple /build-images UX while reducing the risk of building newer, unreviewed commits.
• ➕ Automatable by comparing comment timestamp to head commit metadata.
• ➖ More complex logic (additional API calls, edge cases with rebases/force-pushes).
• ➖ Still weaker than explicit SHA approval in some scenarios.

3. Require `/build-images` to match PR HEAD at comment time (bot echoes SHA)

• ➕ Maintainer uses simple command; workflow/bot resolves and records the exact SHA being built for traceability.
• ➕ Improves auditability in PR timeline without requiring manual SHA entry.
• ➖ Still requires extra implementation (posting/resolving SHA, handling retries).
• ➖ Does not fully prevent races unless the echoed SHA is then enforced for checkout.

Recommendation: If the primary goal is maintainability and ease of use for test builds, the PR’s approach is the simplest and consistent with the updated docs. However, it materially weakens the previous “explicit commit approval” gate: a PR author could push new commits after a maintainer comments, and the workflow would build the new HEAD. If that risk matters in this repo’s threat model, prefer either keeping /build-images or adding a lightweight guard that fails when the head commit is newer than the trigger comment.

Files changed (2) +4 / -19

Documentation (1) +3 / -6

PULL_REQUEST_TEMPLATE.mdDocument '/build-images' as a HEAD-only trigger +3/-6

Document '/build-images' as a HEAD-only trigger

• Updates maintainer/contributor guidance to use '/build-images' without a commit SHA. Clarifies that the command always builds the HEAD of the PR branch.

.github/PULL_REQUEST_TEMPLATE.md

Other (1) +1 / -13

pr-container-build.yamlRemove SHA parsing; always build PR head SHA +1/-13

Remove SHA parsing; always build PR head SHA

• Deletes the comment-body parsing and failure path that enforced a user-supplied SHA. Sets the build ref to 'pr.data.head.sha' so the workflow always checks out and builds the PR’s current HEAD.

.github/workflows/pr-container-build.yaml

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 18 rules

1. TOCTOU builds unreviewed commit 🐞 Bug ⛨ Security

Description

pr-container-build now resolves the build ref from pr.data.head.sha, so if the PR branch is
updated (push/force-push) after a maintainer posts /build-images but before the workflow runs, CI
can build and execute a different commit than the maintainer intended. Since the job logs into Quay
with secrets.QUAY_TOKEN and then runs make release-build from the checked-out PR, that race can
make secret-bearing registry credentials accessible to code the maintainer never reviewed.

Code

.github/workflows/pr-container-build.yaml[52]

+            core.setOutput('sha', pr.data.head.sha);

Relevance

⭐ Low

PR #3076 merged explicitly using pr.data.head.sha to always build PR HEAD; indicates team accepts
this behavior.

PR-#3076
PR-#2293

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The workflow now sets the build SHA directly from the PR’s current head.sha and uses it for
checkout, which can change if the branch is updated after the maintainer’s comment. The job then
logs into Quay with QUAY_TOKEN and runs make release-build from the PR workspace after that
login, so any unexpected change in the checked-out commit directly affects what code runs with
access to registry credentials.

.github/workflows/pr-container-build.yaml[41-55]
.github/workflows/pr-container-build.yaml[56-63]
.github/workflows/pr-container-build.yaml[80-108]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The slash-command workflow now always builds the mutable PR HEAD SHA (`pr.data.head.sha`). If the contributor updates the PR branch after a maintainer triggers `/build-images` (but before the job starts / before the PR details step runs), the workflow may build a different commit than what the maintainer implicitly authorized.

Because the workflow logs into Quay using `secrets.QUAY_TOKEN` and then executes `make release-build` from the checked-out PR, this creates a TOCTOU window where unreviewed PR code can run after a secret-bearing registry login has been performed.

## Issue Context
This job is triggered by `issue_comment` and is intended to build/push test images. Even if the product requirement is “always build HEAD”, the workflow should ensure PR-controlled code does **not** run after any secret-bearing step.

## Fix Focus Areas
- Move registry login after build (split build vs push):
 - .github/workflows/pr-container-build.yaml[80-114]
- Ensure no `make`/repo scripts run after login:
 - .github/workflows/pr-container-build.yaml[87-114]
- (Optional hardening) tighten the command match to avoid accidental triggers (e.g. exact `/build-images`):
 - .github/workflows/pr-container-build.yaml[21-25]

## Suggested implementation approach
1. Split the current “Build and push images” step into:
  - a **build step** that installs dependencies and runs `make release-build` **before** any registry login.
  - a **login step** (`docker/login-action`) immediately before pushing.
  - a **push step** that only runs `podman push` / `skopeo copy` and does not execute any PR-provided scripts.
2. Keep `/build-images` behavior (always HEAD) if desired, but prevent PR code from running after secrets are available.
3. Optionally change the job `if:` condition from `startsWith(..., '/build-images')` to a stricter match to avoid unintended builds now that the SHA argument is no longer required.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---