Skip to content

[release-1.9] ci: simplify /build-images to always build PR HEAD - #3090

Merged
rm3l merged 1 commit into
redhat-developer:release-1.9from
rm3l:cherry-pick/release-1.9/simplify_build-images_slash_command
Jun 25, 2026
Merged

[release-1.9] ci: simplify /build-images to always build PR HEAD#3090
rm3l merged 1 commit into
redhat-developer:release-1.9from
rm3l:cherry-pick/release-1.9/simplify_build-images_slash_command

Conversation

@rm3l

@rm3l rm3l commented Jun 25, 2026

Copy link
Copy Markdown
Member

Manual cherry-pick of #3076

…r#3076)

Co-authored-by: Gennady Azarenkov <gazarenkov@gmail.com>
Assisted-by: Claude
@rm3l
rm3l merged commit 4e16a64 into redhat-developer:release-1.9 Jun 25, 2026
5 of 6 checks passed
@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

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

Grey Divider

Description

• Simplify /build-images trigger to no longer require a commit SHA.
• Update the build workflow to always use the PR’s current head SHA.
• Refresh PR template guidance for maintainers and contributors.
Diagram

graph TD
  A["Maintainer"] --> B["PR comment: /build-images"] --> C["GitHub Actions workflow"] --> D["Fetch PR metadata"] --> E["Use head.sha output"] --> F["Build container images"] --> G[("Quay registry")]
  C --> H["Post image links"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep explicit SHA parameter
  • ➕ Build is pinned to an explicitly authorized commit
  • ➕ Avoids “moving target” builds if new commits land after approval
  • ➖ More friction for maintainers and contributors
  • ➖ More error-prone (wrong/short SHA, formatting issues)
2. Build HEAD but require approval gating
  • ➕ Keeps the simplified UX
  • ➕ Ensures builds only happen after an explicit approval (e.g., label/approval check)
  • ➖ More workflow complexity than this PR
  • ➖ Requires maintaining additional policy logic (labels, reviews, or checks)

Recommendation: The PR’s approach is a pragmatic simplification: it removes fragile comment parsing and standardizes on the PR head SHA, matching what maintainers typically want to test. The main tradeoff is losing an explicit commit pin; if “pinned builds” are a security/compliance requirement, consider the approval-gated HEAD alternative instead.

Files changed (2) +4 / -19

Documentation (1) +3 / -6
PULL_REQUEST_TEMPLATE.mdUpdate /build-images instructions to remove SHA argument +3/-6

Update /build-images instructions to remove SHA argument

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

.github/PULL_REQUEST_TEMPLATE.md

Other (1) +1 / -13
pr-container-build.yamlUse PR head SHA directly instead of parsing comment-provided SHA +1/-13

Use PR head SHA directly instead of parsing comment-provided SHA

• Removes the security gate and regex parsing that required a SHA argument in the comment. Sets the workflow 'sha' output to 'pr.data.head.sha', ensuring builds always use the PR’s current HEAD commit.

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

@rm3l
rm3l deleted the cherry-pick/release-1.9/simplify_build-images_slash_command branch June 25, 2026 10:33
@sonarqubecloud

Copy link
Copy Markdown

@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 18 rules

Grey Divider


Informational

1. Unpinned build authorization 🐞 Bug ⛨ Security
Description
The workflow now builds whatever PR HEAD is at runtime (pr.data.head.sha) rather than a
maintainer-approved commit SHA, reintroducing a time-of-check/time-of-use gap where new commits can
land after review/comment but before the build resolves the SHA. Since the job logs into the
registry and then runs PR-controlled build steps, an unreviewed HEAD commit can execute with access
to registry credentials and publish arbitrary images.
Code

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

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

⭐ Low

Team intentionally switched to building PR HEAD; PR #3076 merged this simplification despite prior
TOCTOU discussions.

PR-#3076
PR-#2293

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow now sets the build target SHA from the PR’s current head (pr.data.head.sha) and
checks out that commit; later, it logs into the registry with secrets and runs make release-build,
so building an unreviewed head commit meaningfully increases risk.

.github/workflows/pr-container-build.yaml[41-64]
.github/workflows/pr-container-build.yaml[80-108]
.github/PULL_REQUEST_TEMPLATE.md[24-33]

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 `/build-images` workflow no longer pins the build to an explicitly authorized commit SHA and instead uses the current PR head SHA at runtime. This weakens the intended security gate for a workflow that authenticates to the registry and executes PR-controlled build logic.

### Issue Context
The workflow runs on `issue_comment` and is gated by commenter association, but it still executes untrusted PR code (checkout + `make release-build`) after logging into the registry. Without SHA pinning, the code that gets built can change between the maintainer’s review intent and the actual build.

### Fix Focus Areas
- .github/workflows/pr-container-build.yaml[41-64]
- .github/workflows/pr-container-build.yaml[80-108]

### Suggested change
- Reintroduce explicit SHA authorization by parsing `/build-images <sha>` (as previously done) and failing the workflow if no SHA is provided.
- Keep using `actions/checkout` with `ref: <authorized_sha>` to ensure the build is pinned to the reviewed commit.
- Update the PR template/docs accordingly if reverting behavior.

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


2. Ambiguous command triggers builds 🐞 Bug ☼ Reliability
Description
With SHA validation removed, any comment that merely starts with /build-images will proceed to
build the PR head, including typos or extra suffixes like /build-images-foo or `/build-images
please`. This increases the chance of unintended builds and makes the documented command behavior
(“comment /build-images”) less precise than what the workflow accepts.
Code

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

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

⭐ Low

Merged workflow already used startsWith('/build-images') trigger without strict parsing; no history
of enforcing exact command matching.

PR-#2293
PR-#3076

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The job-level condition triggers on a simple startsWith check, and after the change there is no
longer any script-level parsing/validation step; the PR template documents the exact /build-images
command.

.github/workflows/pr-container-build.yaml[21-55]
.github/PULL_REQUEST_TEMPLATE.md[24-33]

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 workflow accepts any comment starting with `/build-images` and, after this PR, no longer validates/normalizes the command format. This can trigger builds unintentionally when users type similar prefixes or add extra text.

### Issue Context
The job is gated by `startsWith(github.event.comment.body, '/build-images')` and the script no longer checks the comment body format before proceeding.

### Fix Focus Areas
- .github/workflows/pr-container-build.yaml[21-55]

### Suggested change
- Tighten command validation by requiring an exact match after trimming (e.g., allow `/build-images` with optional trailing whitespace only), and fail fast with a clear message if additional unexpected characters/arguments are present.
- Align the workflow acceptance rules with the PR template documentation.

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


Grey Divider

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant