fix: trying to solve the arm build process issue. #365
Conversation
WalkthroughThe pull request involves the deletion of two GitHub Actions workflow files for publishing Docker images targeting ARM architectures, specifically Changes
Possibly related PRs
Security Recommendations
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🧰 Additional context used🪛 actionlint (1.7.4).github/workflows/docker-publish-rootless.yaml54-54: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting (shellcheck) 179-179: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting (shellcheck) 179-179: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting (shellcheck) .github/workflows/docker-publish.yaml54-54: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting (shellcheck) 177-177: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting (shellcheck) 177-177: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting (shellcheck) 🔇 Additional comments (2).github/workflows/docker-publish-rootless.yaml (2)
The matrix configuration correctly handles multiple platforms with appropriate fail-fast settings.
The build configuration looks good but consider these security enhancements:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
🛑 Comments failed to post (2)
.github/workflows/docker-publish.yaml (2)
108-120:
⚠️ Potential issueFix: Incorrect step reference in digest export
The digest export step contains an error in the step reference:
- digest="${{ steps.build.outputs.digest }}" + digest="${{ steps.build-and-push.outputs.digest }}"📝 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.- name: Export digest run: | mkdir -p /tmp/digests digest="${{ steps.build-and-push.outputs.digest }}" touch "/tmp/digests/${digest#sha256:}" - name: Upload digest uses: actions/upload-artifact@v4 with: name: digests-${{ env.PLATFORM_PAIR }} path: /tmp/digests/* if-no-files-found: error retention-days: 1🧰 Tools
🪛 actionlint (1.7.4)
109-109: property "build" is not defined in object type {build-and-push: {conclusion: string; outcome: string; outputs: {digest: string; imageid: string; metadata: string}}; meta: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
178-186:
⚠️ Potential issueSecurity: Fix command injection vulnerability
The manifest creation step has potential command injection vulnerabilities due to unquoted expansions:
- docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ + "$(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *)"📝 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.- name: Create manifest list and push working-directory: /tmp/digests run: | docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ "$(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *)" - name: Inspect image run: | docker buildx imagetools inspect ${{ env.REGISTRY_IMAGE }}:${{ steps.meta.outputs.version }}🧰 Tools
🪛 actionlint (1.7.4)
180-180: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting
(shellcheck)
180-180: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting
(shellcheck)
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
.github/workflows/docker-publish.yaml (1)
37-44: Matrix strategy configuration looks good!The multi-platform build strategy effectively addresses the ARM build process issues. The
fail-fast: falsesetting ensures that a failure in one platform's build won't stop others.For enhanced security, consider adding platform-specific build arguments or environment variables if certain platforms require different security configurations.
🛑 Comments failed to post (3)
.github/workflows/docker-publish.yaml (3)
108-113:
⚠️ Potential issueFix incorrect step reference
The digest export step references
steps.build.outputs.digestbut the correct step ID isbuild-and-push. This will cause the digest export to fail.Apply this fix:
- digest="${{ steps.build.outputs.digest }}" + digest="${{ steps.build-and-push.outputs.digest }}"📝 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.- name: Export digest run: | mkdir -p /tmp/digests digest="${{ steps.build-and-push.outputs.digest }}" touch "/tmp/digests/${digest#sha256:}"🧰 Tools
🪛 actionlint (1.7.4)
109-109: property "build" is not defined in object type {build-and-push: {conclusion: string; outcome: string; outputs: {digest: string; imageid: string; metadata: string}}; meta: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
99-99:
⚠️ Potential issueFix undefined environment variable
The
outputsconfiguration uses${{ env.REGISTRY_IMAGE }}which is not defined in the environment variables section. This will cause the build to fail.Add this to the env section at the top of the file:
env: REGISTRY: ghcr.io IMAGE_NAME: ${{ github.repository }} + REGISTRY_IMAGE: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}📝 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.env: REGISTRY: ghcr.io IMAGE_NAME: ${{ github.repository }} REGISTRY_IMAGE: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
178-182: 🛠️ Refactor suggestion
Secure the manifest creation command
The current implementation has potential command injection vulnerabilities due to unquoted expansions.
Apply these security improvements:
- docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ + "$(printf '%s' "${{ env.REGISTRY_IMAGE }}@sha256:%s" *)"Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.4)
180-180: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting
(shellcheck)
180-180: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting
(shellcheck)
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
.github/workflows/docker-publish.yaml (1)
Line range hint
1-181: Security: Consider adding container scanningWhile the multi-platform build process is well-structured, consider adding container scanning to the workflow:
- Add a security scanning step using tools like Trivy or Snyk to scan for vulnerabilities in the built images
- Configure policy to fail builds on critical vulnerabilities
- Enable automatic dependency updates for the base images
Example implementation:
- name: Run Trivy vulnerability scanner uses: aquasecurity/trivy-action@master with: image: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} format: 'table' exit-code: '1' ignore-unfixed: true vuln-type: 'os,library' severity: 'CRITICAL,HIGH'🧰 Tools
🪛 actionlint (1.7.4)
175-175: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting
(shellcheck)
175-175: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting
(shellcheck)
🛑 Comments failed to post (3)
.github/workflows/docker-publish-rootless.yaml (3)
14-14:
⚠️ Potential issueSecurity: Workflow trigger configuration needs review
The workflow is triggered on changes to any workflow file (
'.github/workflows/**'). This could potentially create workflow loops if workflows modify other workflow files.Consider restricting the path pattern to only the specific workflow files that should trigger this workflow:
- - '.github/workflows/**' + - '.github/workflows/docker-publish-rootless.yaml' + - '.github/workflows/docker-publish.yaml'Also applies to: 25-25
54-58:
⚠️ Potential issueSecurity: Shell script improvements needed
The shell scripts have potential security issues with unquoted variables and string manipulation:
Apply these security improvements:
- name: Prepare run: | - platform=${{ matrix.platform }} - echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV + platform="${{ matrix.platform }}" + echo "PLATFORM_PAIR=${platform//\//-}" >> "$GITHUB_ENV" - name: Export digest run: | - mkdir -p /tmp/digests - digest="${{ steps.build.outputs.digest }}" - touch "/tmp/digests/${digest#sha256:}" + mkdir -p "/tmp/digests" + digest="${{ steps.build-and-push.outputs.digest }}" + digest_file="/tmp/digests/${digest#sha256:}" + touch "$digest_file"Also applies to: 103-108
🧰 Tools
🪛 actionlint (1.7.4)
55-55: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
173-181:
⚠️ Potential issueSecurity: Secure the manifest creation script
The manifest creation script uses unquoted command substitution which could be unsafe:
Apply these security improvements:
- name: Create manifest list and push working-directory: /tmp/digests run: | - docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + tags="$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" + digests="$(printf '%s' "${{ env.REGISTRY_IMAGE }}@sha256:%s" *)" + docker buildx imagetools create "$tags" "$digests"Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.4)
175-175: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting
(shellcheck)
175-175: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting
(shellcheck)
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 5
🛑 Comments failed to post (5)
.github/workflows/ghcr-cleanup.yaml (2)
16-22:
⚠️ Potential issueEnhance image cleanup configuration
The current configuration might lead to unintended deletion of images. Consider:
- Pin the action version to a specific commit hash for security
- Add protection for specific tags/patterns
- Implement a dry-run mode initially
- uses: dataaxiom/ghcr-cleanup-action@v1 + uses: dataaxiom/[email protected]@{specific-commit-hash} with: token: ${{ secrets.GITHUB_TOKEN }} repository: ${{ github.repository }} delete-ghost-images: true delete-untagged: true older-than: 30d + protected-tags: | + latest + stable-* + v*.*.*Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 18-18: wrong indentation: expected 12 but found 10
(indentation)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
10-11: 💡 Codebase verification
⚠️ Add protection rules for critical container imagesThe GHCR cleanup workflow currently lacks safeguards for protecting critical images. The configuration shows:
- Untagged and ghost images are deleted
- Images older than 30 days are removed
- No exclusion patterns or protection rules are defined
To prevent accidental deletion of important images, please implement these security measures:
- Add
exclude-tagsparameter to protect production tags (e.g.,latest,stable,prod-*)- Pin the action to a specific commit hash instead of using
@v1for better security- Consider reducing permissions scope with
packages: deleteinstead of broadwriteaccessExample configuration:
permissions: packages: delete # More restrictive than 'write' steps: - uses: dataaxiom/ghcr-cleanup-action@5c19fb92ecd9c24f1715e8cd0a6d9d4d3c5cca3k # Pin to commit hash with: exclude-tags: "latest,stable,prod-*" # Protect critical tags🔗 Analysis chain
Review workflow permissions
The workflow has write permissions to packages which is necessary for cleanup but should be monitored. Consider implementing additional safeguards like protected tags for critical images.
Let me analyze the cleanup workflow configuration to understand the safeguards in place for protecting critical images.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for protected tags in repository settings gh api repos/${{ github.repository }}/tags --jq '.[].name' | grep -E 'prod|stable|latest'Length of output: 155
Script:
#!/bin/bash # Check for container image tags in the workflow file rg -U "tags:|tag:" .github/workflows/ghcr-cleanup.yaml # Check for any protection patterns or exclusions in the cleanup workflow rg -U "exclude|protect|keep" .github/workflows/ghcr-cleanup.yaml # Look for container image references in the repository rg -l "ghcr.io"Length of output: 196
Script:
#!/bin/bash # Get the full content of the cleanup workflow cat .github/workflows/ghcr-cleanup.yamlLength of output: 520
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 11-11: wrong indentation: expected 8 but found 6
(indentation)
.github/workflows/docker-publish-rootless.yaml (3)
94-97: 🛠️ Refactor suggestion
Secure build outputs configuration
The build configuration needs additional security controls:
- Add explicit cache configuration
- Enable BuildKit security features
context: Dockerfile.rootless outputs: type=image,name=${{ env.REGISTRY_IMAGE }},push-by-digest=true,name-canonical=true,push=true labels: ${{ steps.meta.outputs.labels }} platforms: ${{ matrix.platform }} + cache-from: type=gha + cache-to: type=gha,mode=max + security-opt: seccomp=defaultCommittable suggestion skipped: line range outside the PR's diff.
53-56:
⚠️ Potential issueFix potential command injection vulnerability
The platform variable handling could be vulnerable to command injection. Use proper quoting and validate the input.
- platform=${{ matrix.platform }} - echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV + platform="${{ matrix.platform }}" + # Validate platform format + if [[ ! "$platform" =~ ^linux/(amd64|arm64|arm/v7)$ ]]; then + echo "Invalid platform format" + exit 1 + fi + echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV📝 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.- name: Prepare run: | platform="${{ matrix.platform }}" # Validate platform format if [[ ! "$platform" =~ ^linux/(amd64|arm64|arm/v7)$ ]]; then echo "Invalid platform format" exit 1 fi echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV🧰 Tools
🪛 actionlint (1.7.4)
54-54: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
172-176:
⚠️ Potential issueFix shell command injection vulnerabilities
The manifest creation step has potential command injection vulnerabilities in the shell script.
- docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + # Create array of tags + readarray -t TAGS < <(jq -r '.tags[]' <<< "$DOCKER_METADATA_OUTPUT_JSON") + # Build command array + cmd=(docker buildx imagetools create) + for tag in "${TAGS[@]}"; do + cmd+=(-t "$tag") + done + # Add digests + for digest in *; do + cmd+=("${{ env.REGISTRY_IMAGE }}@sha256:${digest}") + done + "${cmd[@]}"📝 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.- name: Create manifest list and push working-directory: /tmp/digests run: | # Create array of tags readarray -t TAGS < <(jq -r '.tags[]' <<< "$DOCKER_METADATA_OUTPUT_JSON") # Build command array cmd=(docker buildx imagetools create) for tag in "${TAGS[@]}"; do cmd+=(-t "$tag") done # Add digests for digest in *; do cmd+=("${{ env.REGISTRY_IMAGE }}@sha256:${digest}") done "${cmd[@]}"🧰 Tools
🪛 actionlint (1.7.4)
174-174: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting
(shellcheck)
174-174: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting
(shellcheck)
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
🛑 Comments failed to post (2)
.github/workflows/docker-publish.yaml (1)
174-179:
⚠️ Potential issueSecure manifest creation process
The manifest creation uses shell command substitution. Add error handling and input validation:
- docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + if ! TAGS=$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON"); then + echo "Failed to process tags" >&2 + exit 1 + fi + if ! DIGESTS=$(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *); then + echo "Failed to process digests" >&2 + exit 1 + fi + docker buildx imagetools create ${TAGS} ${DIGESTS}📝 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.- name: Create manifest list and push working-directory: /tmp/digests run: | if ! TAGS=$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON"); then echo "Failed to process tags" >&2 exit 1 fi if ! DIGESTS=$(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *); then echo "Failed to process digests" >&2 exit 1 fi docker buildx imagetools create ${TAGS} ${DIGESTS}🧰 Tools
🪛 actionlint (1.7.4)
176-176: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting
(shellcheck)
176-176: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting
(shellcheck)
.github/workflows/docker-publish-rootless.yaml (1)
35-43: 💡 Codebase verification
Documentation needed for critical security feature: rootless container builds
The codebase currently lacks comprehensive documentation about the security benefits of rootless container builds across different platforms. While there are references to rootless images, the security implications and platform-specific considerations are not documented. This is important for:
- Helping users understand the security advantages of running containers without root privileges
- Providing guidance on platform-specific security considerations for amd64, arm64, and arm/v7
- Explaining the security trade-offs and use cases for each supported platform
Suggested locations for documentation:
- Add a dedicated security section in README.md
- Create a new file at docs/container-security.md
- Include inline documentation in the docker-publish-rootless.yaml workflow
🔗 Analysis chain
Document rootless build security benefits
The matrix build strategy for rootless containers is a security enhancement. Consider documenting:
- Security benefits of rootless builds
- Platform-specific security considerations
- Use cases for each platform
Let me check if there's any additional documentation about security or rootless builds in other common documentation locations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if rootless build benefits are documented fd -t f "README.md|docs/" -x rg -i "rootless|security|container security"Length of output: 168
Script:
#!/bin/bash # Check for documentation in various common locations fd -t f "(SECURITY|CONTRIBUTING|DEVELOPMENT|INSTALL|BUILD|DOCKER|Container)" -x rg -i "rootless|security|container security|unprivileged" # Also check yaml files for comments about rootless fd -e yaml -e yml -x rg -i "rootless|security|container security|unprivileged" # Check if there are any markdown files containing documentation fd -e md -x rg -i "rootless|security|container security|unprivileged"Length of output: 2364
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/docker-publish-rootless.yaml (1)
14-14: Consider restricting workflow trigger paths for better securityThe broad pattern
.github/workflows/**will trigger builds for any workflow file changes, which could potentially:
- Lead to unnecessary builds and resource consumption
- Create a potential attack vector through workflow file modifications
Consider restricting the pattern to only the specific workflow files that should trigger this build:
- '.github/workflows/**' + '.github/workflows/docker-publish*.yaml'Also applies to: 25-25
🛑 Comments failed to post (1)
.github/workflows/docker-publish-rootless.yaml (1)
54-56:
⚠️ Potential issueFix shell script security vulnerabilities
Several shell scripts have potential security issues:
- Unquoted variables that could lead to word splitting
- Potential command injection vulnerabilities
Apply these security fixes:
# In Prepare step - platform=${{ matrix.platform }} - echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV + platform="${{ matrix.platform }}" + echo "PLATFORM_PAIR=${platform//\//-}" >> "$GITHUB_ENV" # In Export digest step - digest="${{ steps.build.outputs.digest }}" - touch "/tmp/digests/${digest#sha256:}" + digest="${{ steps.build-and-push.outputs.digest }}" + touch "/tmp/digests/${digest#sha256:}" # In Create manifest list step - docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ + "$(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *)"Also applies to: 108-111, 179-181
🧰 Tools
🪛 actionlint (1.7.4)
54-54: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
.github/workflows/docker-publish-rootless.yaml (2)
37-43: LGTM: Multi-platform build setupThe matrix strategy with QEMU setup looks good for cross-platform builds. Consider adding a security scanning step for each platform's build artifacts.
Consider adding Trivy or Snyk container scanning:
- name: Run Trivy vulnerability scanner uses: aquasecurity/trivy-action@master with: image: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} format: 'table' exit-code: '1' ignore-unfixed: true vuln-type: 'os,library'Also applies to: 72-74
129-182: LGTM: Secure manifest handlingThe merge job implementation looks good with proper permissions and authentication. Consider adding these security enhancements:
- Add image signing using cosign:
- name: Sign the images uses: sigstore/cosign-installer@main with: cosign-release: 'v1.13.1' - run: | cosign sign --key env://COSIGN_PRIVATE_KEY ${TAGS} env: COSIGN_PRIVATE_KEY: ${{ secrets.COSIGN_PRIVATE_KEY }}
- Add SHA-256 digest verification before pushing manifests
- Consider implementing rate limiting for registry operations
🧰 Tools
🪛 actionlint (1.7.4)
179-179: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting
(shellcheck)
179-179: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting
(shellcheck)
🛑 Comments failed to post (2)
.github/workflows/docker-publish-rootless.yaml (2)
14-14:
⚠️ Potential issueSecurity: Restrict workflow file trigger patterns
The pattern
.github/workflows/**could potentially create recursive workflow triggers if a workflow modifies another workflow file. Consider using more specific patterns or excluding workflow files from triggers to prevent potential CI/CD loops.- - '.github/workflows/**' + - '.github/workflows/docker-publish*.yaml'Also applies to: 25-25
54-56:
⚠️ Potential issueSecurity: Fix shell script injection vulnerabilities
Several shell scripts have potential command injection vulnerabilities due to improper quoting:
Apply these fixes:
- platform=${{ matrix.platform }} + platform="${{ matrix.platform }}" - echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV + echo "PLATFORM_PAIR=${platform//\//-}" >> "$GITHUB_ENV" - digest="${{ steps.build.outputs.digest }}" + digest="${{ steps.build-and-push.outputs.digest }}" - touch "/tmp/digests/${digest#sha256:}" + touch "/tmp/digests/${digest#sha256:}" - docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ + "$(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *)"Also applies to: 108-111, 179-181
🧰 Tools
🪛 actionlint (1.7.4)
54-54: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 4
🛑 Comments failed to post (4)
.github/workflows/docker-publish-rootless.yaml (2)
14-14:
⚠️ Potential issueSecurity: Restrict workflow path triggers
The path filter
.github/workflows/**could potentially allow workflow modification attacks. Consider restricting it to specific workflow files that this workflow depends on.- - '.github/workflows/**' + - '.github/workflows/docker-publish-rootless.yaml' + - '.github/workflows/cleanup.yaml'Also applies to: 25-25
54-56:
⚠️ Potential issueSecurity: Fix shell script quoting
The shell scripts contain unquoted variables that could lead to command injection or word splitting issues.
Apply these fixes:
- platform=${{ matrix.platform }} - echo "PLATFORM_PAIR=${platform//\//-}" >> $GITHUB_ENV + platform="${{ matrix.platform }}" + echo "PLATFORM_PAIR=${platform//\//-}" >> "$GITHUB_ENV"- mkdir -p /tmp/digests - digest="${{ steps.build-and-push.outputs.digest }}" - touch "/tmp/digests/${digest#sha256:}" + mkdir -p "/tmp/digests" + digest="${{ steps.build-and-push.outputs.digest }}" + touch "/tmp/digests/${digest#sha256:}"Also applies to: 108-111
🧰 Tools
🪛 actionlint (1.7.4)
54-54: shellcheck reported issue in this script: SC2086:info:2:43: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/docker-publish.yaml (2)
177-179:
⚠️ Potential issueSecurity: Fix command injection vulnerability
The manifest creation step is vulnerable to command injection through unquoted command substitution.
- docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \ - $(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *) + docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ + "$(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *)"📝 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.run: | docker buildx imagetools create "$(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON")" \ "$(printf '${{ env.REGISTRY_IMAGE }}@sha256:%s ' *)"🧰 Tools
🪛 actionlint (1.7.4)
177-177: shellcheck reported issue in this script: SC2046:warning:1:33: Quote this to prevent word splitting
(shellcheck)
177-177: shellcheck reported issue in this script: SC2046:warning:2:3: Quote this to prevent word splitting
(shellcheck)
167-173:
⚠️ Potential issueSecurity: Review tag generation patterns
The semver tag patterns could potentially be manipulated in a PR. Consider:
- Restricting semver tags to release events only
- Adding tag signature verification
tags: | type=ref,event=branch type=ref,event=pr - type=semver,pattern={{version}} - type=semver,pattern={{major}}.{{minor}} - type=semver,pattern={{major}} + type=semver,pattern={{version}},enable=${{ github.event_name == 'release' }} + type=semver,pattern={{major}}.{{minor}},enable=${{ github.event_name == 'release' }} + type=semver,pattern={{major}},enable=${{ github.event_name == 'release' }}📝 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.tags: | type=ref,event=branch type=ref,event=pr type=semver,pattern={{version}},enable=${{ github.event_name == 'release' }} type=semver,pattern={{major}}.{{minor}},enable=${{ github.event_name == 'release' }} type=semver,pattern={{major}},enable=${{ github.event_name == 'release' }} type=schedule,pattern=nightly
What type of PR is this?
What this PR does / why we need it:
This PR attempts to use the matrix and separate build processes for all the different platforms. Which should prevent ARM from breaking builds via timeout, while still letting us validate that docker builds functions properly for PRs quickly.
Details from https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners
Summary by CodeRabbit
New Features
linux/amd64,linux/arm64, andlinux/arm/v7.Improvements
.github/workflowsdirectory.Bug Fixes
Chores