Split build-and-push workflow into router + mode-specific reusable workflows#59
Open
CodeByNikolas wants to merge 3 commits intomainfrom
Open
Split build-and-push workflow into router + mode-specific reusable workflows#59CodeByNikolas wants to merge 3 commits intomainfrom
CodeByNikolas wants to merge 3 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds two new reusable GitHub Actions workflows (GitHub runners and self-hosted BuildKit) for building and pushing multi-architecture Docker images, refactors the main build workflow to delegate to them based on execution mode, and updates Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller Workflow
participant MainWF as build-and-push-docker-image.yml
participant GHR as build-and-push-docker-image-github-runners.yml
participant SH as build-and-push-docker-image-self-hosted-buildkit.yml
participant Registry as Docker Registry
participant BuildKit as Remote BuildKit
Caller->>MainWF: workflow_call(inputs, secrets)
MainWF->>MainWF: evaluate inputs.execution-mode
alt github-runners
MainWF->>GHR: workflow_call(inputs, secrets)
GHR->>GHR: setup-matrix (tags, cache, matrix)
GHR->>GHR: build (per-platform jobs)
alt multi-arch
GHR->>GHR: upload digests
GHR->>Registry: assemble & push manifest list
else single-arch
GHR->>Registry: push image
end
GHR-->>MainWF: outputs (image_tag, base_tag, sha_tag, cache_tag, image_repo)
else self-hosted-buildkit
MainWF->>SH: workflow_call(inputs, secrets)
SH->>SH: setup-matrix (tags, cache, matrix)
SH->>BuildKit: configure remote Buildx driver
SH->>SH: build (per-platform, export digests)
alt multi-arch
SH->>SH: upload digests
SH->>Registry: assemble & push manifest list
else single-arch
SH->>Registry: push image
end
SH-->>MainWF: outputs (image_tag, base_tag, sha_tag, cache_tag, image_repo)
end
MainWF-->>Caller: final outputs (image_tag, base_tag, sha_tag, cache_tag, image_repo)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml (3)
261-263: Hardcoded internal IP address for registry mirror default.The default mirror
131.159.88.117:30081is hardcoded. Consider:
- Using a DNS name instead of an IP for resilience
- Documenting this default in the workflow description
- Making it configurable via repository/organization variables if this is org-wide infrastructure
This isn't blocking if intentional for your infrastructure, but hardcoded IPs are fragile if infrastructure changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml around lines 261 - 263, The workflow currently hardcodes the registry mirror IP by assigning CANDIDATE="131.159.88.117:30081"; change this to use a configurable/default DNS-based value: read CANDIDATE from a workflow/environment input (or repository/organization secret/variable) and fall back to a DNS name (not a raw IP) documented in the workflow header/description; update the fallback logic in the same conditional that sets CANDIDATE and add a short comment in the workflow describing the default and how to override it.
457-458: Inconsistent artifact action versions (same as GitHub runners workflow).Uses
actions/download-artifact@v6butactions/upload-artifact@v7at line 436. Align versions for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml around lines 457 - 458, The workflow uses mismatched artifact action versions—`actions/upload-artifact@v7` and `actions/download-artifact@v6`—so update the `actions/download-artifact` invocation to `actions/download-artifact@v7` (or otherwise ensure both upload and download use the same version) to keep the artifact steps consistent; modify the line that currently references `actions/download-artifact@v6` to use `@v7`.
283-311: Consider extracting duplicated tag configuration logic.The tag configuration block (lines 283-311) is duplicated in the merge job (lines 474-502), and this same pattern appears in both workflow files (4 total copies). Consider extracting this to a composite action or a shared script to reduce maintenance burden and ensure consistency.
Also applies to: 474-502
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml around lines 283 - 311, The "Prepare tag configuration" step (id: tag_config) is duplicated across jobs/workflows; extract the logic into a single reusable unit (either a composite GitHub Action or a shell script checked into the repo) and replace the inline block with a call to that unit; specifically, move the bash that builds TAGS_CONFIG (the echo "TAGS_CONFIG<<EOF" ... echo "EOF" >> $GITHUB_ENV sequence and its parsing of inputs.tags and base_tag/sha_tag) into a new action or script (e.g., actions/prepare-tags/action.yml or scripts/prepare-tags.sh), update the workflow steps to call the composite action (uses: ./actions/prepare-tags) or run the script (run: ./scripts/prepare-tags.sh) and ensure it sets the same GITHUB_ENV entries and preserves behavior for lines that handle "type=" entries and comma-separated tags so existing consumers of TAGS_CONFIG remain unchanged..github/workflows/build-and-push-docker-image-github-runners.yml (1)
131-131: Potential edge case:SAFE_REF_NAMEmay produce invalid Docker tags.If
REF_NAMEstarts with an uppercase letter or special character, the resultingSAFE_REF_NAMEcould begin with a hyphen (e.g.,@feature→-feature), which is invalid for Docker tags. Consider stripping leading/trailing hyphens:SAFE_REF_NAME=$(echo "${REF_NAME}" | tr '[:upper:]' '[:lower:]' | tr -c 'a-z0-9_.-' '-') +SAFE_REF_NAME="${SAFE_REF_NAME#-}" # strip leading hyphen +SAFE_REF_NAME="${SAFE_REF_NAME%-}" # strip trailing hyphen🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-github-runners.yml at line 131, SAFE_REF_NAME can begin with a hyphen after the current normalization, producing invalid Docker tags; update the pipeline that produces SAFE_REF_NAME (the transformation from REF_NAME) to strip any leading and trailing hyphens after the lowercase/non-alnum replacement and validate the result (e.g., fall back to a safe default like "ref" or the commit SHA if it becomes empty), ensuring the final SAFE_REF_NAME does not start or end with '-' and meets Docker tag character/length rules..github/workflows/build-and-push-docker-image.yml (1)
129-131: Hardcoded ARC runner names reduce portability.The runner names
arc-buildkit-eduide-amd64andarc-buildkit-eduide-arm64are hardcoded. Consider exposing these as inputs (similar torunner-amd64/runner-arm64in the child workflows) to allow other teams or forks to use different runner sets without modifying this workflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image.yml around lines 129 - 131, Replace the hardcoded runner names by adding workflow inputs for runner-amd64, runner-arm64 and runner-merge and use those input variables where the names are referenced (replace literal "arc-buildkit-eduide-amd64", "arc-buildkit-eduide-arm64" and the merge entry with inputs.runner-amd64, inputs.runner-arm64 and inputs.runner-merge); add sensible defaults matching the current values so existing behavior is unchanged but callers/forks can override the runners when invoking the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-and-push-docker-image-github-runners.yml:
- Around line 415-420: The "Login to GHCR" step currently hardcodes registry:
ghcr.io which breaks non-GHCR registries; update the docker/login-action@v4 step
to use the workflow input (inputs.registry) instead of the literal "ghcr.io" so
the merge job logs into the correct registry; ensure the step still uses
username: ${{ github.repository_owner }} and password: ${{ secrets.GITHUB_TOKEN
}} but replace the registry value with ${{ inputs.registry }} (or the exact
input variable name used by the workflow) so pushes succeed for Docker Hub, ECR,
or other registries.
- Around line 242-260: The workflow currently injects ${{ inputs.tags }}
directly into the shell which allows command substitution and can lead to
command injection; instead set the input into a safe environment variable (e.g.,
TAGS) in the job/step env and reference it inside the script as "$TAGS" (not via
literal interpolation of ${{ inputs.tags }}), then process it with safe
string-handling (e.g., printf '%s\n' "$TAGS" | while IFS= read -r line ...) and
keep existing logic that splits into TAG_ARRAY and writes to GITHUB_ENV; ensure
you never use eval or unquoted variable expansions when handling TAGS, and
update references to inputs.tags in this block to use the new "$TAGS" env
variable while preserving the TAG_ARRAY, line, and tag handling.
In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml:
- Around line 291-309: The workflow currently reads ${{ inputs.tags }} directly
into the shell loop which allows command injection; instead assign the input to
a safe environment variable (e.g., INPUT_TAGS="${{ inputs.tags }}") and use that
variable everywhere in the parsing logic (replace references to ${{ inputs.tags
}} with "$INPUT_TAGS"), using printf '%s' "$INPUT_TAGS" | while IFS= read -r
line || [[ -n "$line" ]]; do and always quote variable expansions (e.g.,
"$line", "$tag") when testing or writing to GITHUB_ENV to prevent word-splitting
and injection; keep the existing logic that detects "type=" and splits on
commas, but operate solely on the quoted INPUT_TAGS variable.
- Around line 464-469: The "Login to GHCR" step hardcodes registry: ghcr.io;
update the docker/login-action@v4 step (the job step named "Login to GHCR") to
use the workflow input variable instead by replacing the hardcoded registry
value with ${{ inputs.registry }} so the merge job supports alternative
registries; keep username and password as-is.
In @.github/workflows/build-and-push-docker-image.yml:
- Around line 79-82: The workflow's execution-mode input handling is incomplete:
the input named execution-mode and the two jobs (self-hosted-buildkit and
github-runners) don't handle the default '' or the 'auto-routing' value, so
builds are skipped; either implement auto-routing logic or require explicit
modes. Fix by updating the conditional logic for the jobs or the input: either
(A) implement auto-routing inside the workflow by detecting runner availability
and routing execution-mode == 'auto-routing' or execution-mode == '' to the
appropriate job (modify the job if: conditions for self-hosted-buildkit and
github-runners to accept execution-mode == 'auto-routing' / '' and add steps to
probe for self-hosted runner availability before choosing), or (B) change the
execution-mode input to required (remove default '') and update the description
to remove 'auto-routing' so callers must provide 'self-hosted-buildkit' or
'github-runners'; apply the same change consistently where execution-mode is
referenced (the self-hosted-buildkit and github-runners job conditionals).
---
Nitpick comments:
In @.github/workflows/build-and-push-docker-image-github-runners.yml:
- Line 131: SAFE_REF_NAME can begin with a hyphen after the current
normalization, producing invalid Docker tags; update the pipeline that produces
SAFE_REF_NAME (the transformation from REF_NAME) to strip any leading and
trailing hyphens after the lowercase/non-alnum replacement and validate the
result (e.g., fall back to a safe default like "ref" or the commit SHA if it
becomes empty), ensuring the final SAFE_REF_NAME does not start or end with '-'
and meets Docker tag character/length rules.
In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml:
- Around line 261-263: The workflow currently hardcodes the registry mirror IP
by assigning CANDIDATE="131.159.88.117:30081"; change this to use a
configurable/default DNS-based value: read CANDIDATE from a workflow/environment
input (or repository/organization secret/variable) and fall back to a DNS name
(not a raw IP) documented in the workflow header/description; update the
fallback logic in the same conditional that sets CANDIDATE and add a short
comment in the workflow describing the default and how to override it.
- Around line 457-458: The workflow uses mismatched artifact action
versions—`actions/upload-artifact@v7` and `actions/download-artifact@v6`—so
update the `actions/download-artifact` invocation to
`actions/download-artifact@v7` (or otherwise ensure both upload and download use
the same version) to keep the artifact steps consistent; modify the line that
currently references `actions/download-artifact@v6` to use `@v7`.
- Around line 283-311: The "Prepare tag configuration" step (id: tag_config) is
duplicated across jobs/workflows; extract the logic into a single reusable unit
(either a composite GitHub Action or a shell script checked into the repo) and
replace the inline block with a call to that unit; specifically, move the bash
that builds TAGS_CONFIG (the echo "TAGS_CONFIG<<EOF" ... echo "EOF" >>
$GITHUB_ENV sequence and its parsing of inputs.tags and base_tag/sha_tag) into a
new action or script (e.g., actions/prepare-tags/action.yml or
scripts/prepare-tags.sh), update the workflow steps to call the composite action
(uses: ./actions/prepare-tags) or run the script (run:
./scripts/prepare-tags.sh) and ensure it sets the same GITHUB_ENV entries and
preserves behavior for lines that handle "type=" entries and comma-separated
tags so existing consumers of TAGS_CONFIG remain unchanged.
In @.github/workflows/build-and-push-docker-image.yml:
- Around line 129-131: Replace the hardcoded runner names by adding workflow
inputs for runner-amd64, runner-arm64 and runner-merge and use those input
variables where the names are referenced (replace literal
"arc-buildkit-eduide-amd64", "arc-buildkit-eduide-arm64" and the merge entry
with inputs.runner-amd64, inputs.runner-arm64 and inputs.runner-merge); add
sensible defaults matching the current values so existing behavior is unchanged
but callers/forks can override the runners when invoking the workflow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9cd2493-99d0-4132-9fda-9cd07dba0da5
📒 Files selected for processing (4)
.github/workflows/build-and-push-docker-image-github-runners.yml.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml.github/workflows/build-and-push-docker-image.yml.gitignore
Comment on lines
+242
to
+260
| if [ -n "${{ inputs.tags }}" ]; then | ||
| echo "${{ inputs.tags }}" | while IFS= read -r line || [[ -n "$line" ]]; do | ||
| line=$(echo "$line" | xargs) | ||
|
|
||
| if [ -n "$line" ]; then | ||
| if [[ "$line" == *"type="* ]]; then | ||
| echo "$line" >> $GITHUB_ENV | ||
| else | ||
| IFS=',' read -ra TAG_ARRAY <<< "$line" | ||
| for tag in "${TAG_ARRAY[@]}"; do | ||
| tag=$(echo "$tag" | xargs) | ||
| if [ -n "$tag" ]; then | ||
| echo "type=raw,value=${tag}" >> $GITHUB_ENV | ||
| fi | ||
| done | ||
| fi | ||
| fi | ||
| done | ||
| fi |
There was a problem hiding this comment.
Potential command injection via inputs.tags interpolation.
The ${{ inputs.tags }} value is directly interpolated into the shell script. If a caller passes malicious content (e.g., containing backticks or $(command)), it could be executed. Consider using an environment variable instead:
🛡️ Safer approach using environment variable
- name: Prepare tag configuration
id: tag_config
+ env:
+ INPUT_TAGS: ${{ inputs.tags }}
run: |
echo "TAGS_CONFIG<<EOF" >> $GITHUB_ENV
echo "type=raw,value=${{ needs.setup-matrix.outputs.base_tag }}" >> $GITHUB_ENV
echo "type=raw,value=${{ needs.setup-matrix.outputs.sha_tag }}" >> $GITHUB_ENV
- if [ -n "${{ inputs.tags }}" ]; then
- echo "${{ inputs.tags }}" | while IFS= read -r line || [[ -n "$line" ]]; do
+ if [ -n "${INPUT_TAGS}" ]; then
+ echo "${INPUT_TAGS}" | while IFS= read -r line || [[ -n "$line" ]]; do🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image-github-runners.yml around
lines 242 - 260, The workflow currently injects ${{ inputs.tags }} directly into
the shell which allows command substitution and can lead to command injection;
instead set the input into a safe environment variable (e.g., TAGS) in the
job/step env and reference it inside the script as "$TAGS" (not via literal
interpolation of ${{ inputs.tags }}), then process it with safe string-handling
(e.g., printf '%s\n' "$TAGS" | while IFS= read -r line ...) and keep existing
logic that splits into TAG_ARRAY and writes to GITHUB_ENV; ensure you never use
eval or unquoted variable expansions when handling TAGS, and update references
to inputs.tags in this block to use the new "$TAGS" env variable while
preserving the TAG_ARRAY, line, and tag handling.
Comment on lines
+415
to
+420
| - name: Login to GHCR | ||
| uses: docker/login-action@v4 | ||
| with: | ||
| registry: ghcr.io | ||
| username: ${{ github.repository_owner }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Hardcoded ghcr.io registry in merge job may break non-GHCR registries.
The merge job logs into ghcr.io explicitly, but inputs.registry could be a different registry (e.g., Docker Hub, ECR). This will cause the manifest push to fail for non-GHCR registries.
🐛 Proposed fix to use inputs.registry
- name: Login to GHCR
uses: docker/login-action@v4
with:
- registry: ghcr.io
- username: ${{ github.repository_owner }}
- password: ${{ secrets.GITHUB_TOKEN }}
+ registry: ${{ inputs.registry }}
+ username: ${{ secrets.registry-user || github.actor }}
+ password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image-github-runners.yml around
lines 415 - 420, The "Login to GHCR" step currently hardcodes registry: ghcr.io
which breaks non-GHCR registries; update the docker/login-action@v4 step to use
the workflow input (inputs.registry) instead of the literal "ghcr.io" so the
merge job logs into the correct registry; ensure the step still uses username:
${{ github.repository_owner }} and password: ${{ secrets.GITHUB_TOKEN }} but
replace the registry value with ${{ inputs.registry }} (or the exact input
variable name used by the workflow) so pushes succeed for Docker Hub, ECR, or
other registries.
Comment on lines
+291
to
+309
| if [ -n "${{ inputs.tags }}" ]; then | ||
| echo "${{ inputs.tags }}" | while IFS= read -r line || [[ -n "$line" ]]; do | ||
| line=$(echo "$line" | xargs) | ||
|
|
||
| if [ -n "$line" ]; then | ||
| if [[ "$line" == *"type="* ]]; then | ||
| echo "$line" >> $GITHUB_ENV | ||
| else | ||
| IFS=',' read -ra TAG_ARRAY <<< "$line" | ||
| for tag in "${TAG_ARRAY[@]}"; do | ||
| tag=$(echo "$tag" | xargs) | ||
| if [ -n "$tag" ]; then | ||
| echo "type=raw,value=${tag}" >> $GITHUB_ENV | ||
| fi | ||
| done | ||
| fi | ||
| fi | ||
| done | ||
| fi |
There was a problem hiding this comment.
Same command injection concern as GitHub runners workflow.
The ${{ inputs.tags }} interpolation has the same injection risk as noted in the GitHub runners workflow. Apply the same fix using an environment variable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml
around lines 291 - 309, The workflow currently reads ${{ inputs.tags }} directly
into the shell loop which allows command injection; instead assign the input to
a safe environment variable (e.g., INPUT_TAGS="${{ inputs.tags }}") and use that
variable everywhere in the parsing logic (replace references to ${{ inputs.tags
}} with "$INPUT_TAGS"), using printf '%s' "$INPUT_TAGS" | while IFS= read -r
line || [[ -n "$line" ]]; do and always quote variable expansions (e.g.,
"$line", "$tag") when testing or writing to GITHUB_ENV to prevent word-splitting
and injection; keep the existing logic that detects "type=" and splits on
commas, but operate solely on the quoted INPUT_TAGS variable.
Comment on lines
+464
to
+469
| - name: Login to GHCR | ||
| uses: docker/login-action@v4 | ||
| with: | ||
| registry: ghcr.io | ||
| username: ${{ github.repository_owner }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Hardcoded ghcr.io in merge job (same as GitHub runners workflow).
The merge job hardcodes ghcr.io but should use ${{ inputs.registry }} to support other registries.
🐛 Proposed fix
- name: Login to GHCR
uses: docker/login-action@v4
with:
- registry: ghcr.io
- username: ${{ github.repository_owner }}
- password: ${{ secrets.GITHUB_TOKEN }}
+ registry: ${{ inputs.registry }}
+ username: ${{ secrets.registry-user || github.actor }}
+ password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Login to GHCR | |
| uses: docker/login-action@v4 | |
| with: | |
| registry: ghcr.io | |
| username: ${{ github.repository_owner }} | |
| password: ${{ secrets.GITHUB_TOKEN }} | |
| - name: Login to GHCR | |
| uses: docker/login-action@v4 | |
| with: | |
| registry: ${{ inputs.registry }} | |
| username: ${{ secrets.registry-user || github.actor }} | |
| password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml
around lines 464 - 469, The "Login to GHCR" step hardcodes registry: ghcr.io;
update the docker/login-action@v4 step (the job step named "Login to GHCR") to
use the workflow input variable instead by replacing the hardcoded registry
value with ${{ inputs.registry }} so the merge job supports alternative
registries; keep username and password as-is.
Comment on lines
+79
to
+82
| execution-mode: | ||
| type: string | ||
| default: '' | ||
| description: "Execution mode input. Only 3 supported operating options overall: auto-routing, self-hosted-buildkit, github-runners. This input accepts self-hosted-buildkit or github-runners." |
There was a problem hiding this comment.
No job handles auto-routing or empty execution-mode (the default).
The description mentions "auto-routing" as a supported option, and the default is '', but neither condition handles these cases:
self-hosted-buildkitjob runs only whenexecution-mode == 'self-hosted-buildkit'github-runnersjob runs only whenexecution-mode == 'github-runners'
If a caller uses the default or specifies auto-routing, both jobs are skipped and no build occurs. Either implement auto-routing logic or update the description and require explicit mode selection.
Also applies to: 110-111, 139-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image.yml around lines 79 - 82, The
workflow's execution-mode input handling is incomplete: the input named
execution-mode and the two jobs (self-hosted-buildkit and github-runners) don't
handle the default '' or the 'auto-routing' value, so builds are skipped; either
implement auto-routing logic or require explicit modes. Fix by updating the
conditional logic for the jobs or the input: either (A) implement auto-routing
inside the workflow by detecting runner availability and routing execution-mode
== 'auto-routing' or execution-mode == '' to the appropriate job (modify the job
if: conditions for self-hosted-buildkit and github-runners to accept
execution-mode == 'auto-routing' / '' and add steps to probe for self-hosted
runner availability before choosing), or (B) change the execution-mode input to
required (remove default '') and update the description to remove 'auto-routing'
so callers must provide 'self-hosted-buildkit' or 'github-runners'; apply the
same change consistently where execution-mode is referenced (the
self-hosted-buildkit and github-runners job conditionals).
CodeByNikolas
force-pushed
the
feature/split-build-workflow-modes
branch
from
4b49c5f to
4c60694
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml (2)
246-253: Sameinputs.build-argsinterpolation pattern as GitHub runners workflow.Consider the same environment variable approach for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml around lines 246 - 253, The Prepare build args step uses inline shell interpolation of ${{ inputs.build-args }} into BUILD_ARGS; switch to reading the value from an environment variable (e.g., export BUILD_ARGS from the workflow env or set via with: env) and then write that env var to GITHUB_OUTPUT to match the GitHub runners pattern—update the step named "Prepare build args" (id: build-args) to reference the BUILD_ARGS environment variable instead of directly interpolating inputs.build-args, and ensure you still echo the variable into GITHUB_OUTPUT (using the same "build-args<<EOF" block).
255-267: Hardcoded IP address for default registry mirror.The default mirror
131.159.88.117:30081is a hardcoded IP address, which is fragile if the infrastructure changes. Consider using a DNS name or documenting this as an internal infrastructure assumption.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml around lines 255 - 267, The workflow step "Determine registry mirror (ARC default)" currently hardcodes CANDIDATE="131.159.88.117:30081" which is brittle; change the default to a DNS hostname or make it configurable via an environment variable/inputs (inputs.registry-mirror or a new input like default-registry-mirror) and update the echo outputs (mirror_hostport and mirror_url) accordingly, or alternatively remove the hardcoded fallback and add documentation in the workflow comment explaining that a registry mirror must be supplied for the mirror step to work. Ensure you update references to CANDIDATE, NORMALIZED, mirror_hostport and mirror_url so they reflect the chosen DNS/configurable default..github/workflows/build-and-push-docker-image-github-runners.yml (1)
218-225: Direct interpolation ofinputs.build-argscould allow injection.While lower risk than user-controlled inputs, directly interpolating
${{ inputs.build-args }}into the shell script is susceptible to command injection if a malicious caller passes crafted content.Consider using an environment variable approach similar to what's recommended for
inputs.tags:🛡️ Safer approach using environment variable
- name: Prepare build args id: build-args + env: + INPUT_BUILD_ARGS: ${{ inputs.build-args }} run: | - BUILD_ARGS="${{ inputs.build-args }}" + BUILD_ARGS="${INPUT_BUILD_ARGS}" echo "build-args<<EOF" >> $GITHUB_OUTPUT echo "$BUILD_ARGS" >> $GITHUB_OUTPUT echo "EOF" >> $GITHUB_OUTPUT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-github-runners.yml around lines 218 - 225, The "Prepare build args" step (id: build-args) currently interpolates ${{ inputs.build-args }} directly into a shell command which can permit command injection; change the step to receive the input via the step's env (set an environment variable like BUILD_ARGS from ${{ inputs.build-args }}) and then reference that safe env var inside the run block (use the existing BUILD_ARGS variable and write it to GITHUB_OUTPUT), ensuring you do not inline the GitHub expression in the shell code; also ensure the value is quoted/treated as plain data when echoing to GITHUB_OUTPUT to avoid word-splitting or command interpretation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-and-push-docker-image-github-runners.yml:
- Around line 204-216: Update the GitHub Actions workflow to use the latest
download-artifact action by replacing uses: actions/download-artifact@v6 with
actions/download-artifact@v8 (or v8.0.1); locate the occurrences of the
download-artifact action in the workflow file (the line containing "uses:
actions/download-artifact@...") and update the version string, then verify
compatibility with steps that consume artifacts such as any step IDs referencing
downloaded artifacts.
---
Nitpick comments:
In @.github/workflows/build-and-push-docker-image-github-runners.yml:
- Around line 218-225: The "Prepare build args" step (id: build-args) currently
interpolates ${{ inputs.build-args }} directly into a shell command which can
permit command injection; change the step to receive the input via the step's
env (set an environment variable like BUILD_ARGS from ${{ inputs.build-args }})
and then reference that safe env var inside the run block (use the existing
BUILD_ARGS variable and write it to GITHUB_OUTPUT), ensuring you do not inline
the GitHub expression in the shell code; also ensure the value is quoted/treated
as plain data when echoing to GITHUB_OUTPUT to avoid word-splitting or command
interpretation.
In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml:
- Around line 246-253: The Prepare build args step uses inline shell
interpolation of ${{ inputs.build-args }} into BUILD_ARGS; switch to reading the
value from an environment variable (e.g., export BUILD_ARGS from the workflow
env or set via with: env) and then write that env var to GITHUB_OUTPUT to match
the GitHub runners pattern—update the step named "Prepare build args" (id:
build-args) to reference the BUILD_ARGS environment variable instead of directly
interpolating inputs.build-args, and ensure you still echo the variable into
GITHUB_OUTPUT (using the same "build-args<<EOF" block).
- Around line 255-267: The workflow step "Determine registry mirror (ARC
default)" currently hardcodes CANDIDATE="131.159.88.117:30081" which is brittle;
change the default to a DNS hostname or make it configurable via an environment
variable/inputs (inputs.registry-mirror or a new input like
default-registry-mirror) and update the echo outputs (mirror_hostport and
mirror_url) accordingly, or alternatively remove the hardcoded fallback and add
documentation in the workflow comment explaining that a registry mirror must be
supplied for the mirror step to work. Ensure you update references to CANDIDATE,
NORMALIZED, mirror_hostport and mirror_url so they reflect the chosen
DNS/configurable default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9f3a6ec-21f2-46d5-a50c-f6679723e4d5
📒 Files selected for processing (4)
.github/workflows/build-and-push-docker-image-github-runners.yml.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml.github/workflows/build-and-push-docker-image.yml.gitignore
✅ Files skipped from review due to trivial changes (1)
- .gitignore
Comment on lines
+204
to
+216
| uses: actions/checkout@v6 | ||
| if: ${{ inputs.ref != '' }} | ||
| with: | ||
| ref: ${{ inputs.ref }} | ||
| - name: Git Checkout (default) | ||
| uses: actions/checkout@v6 | ||
| if: ${{ inputs.ref == '' }} | ||
| with: | ||
| fetch-depth: 1 | ||
|
|
||
| - name: Install Docker Buildx | ||
| id: setup-buildx | ||
| uses: docker/setup-buildx-action@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest version of actions/checkout GitHub Action?
💡 Result:
The latest version of the actions/checkout GitHub Action is v6.0.2, released on January 9, 2026.
Citations:
- 1: https://github.com/actions/checkout/releases
- 2: https://github.com/actions/checkout
- 3: https://github.com/marketplace/actions/checkout
- 4: https://github.com/actions/checkout/blob/main/CHANGELOG.md
- 5: https://github.com/actions/checkout/tags
🌐 Web query:
What is the latest version of docker/setup-buildx-action GitHub Action?
💡 Result:
The latest version of the docker/setup-buildx-action GitHub Action is v4.0.0, released on 2026-03-05.
Citations:
- 1: https://github.com/docker/setup-buildx-action
- 2: https://docs.docker.com/build/ci/github-actions/configure-builder/
- 3: https://github.com/docker/setup-buildx-action/pkgs/container/setup-buildx-action
- 4: https://github.com/marketplace/actions/docker-setup-buildx
- 5: https://newreleases.io/project/github/docker/setup-buildx-action/release/v4.0.0
🌐 Web query:
What is the latest version of docker/build-push-action GitHub Action?
💡 Result:
The latest version of the docker/build-push-action GitHub Action is v7.0.0, released on March 5, 2026.
Citations:
- 1: https://github.com/docker/build-push-action/releases
- 2: https://github.com/docker/build-push-action
- 3: https://github.com/marketplace/actions/build-and-push-docker-images
- 4: https://github.com/docker/build-push-action?search=1
🌐 Web query:
What is the latest version of actions/upload-artifact GitHub Action?
💡 Result:
The latest version of the actions/upload-artifact GitHub Action is v7.0.0.
Citations:
- 1: https://github.com/actions/upload-artifact/releases
- 2: https://github.com/marketplace/actions/upload-a-build-artifact
- 3: https://newreleases.io/project/github/actions/upload-artifact/release/v7.0.0
- 4: actions/upload-artifact@v6...v7
🌐 Web query:
What is the latest version of actions/download-artifact GitHub Action?
💡 Result:
The latest version of the actions/download-artifact GitHub Action is v8.0.1.
Citations:
- 1: https://github.com/marketplace/actions/download-a-build-artifact
- 2: https://newreleases.io/project/github/actions/download-artifact/release/v8.0.1
- 3: https://github.com/actions/download-artifact
Update actions/download-artifact to v8.
The workflow uses outdated actions/download-artifact@v6; the latest stable version is v8.0.1. The other actions are current: actions/checkout@v6, docker/setup-buildx-action@v4, docker/build-push-action@v7, and actions/upload-artifact@v7 are all at their latest versions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image-github-runners.yml around
lines 204 - 216, Update the GitHub Actions workflow to use the latest
download-artifact action by replacing uses: actions/download-artifact@v6 with
actions/download-artifact@v8 (or v8.0.1); locate the occurrences of the
download-artifact action in the workflow file (the line containing "uses:
actions/download-artifact@...") and update the version string, then verify
compatibility with steps that consume artifacts such as any step IDs referencing
downloaded artifacts.
4dbe402 to
209878f
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
.github/workflows/build-and-push-docker-image-github-runners.yml (1)
433-451:⚠️ Potential issue | 🟠 MajorDuplicate command injection risk via
inputs.tagsin merge job.The same
inputs.tagsinjection vulnerability exists here in the merge job. Apply the same fix as recommended for the build job (lines 242-260) using environment variables.🛡️ Safer approach using environment variable
- name: Prepare tag configuration id: tag_config + env: + INPUT_TAGS: ${{ inputs.tags }} run: | echo "TAGS_CONFIG<<EOF" >> $GITHUB_ENV echo "type=raw,value=${{ needs.setup-matrix.outputs.base_tag }}" >> $GITHUB_ENV echo "type=raw,value=${{ needs.setup-matrix.outputs.sha_tag }}" >> $GITHUB_ENV - if [ -n "${{ inputs.tags }}" ]; then - echo "${{ inputs.tags }}" | while IFS= read -r line || [[ -n "$line" ]]; do + if [ -n "${INPUT_TAGS}" ]; then + printf '%s\n' "${INPUT_TAGS}" | while IFS= read -r line || [[ -n "$line" ]]; do🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-github-runners.yml around lines 433 - 451, The same unsafe echo of ${{ inputs.tags }} should be replaced by first storing the input in a quoted environment variable and iterating that safely (avoid piping the raw workflow expression into the shell). For example, set TAGS_INPUT="${{ inputs.tags }}" (export or assign to a local var), then use a safe producer like printf '%s\n' "$TAGS_INPUT" | while IFS= read -r line || [[ -n "$line" ]]; do ...; keep all expansions quoted (e.g. tag="$(echo "$tag" | xargs)") and continue writing to $GITHUB_ENV as before; ensure you do not use eval or unquoted variable expansions for TAG_ARRAY, tag or line and use IFS=',' read -ra TAG_ARRAY <<< "$line" exactly as shown but with the input coming from the quoted TAGS_INPUT variable.
🧹 Nitpick comments (2)
README.md (1)
99-99: Make the setup command idempotent for reruns.Using plain
mkdirwill fail if/opt/githubalready exists. Prefermkdir -pso operators can safely re-run the bootstrap step.Suggested doc tweak
-5. Create the deployment directory `/opt/github` and give `github_deployment` access: `sudo mkdir /opt/github && sudo chown github_deployment:github_deployment /opt/github` +5. Create the deployment directory `/opt/github` and give `github_deployment` access: `sudo mkdir -p /opt/github && sudo chown github_deployment:github_deployment /opt/github`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 99, The README's setup command uses plain mkdir which fails if /opt/github already exists; update the command shown (the line containing "sudo mkdir /opt/github && sudo chown github_deployment:github_deployment /opt/github") to use mkdir -p so the step is idempotent (e.g., "sudo mkdir -p /opt/github" followed by the existing chown to set github_deployment:github_deployment ownership)..github/workflows/build-and-push-docker-image-github-runners.yml (1)
168-174: Consider sanitizing runner inputs before embedding in JSON.The runner input values are directly embedded into JSON strings. While the delegator passes safe hardcoded values, malicious or malformed runner names containing quotes could break the JSON structure.
🛡️ Defensive approach using jq for JSON construction
+ # Safely construct JSON using jq + MATRIX=$(jq -n --arg amd64_runner "${{ inputs.runner-amd64 }}" \ + --arg arm64_runner "${{ inputs.runner-arm64 }}" \ + --argjson build_amd64 "${{ inputs.build-amd64 }}" \ + --argjson build_arm64 "${{ inputs.build-arm64 }}" ' + {include: ( + (if $build_amd64 then [{platform: "linux/amd64", runner: $amd64_runner}] else [] end) + + (if $build_arm64 then [{platform: "linux/arm64", runner: $arm64_runner}] else [] end) + )}') + NEEDS_MERGE=$(echo "$MATRIX" | jq -r '.include | length > 1') - MATRIX='{"include":[' - PLATFORMS=() - if [[ "${{ inputs.build-amd64 }}" == "true" ]]; then - PLATFORMS+=('{"platform":"linux/amd64","runner":"${{ inputs.runner-amd64 }}"}') - fi - if [[ "${{ inputs.build-arm64 }}" == "true" ]]; then - PLATFORMS+=('{"platform":"linux/arm64","runner":"${{ inputs.runner-arm64 }}"}') - fi - JOINED=$(IFS=,; echo "${PLATFORMS[*]}") - MATRIX="${MATRIX}${JOINED}]}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-github-runners.yml around lines 168 - 174, Replace the direct string interpolation that embeds runner inputs into JSON (the PLATFORMS+= lines that reference inputs.build-amd64/runner-amd64 and inputs.build-arm64/runner-arm64) with a safe JSON constructor: build each platform object using a JSON tool (e.g., jq) or a proper toJSON method so the runner value is passed as a bound string argument rather than injected raw text; append the resulting JSON object to PLATFORMS instead of concatenating quoted strings to avoid breaking JSON if runner contains quotes or special characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-and-push-docker-image-github-runners.yml:
- Around line 218-225: The workflow step with id "build-args" currently
interpolates `${{ inputs.build-args }}` directly into a run shell block which
can lead to command injection; change it to pass the input via an environment
variable (e.g., set env: BUILD_ARGS: ${{ inputs.build-args }}) and then
reference that env var inside the run script (use the BUILD_ARGS environment
variable when writing to GITHUB_OUTPUT) so the input is not re-evaluated by the
shell; update the "Prepare build args" step to use the env-provided BUILD_ARGS
and write it safely to GITHUB_OUTPUT.
- Around line 465-469: The manifest creation step currently interpolates ${{
inputs.registry }} and ${{ inputs.image-name }} directly into the shell line
used with docker buildx imagetools create, which can lead to command-injection;
change the job to export safe environment variables (e.g., REGISTRY and
IMAGE_NAME) from those inputs and use them in the shell, and replace the unsafe
printf/glob usage with iterating over the digest files (e.g., a for loop over *
producing lines like "${REGISTRY}/${IMAGE_NAME}@sha256:${digest}") and pipe the
result into docker buildx imagetools create together with the jq-generated tags;
locate the step that runs "docker buildx imagetools create" in the manifest
creation step and update it to use REGISTRY/IMAGE_NAME env vars and a loop +
xargs instead of direct interpolation and printf '*'.
- Around line 134-136: The workflow currently interpolates `${{ inputs.image-tag
}}` directly into the shell script (OVERRIDE="${{ inputs.image-tag }}"), which
risks command injection; instead set OVERRIDE (and likewise PR_NUMBER and
RELEASE_TAG) via the workflow env/context (e.g., an env: block or step.env) so
the action supplies the raw value and the shell reads it as $OVERRIDE without
re-evaluating; change the step to export/read OVERRIDE, PR_NUMBER, RELEASE_TAG
from environment variables and reference them in the script, and optionally add
a simple whitelist/validation for inputs.image-tag before using it.
In @.github/workflows/deploy-docker-compose.yml:
- Line 123: Update the workflow entry that pins the action
"appleboy/ssh-action@v1.2.3": either revert to the latest compatible version
(e.g., v1.2.5) or add an inline comment next to the uses declaration explaining
the precise reason for downgrading (compatibility break, CI failure, behavior of
GITHUB_OUTPUT handling, security/regression found, etc.), and reference the
decision rationale and any linked issue/PR so reviewers know why v1.2.3 was
chosen; ensure the comment is adjacent to the uses: appleboy/ssh-action@v1.2.3
line so it’s discoverable during reviews.
In `@docs/ci-follow-up-notes-eduide-and-cloud.md`:
- Line 5: Update the PR reference in the sentence that currently reads “merged
PR `#58`” inside docs/ci-follow-up-notes-eduide-and-cloud.md to either “merged PR
`#59`” or rephrase to “originally in `#58`, now continued in `#59`” so the document
accurately reflects the current work (search for the literal string “#58” or the
phrase “merged PR `#58`” to locate the line).
- Around line 24-25: Update the ambiguous note "PR `#119` was not found in
`EduIDE/EduIDE` during current lookup." to include an explicit lookup timestamp
and source (e.g., "as of 2026-03-21 UTC via GitHub web UI" or "as of 2026-03-21
UTC via GitHub API") and keep the follow-up action text ("Action needed before
closing: confirm repository + PR URL for `#119`.") so readers can trust when and
how the check was performed; locate and edit the line containing "PR `#119` was
not found in `EduIDE/EduIDE` during current lookup." to replace it with the
time- and source-scoped phrasing.
---
Duplicate comments:
In @.github/workflows/build-and-push-docker-image-github-runners.yml:
- Around line 433-451: The same unsafe echo of ${{ inputs.tags }} should be
replaced by first storing the input in a quoted environment variable and
iterating that safely (avoid piping the raw workflow expression into the shell).
For example, set TAGS_INPUT="${{ inputs.tags }}" (export or assign to a local
var), then use a safe producer like printf '%s\n' "$TAGS_INPUT" | while IFS=
read -r line || [[ -n "$line" ]]; do ...; keep all expansions quoted (e.g.
tag="$(echo "$tag" | xargs)") and continue writing to $GITHUB_ENV as before;
ensure you do not use eval or unquoted variable expansions for TAG_ARRAY, tag or
line and use IFS=',' read -ra TAG_ARRAY <<< "$line" exactly as shown but with
the input coming from the quoted TAGS_INPUT variable.
---
Nitpick comments:
In @.github/workflows/build-and-push-docker-image-github-runners.yml:
- Around line 168-174: Replace the direct string interpolation that embeds
runner inputs into JSON (the PLATFORMS+= lines that reference
inputs.build-amd64/runner-amd64 and inputs.build-arm64/runner-arm64) with a safe
JSON constructor: build each platform object using a JSON tool (e.g., jq) or a
proper toJSON method so the runner value is passed as a bound string argument
rather than injected raw text; append the resulting JSON object to PLATFORMS
instead of concatenating quoted strings to avoid breaking JSON if runner
contains quotes or special characters.
In `@README.md`:
- Line 99: The README's setup command uses plain mkdir which fails if
/opt/github already exists; update the command shown (the line containing "sudo
mkdir /opt/github && sudo chown github_deployment:github_deployment
/opt/github") to use mkdir -p so the step is idempotent (e.g., "sudo mkdir -p
/opt/github" followed by the existing chown to set
github_deployment:github_deployment ownership).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9b729f0-166f-42ff-b309-ba25af23e018
📒 Files selected for processing (9)
.github/workflows/build-and-push-docker-image-github-runners.yml.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml.github/workflows/build-docusaurus-gh-pages.yml.github/workflows/deploy-docker-compose.yml.github/workflows/deploy-docker-traefik.ymlREADME.mddocs/ci-follow-up-notes-eduide-and-cloud.mdrenovate.jsontraefik/docker-compose.yml
💤 Files with no reviewable changes (4)
- renovate.json
- traefik/docker-compose.yml
- .github/workflows/build-docusaurus-gh-pages.yml
- .github/workflows/deploy-docker-traefik.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml
Comment on lines
+134
to
+136
| OVERRIDE="${{ inputs.image-tag }}" | ||
| PR_NUMBER="${{ github.event.pull_request.number || '' }}" | ||
| RELEASE_TAG="${{ github.event.release.tag_name || '' }}" |
There was a problem hiding this comment.
Potential command injection via direct input interpolation.
The ${{ inputs.image-tag }} value is directly interpolated into the shell script. A malicious caller could pass values like $(malicious_command) or backtick-enclosed commands. Use environment variables instead for safer handling.
🛡️ Safer approach using environment variables
- id: setup
shell: bash
+ env:
+ INPUT_IMAGE_TAG: ${{ inputs.image-tag }}
+ PR_NUMBER: ${{ github.event.pull_request.number }}
+ RELEASE_TAG: ${{ github.event.release.tag_name }}
run: |
set -euo pipefail
SHORT_SHA="${GITHUB_SHA::7}"
REF_NAME="${GITHUB_REF_NAME}"
SAFE_REF_NAME=$(echo "${REF_NAME}" | tr '[:upper:]' '[:lower:]' | tr -c 'a-z0-9_.-' '-')
IMAGE_REPO=$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')
- OVERRIDE="${{ inputs.image-tag }}"
- PR_NUMBER="${{ github.event.pull_request.number || '' }}"
- RELEASE_TAG="${{ github.event.release.tag_name || '' }}"
+ OVERRIDE="${INPUT_IMAGE_TAG:-}"
+ PR_NUM="${PR_NUMBER:-}"
+ REL_TAG="${RELEASE_TAG:-}"
if [[ -n "${OVERRIDE}" ]]; then
BASE_TAG="${OVERRIDE}"
elif [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then
- if [[ -z "${PR_NUMBER}" ]]; then
+ if [[ -z "${PR_NUM}" ]]; then
echo "::error::pull_request event detected but PR number is missing"
exit 1
fi
- BASE_TAG="pr-${PR_NUMBER}"
+ BASE_TAG="pr-${PR_NUM}"
elif [[ "${GITHUB_EVENT_NAME}" == "release" ]]; then
- if [[ -n "${RELEASE_TAG}" ]]; then
- BASE_TAG=$(echo "${RELEASE_TAG}" | tr '[:upper:]' '[:lower:]' | tr -c 'a-z0-9_.-' '-')
+ if [[ -n "${REL_TAG}" ]]; then
+ BASE_TAG=$(echo "${REL_TAG}" | tr '[:upper:]' '[:lower:]' | tr -c 'a-z0-9_.-' '-')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image-github-runners.yml around
lines 134 - 136, The workflow currently interpolates `${{ inputs.image-tag }}`
directly into the shell script (OVERRIDE="${{ inputs.image-tag }}"), which risks
command injection; instead set OVERRIDE (and likewise PR_NUMBER and RELEASE_TAG)
via the workflow env/context (e.g., an env: block or step.env) so the action
supplies the raw value and the shell reads it as $OVERRIDE without
re-evaluating; change the step to export/read OVERRIDE, PR_NUMBER, RELEASE_TAG
from environment variables and reference them in the script, and optionally add
a simple whitelist/validation for inputs.image-tag before using it.
Comment on lines
+218
to
+225
| - name: Prepare build args | ||
| id: build-args | ||
| run: | | ||
| BUILD_ARGS="${{ inputs.build-args }}" | ||
|
|
||
| echo "build-args<<EOF" >> $GITHUB_OUTPUT | ||
| echo "$BUILD_ARGS" >> $GITHUB_OUTPUT | ||
| echo "EOF" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Potential command injection via inputs.build-args interpolation.
Similar to the inputs.tags issue, ${{ inputs.build-args }} is directly interpolated into the shell script. Use an environment variable for safer handling.
🛡️ Safer approach using environment variable
- name: Prepare build args
id: build-args
+ env:
+ INPUT_BUILD_ARGS: ${{ inputs.build-args }}
run: |
- BUILD_ARGS="${{ inputs.build-args }}"
+ BUILD_ARGS="${INPUT_BUILD_ARGS}"
echo "build-args<<EOF" >> $GITHUB_OUTPUT
echo "$BUILD_ARGS" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image-github-runners.yml around
lines 218 - 225, The workflow step with id "build-args" currently interpolates
`${{ inputs.build-args }}` directly into a run shell block which can lead to
command injection; change it to pass the input via an environment variable
(e.g., set env: BUILD_ARGS: ${{ inputs.build-args }}) and then reference that
env var inside the run script (use the BUILD_ARGS environment variable when
writing to GITHUB_OUTPUT) so the input is not re-evaluated by the shell; update
the "Prepare build args" step to use the env-provided BUILD_ARGS and write it
safely to GITHUB_OUTPUT.
Comment on lines
+465
to
+469
| - name: Create manifest list and push | ||
| working-directory: ${{ runner.temp }}/digests | ||
| run: | | ||
| docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ | ||
| $(printf '${{ inputs.registry }}/${{ inputs.image-name }}@sha256:%s ' *) |
There was a problem hiding this comment.
Potential command injection in manifest creation step.
The ${{ inputs.registry }} and ${{ inputs.image-name }} values are directly interpolated into the shell command. Use environment variables for safer handling.
🛡️ Safer approach using environment variables
- name: Create manifest list and push
working-directory: ${{ runner.temp }}/digests
+ env:
+ REGISTRY: ${{ inputs.registry }}
+ IMAGE_NAME: ${{ inputs.image-name }}
run: |
docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \
- $(printf '${{ inputs.registry }}/${{ inputs.image-name }}@sha256:%s ' *)
+ $(printf '%s/%s@sha256:%s ' "$REGISTRY" "$IMAGE_NAME" $(ls -1))Note: The printf with glob * also needs adjustment to work with env vars - consider using a loop instead:
for digest in *; do
echo "${REGISTRY}/${IMAGE_NAME}@sha256:${digest}"
done | xargs docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image-github-runners.yml around
lines 465 - 469, The manifest creation step currently interpolates ${{
inputs.registry }} and ${{ inputs.image-name }} directly into the shell line
used with docker buildx imagetools create, which can lead to command-injection;
change the job to export safe environment variables (e.g., REGISTRY and
IMAGE_NAME) from those inputs and use them in the shell, and replace the unsafe
printf/glob usage with iterating over the digest files (e.g., a for loop over *
producing lines like "${REGISTRY}/${IMAGE_NAME}@sha256:${digest}") and pipe the
result into docker buildx imagetools create together with the jq-generated tags;
locate the step that runs "docker buildx imagetools create" in the manifest
creation step and update it to use REGISTRY/IMAGE_NAME env vars and a loop +
xargs instead of direct interpolation and printf '*'.
|
|
||
| - name: SSH to VM and execute docker compose down (if exists) | ||
| uses: appleboy/ssh-action@v1.2.5 | ||
| uses: appleboy/ssh-action@v1.2.3 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read-only verification: inspect what changed between v1.2.3 and v1.2.5
repo="appleboy/ssh-action"
echo "== Recent tags =="
curl -s "https://api.github.com/repos/${repo}/tags?per_page=20" \
| jq -r '.[].name' | head -n 10
echo
echo "== Release metadata for v1.2.3 and v1.2.5 =="
for tag in v1.2.3 v1.2.5; do
echo "---- ${tag} ----"
curl -s "https://api.github.com/repos/${repo}/releases/tags/${tag}" \
| jq -r '.name, .published_at, (.body // "no release notes")'
done
echo
echo "== Commit diff v1.2.3...v1.2.5 =="
curl -s "https://api.github.com/repos/${repo}/compare/v1.2.3...v1.2.5" \
| jq -r '"status=\(.status) total_commits=\(.total_commits)",
(.commits[]? | "- \(.sha[0:7]) \(.commit.message|split("\n")[0])")'Repository: ls1intum/.github
Length of output: 3885
Document the reason for downgrading appleboy/ssh-action from v1.2.5 to v1.2.3.
Downgrading without explicit justification should be avoided. Between v1.2.3 and v1.2.5, changes are primarily documentation updates and a refactoring of GITHUB_OUTPUT handling (#404). If this downgrade is intentional, add a comment explaining the compatibility or operational rationale.
Also applies to: 217-217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deploy-docker-compose.yml at line 123, Update the workflow
entry that pins the action "appleboy/ssh-action@v1.2.3": either revert to the
latest compatible version (e.g., v1.2.5) or add an inline comment next to the
uses declaration explaining the precise reason for downgrading (compatibility
break, CI failure, behavior of GITHUB_OUTPUT handling, security/regression
found, etc.), and reference the decision rationale and any linked issue/PR so
reviewers know why v1.2.3 was chosen; ensure the comment is adjacent to the
uses: appleboy/ssh-action@v1.2.3 line so it’s discoverable during reviews.
|
|
||
| Date: 2026-03-17 | ||
| Owner: @CodeByNikolas | ||
| Context: after merging reusable workflow refactor in `ls1intum/.github` (PR #58) |
There was a problem hiding this comment.
Update the PR reference to avoid historical drift.
This context currently anchors to “merged PR #58,” but this work is being carried via PR #59. Please either reference PR #59 here or phrase it as “originally in #58, now continued in #59” to prevent confusion.
Suggested doc tweak
-Context: after merging reusable workflow refactor in `ls1intum/.github` (PR `#58`)
+Context: follow-up after reusable workflow refactor in `ls1intum/.github` (originally PR `#58`, continued in PR `#59`)🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: The official name of this software platform is spelled with a capital “H”.
Context: ...r merging reusable workflow refactor in ls1intum/.github (PR #58) ## Requested follow-up actio...
(GITHUB)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/ci-follow-up-notes-eduide-and-cloud.md` at line 5, Update the PR
reference in the sentence that currently reads “merged PR `#58`” inside
docs/ci-follow-up-notes-eduide-and-cloud.md to either “merged PR `#59`” or
rephrase to “originally in `#58`, now continued in `#59`” so the document accurately
reflects the current work (search for the literal string “#58” or the phrase
“merged PR `#58`” to locate the line).
Comment on lines
+24
to
+25
| - Note: PR #119 was not found in `EduIDE/EduIDE` during current lookup. | ||
| - Action needed before closing: confirm repository + PR URL for #119. |
There was a problem hiding this comment.
Make the “PR #119 not found” note explicitly time-scoped.
“During current lookup” is ambiguous and may read as a present fact later. Add an explicit lookup date/source in the note so readers can trust the context.
Suggested doc tweak
- - Note: PR `#119` was not found in `EduIDE/EduIDE` during current lookup.
+ - Note: PR `#119` was not found in `EduIDE/EduIDE` during lookup on 2026-03-17 (manual check).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/ci-follow-up-notes-eduide-and-cloud.md` around lines 24 - 25, Update the
ambiguous note "PR `#119` was not found in `EduIDE/EduIDE` during current lookup."
to include an explicit lookup timestamp and source (e.g., "as of 2026-03-21 UTC
via GitHub web UI" or "as of 2026-03-21 UTC via GitHub API") and keep the
follow-up action text ("Action needed before closing: confirm repository + PR
URL for `#119`.") so readers can trust when and how the check was performed;
locate and edit the line containing "PR `#119` was not found in `EduIDE/EduIDE`
during current lookup." to replace it with the time- and source-scoped phrasing.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml (2)
464-469:⚠️ Potential issue | 🟠 MajorUse configurable registry in merge login step.
At Line 467, the merge job still hardcodes
ghcr.io; this remains inconsistent withinputs.registryand breaks non-GHCR targets.🐛 Proposed fix
- name: Login to GHCR uses: docker/login-action@v4 with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.GITHUB_TOKEN }} + registry: ${{ inputs.registry }} + username: ${{ secrets.registry-user || github.actor }} + password: ${{ secrets.registry-password || secrets.GITHUB_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml around lines 464 - 469, The "Login to GHCR" step hardcodes ghcr.io and should use the configurable registry input instead; update the docker/login-action@v4 step (step name "Login to GHCR") to set registry: ${{ inputs.registry }} (leaving username and password as-is) so the merge job honors the inputs.registry value for non-GHCR targets.
291-309:⚠️ Potential issue | 🟠 MajorAvoid direct shell interpolation of
inputs.tagsin both tag parsing blocks.At Line 291 and Line 482,
${{ inputs.tags }}is injected directly into shell execution flow. This is the same unresolved injection surface previously flagged.🔒 Proposed hardening pattern
- name: Prepare tag configuration id: tag_config + env: + INPUT_TAGS: ${{ inputs.tags }} run: | echo "TAGS_CONFIG<<EOF" >> $GITHUB_ENV echo "type=raw,value=${{ needs.setup-matrix.outputs.base_tag }}" >> $GITHUB_ENV echo "type=raw,value=${{ needs.setup-matrix.outputs.sha_tag }}" >> $GITHUB_ENV - if [ -n "${{ inputs.tags }}" ]; then - echo "${{ inputs.tags }}" | while IFS= read -r line || [[ -n "$line" ]]; do + if [[ -n "$INPUT_TAGS" ]]; then + while IFS= read -r line || [[ -n "$line" ]]; do line=$(echo "$line" | xargs) if [ -n "$line" ]; then if [[ "$line" == *"type="* ]]; then echo "$line" >> $GITHUB_ENV else IFS=',' read -ra TAG_ARRAY <<< "$line" for tag in "${TAG_ARRAY[@]}"; do tag=$(echo "$tag" | xargs) if [ -n "$tag" ]; then echo "type=raw,value=${tag}" >> $GITHUB_ENV fi done fi fi - done + done < <(printf '%s' "$INPUT_TAGS") fi echo "EOF" >> $GITHUB_ENVAlso applies to: 482-500
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml around lines 291 - 309, The workflow injects inputs.tags directly into shell commands which risks word-splitting/globbing; fix both parsing blocks (the shown loop and the similar block at lines ~482-500) by first assigning the raw input to a quoted shell variable (e.g., TAGS="${{ inputs.tags }}"), then feed it safely into the line loop with printf '%s\n' "$TAGS" | while IFS= read -r line || [[ -n $line ]]; do ...; ensure all expansions are quoted (e.g., "$line", "$tag", and "$GITHUB_ENV") and consider using printf to write env lines instead of unquoted echo to avoid interpolation issues.
🧹 Nitpick comments (1)
.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml (1)
283-311: Consider deduplicating tag parsing logic between build and merge jobs.The same parser logic is duplicated in two places, which increases drift risk for future fixes (especially security-related ones).
Also applies to: 474-503
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml around lines 283 - 311, The tag-parsing block under the "Prepare tag configuration" step (which builds TAGS_CONFIG into $GITHUB_ENV using inputs.tags and needs.setup-matrix.outputs.base_tag/sha_tag) is duplicated; refactor by extracting the logic into a single reusable component (either a composite GitHub Action or a shell script checked into the repo) and replace both occurrences with calls to that component, ensuring it accepts inputs.tags and emits the same "type=raw,value=..." lines into $GITHUB_ENV (or prints them for capturing). Update both places (the current Prepare tag configuration step and the duplicate at lines ~474-503) to invoke the new shared action/script so parsing logic lives in one location.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml:
- Around line 183-187: The script currently sets NEEDS_MERGE based on PLATFORMS
length but doesn't handle the case where PLATFORMS is empty (both build-amd64
and build-arm64 disabled), allowing the job to succeed with no outputs; add an
explicit failure when PLATFORMS has zero elements by checking if
${`#PLATFORMS`[@]} -eq 0 and printing a clear error (mentioning disabled
build-amd64/build-arm64) and exiting non‑zero (e.g., exit 1) so the workflow
fails fast; update the block that references PLATFORMS and NEEDS_MERGE to
perform this check before setting NEEDS_MERGE.
---
Duplicate comments:
In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml:
- Around line 464-469: The "Login to GHCR" step hardcodes ghcr.io and should use
the configurable registry input instead; update the docker/login-action@v4 step
(step name "Login to GHCR") to set registry: ${{ inputs.registry }} (leaving
username and password as-is) so the merge job honors the inputs.registry value
for non-GHCR targets.
- Around line 291-309: The workflow injects inputs.tags directly into shell
commands which risks word-splitting/globbing; fix both parsing blocks (the shown
loop and the similar block at lines ~482-500) by first assigning the raw input
to a quoted shell variable (e.g., TAGS="${{ inputs.tags }}"), then feed it
safely into the line loop with printf '%s\n' "$TAGS" | while IFS= read -r line
|| [[ -n $line ]]; do ...; ensure all expansions are quoted (e.g., "$line",
"$tag", and "$GITHUB_ENV") and consider using printf to write env lines instead
of unquoted echo to avoid interpolation issues.
---
Nitpick comments:
In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml:
- Around line 283-311: The tag-parsing block under the "Prepare tag
configuration" step (which builds TAGS_CONFIG into $GITHUB_ENV using inputs.tags
and needs.setup-matrix.outputs.base_tag/sha_tag) is duplicated; refactor by
extracting the logic into a single reusable component (either a composite GitHub
Action or a shell script checked into the repo) and replace both occurrences
with calls to that component, ensuring it accepts inputs.tags and emits the same
"type=raw,value=..." lines into $GITHUB_ENV (or prints them for capturing).
Update both places (the current Prepare tag configuration step and the duplicate
at lines ~474-503) to invoke the new shared action/script so parsing logic lives
in one location.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e8e33ed-f039-4797-b55c-38b25c60b0e2
📒 Files selected for processing (2)
.github/workflows/build-and-push-docker-image-github-runners.yml.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-and-push-docker-image-github-runners.yml
Comment on lines
+183
to
+187
| if [[ ${#PLATFORMS[@]} -gt 1 ]]; then | ||
| NEEDS_MERGE="true" | ||
| else | ||
| NEEDS_MERGE="false" | ||
| fi |
There was a problem hiding this comment.
Fail fast when no build platform is enabled.
At Line 183, when both build-amd64 and build-arm64 are false, the workflow skips build/merge and can still pass with empty outputs. This should be an explicit configuration error.
🐛 Proposed fix
- if [[ ${`#PLATFORMS`[@]} -gt 1 ]]; then
+ if [[ ${`#PLATFORMS`[@]} -eq 0 ]]; then
+ echo "::error::At least one platform must be enabled (build-amd64/build-arm64)."
+ exit 1
+ elif [[ ${`#PLATFORMS`[@]} -gt 1 ]]; then
NEEDS_MERGE="true"
else
NEEDS_MERGE="false"
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-push-docker-image-self-hosted-buildkit.yml
around lines 183 - 187, The script currently sets NEEDS_MERGE based on PLATFORMS
length but doesn't handle the case where PLATFORMS is empty (both build-amd64
and build-arm64 disabled), allowing the job to succeed with no outputs; add an
explicit failure when PLATFORMS has zero elements by checking if
${`#PLATFORMS`[@]} -eq 0 and printing a clear error (mentioning disabled
build-amd64/build-arm64) and exiting non‑zero (e.g., exit 1) so the workflow
fails fast; update the block that references PLATFORMS and NEEDS_MERGE to
perform this check before setting NEEDS_MERGE.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Current PR #58 targets a feature base branch; this PR retargets the same branch work to for direct merge readiness.
Summary by CodeRabbit
New Features
Chores