Skip to content

Updated PHP, Swoole, Imagemagick, Add more CI scanning, ...#63

Merged
davidjeddy merged 23 commits intomainfrom
clo-4095-cve-maintenance-01
Mar 13, 2026
Merged

Updated PHP, Swoole, Imagemagick, Add more CI scanning, ...#63
davidjeddy merged 23 commits intomainfrom
clo-4095-cve-maintenance-01

Conversation

@davidjeddy
Copy link
Contributor

@davidjeddy davidjeddy commented Mar 10, 2026

… README.md, and test.yaml. This rebuild will pull imagemagick > 7.1 to address CVEs as found by Trivy.

What does this PR do?

## Version 1.1.0

### Add

* .dockerignore
* .github/workflows/pr-scan.yml to scan all commit pushes for vulnerabilities
* base_image and php_build_date to container labels
* container image build action to publish image using commit sha
* container-structure-test to check PHP version (currently set to 8.5.3)
* container-structure-test to check swoole version (currently set to 6.2.0)
* SECURITY.md to align with appwrite/appwrite

### Change

* .github/*.yml steps updated to latest versions
* Better document use of `docker buildx ...` for local builds
* Better noted and organized the different build processes for PHP extensions
* Date component of PHP extension shared objects directory now a build argument
* Dockerfile base now based on `phpswoole/swoole:php8.5-alpine`
* Dockerfile compile and final stage system packages aligned
* GitHub action for container-structure-test now uses a marketplace action
* GitHub action runners pinned to Ubuntu 24.04
* ImageMagick version bumped to 7.1.2.15 via APK
* PHP version bumped to 8.5
* Refactored multi-arch build process to prevent cross-arch builds requiring long wait times

### Fixes

* README.md usage instructions more detailed

### Miscellaneous

### Removed

* Build tools from final stage of Dockerfile
* GitHub action to Setup QEMU as GitHub now provides native ARM runners

Test Plan

trivy image --format json --pkg-types  os,library --severity  CRITICAL,HIGH --output trivy-image-results.json appwrite/base:latest | tee "scan-$(date +%s).log"

container-structure-test test --config tests.yaml --image appwrite/base:latest | tee "test-$(date +%s).log"

Used the new image to docker-compose up the appwrite/appwrite project and am able to load the web applications landing page.

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

yes

Multi-arch build results

image image image

Summary by CodeRabbit

  • New Features

    • Added Docker build optimization configuration
    • Multi-architecture image building and deployment support
    • Automated container testing and vulnerability scanning
  • Documentation

    • Added security vulnerability disclosure policy
    • Updated build and deployment guidance
    • Added release notes and development roadmap
  • Tests

    • New container structure validation workflow
    • Image efficiency analysis and monitoring
    • PHP and extension version verification
  • Chores

    • Enhanced CI/CD workflows and automation
    • Updated build configuration and dependencies
    • Integrated testing and scanning tools

…DM.md, and test.yaml. This rebuild will pull imagemagick > 7.1 to address CVEs as found by Trivy.
@coderabbitai
Copy link

coderabbitai bot commented Mar 10, 2026

📝 Walkthrough

Walkthrough

This pull request substantially restructures the CI/CD infrastructure and Docker build process. It introduces multi-architecture builds with per-branch manifest handling, enhances vulnerability scanning with Trivy SARIF reports, adds container validation workflows, extends PHP extensions in the base image, and establishes security and documentation policies.

Changes

Cohort / File(s) Summary
CI/CD Workflow Refactoring
.github/workflows/build-and-push.yml, .github/workflows/trivy.yml
Build workflow transitions from release-based to push-based with matrix builds per architecture (amd64, arm64) and separate manifest creation for feature vs main branches; Trivy workflow adds scheduled scans, pull request triggers, broader permissions, SARIF output format, and updated action versions.
New Validation Workflows
.github/workflows/dive.yml, .github/workflows/structure-test.yml, .github/workflows/test.yml (removed)
Introduces Dive image efficiency analysis and container-structure-test workflows; removes legacy pull request test workflow in favor of push-triggered validation.
Docker Build Configuration
Dockerfile, .dockerignore
Renames BASEIMAGE to BASE_IMAGE; adds seven new PHP extensions (brotli, lz4, opentelemetry, protobuf, yaml, zstd); restructures multi-stage builds; updates final stage CMD to tail -f /dev/null; excludes build artifacts and documentation from Docker context.
Testing & Validation Config
tests.yaml, .dive-ci.yml
Narrows ImageMagick version regex; adds PHP version and Swoole version verification tests; introduces Dive CI thresholds for image efficiency (0.90 lowest efficiency, 128MB waste limit).
Documentation & Policies
README.md, CHANGES.md, SECURITY.md, TODO.md
Updates README with build badges, restructured sections, and buildx/Trivy/container-structure-test tooling guidance; adds Version 1.1.0 changelog; introduces comprehensive security policy; includes optimization TODOs.

Sequence Diagram(s)

sequenceDiagram
    actor Developer
    participant GitHub as GitHub Actions
    participant BuildArch as Builder<br/>(per-arch)
    participant Registry as Docker Registry
    participant Manifest as Manifest<br/>Service

    Developer->>GitHub: Push to main/feature
    
    alt Main Branch Push
        GitHub->>BuildArch: Build image (amd64)
        BuildArch->>Registry: Push image:sha-amd64
        GitHub->>BuildArch: Build image (arm64)
        BuildArch->>Registry: Push image:sha-arm64
        GitHub->>Manifest: Create manifest from both
        Manifest->>Registry: Push manifest:release-tag
    else Feature Branch Push
        GitHub->>BuildArch: Build image (amd64)
        BuildArch->>Registry: Push image:sha-amd64
        GitHub->>BuildArch: Build image (arm64)
        BuildArch->>Registry: Push image:sha-arm64
        GitHub->>Manifest: Create manifest from both
        Manifest->>Registry: Push manifest:commit-sha
    end

    GitHub->>GitHub: Parallel: Run Trivy, Dive, Structure Tests
    GitHub->>Registry: Scan for vulnerabilities
    GitHub->>Registry: Analyze image efficiency
    GitHub->>Registry: Validate container structure
Loading
sequenceDiagram
    participant Workflow as Trivy Workflow
    participant Docker as Docker
    participant Trivy as Trivy Scanner
    participant CodeQL as GitHub CodeQL
    participant Storage as SARIF Storage

    Workflow->>Docker: Build image
    Docker-->>Workflow: Image ready
    Workflow->>Trivy: Scan image for vulnerabilities
    Trivy->>Trivy: Generate SARIF report
    Trivy-->>Workflow: trivy-image-results.sarif
    Workflow->>CodeQL: Upload SARIF results
    CodeQL->>Storage: Store security findings
    CodeQL-->>Workflow: Results published
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 The builders leap with joy so bright,
Multi-arch images built just right!
New PHP tools join the quest,
Trivy and Dive put security to test!
With workflows dancing, feature to main,
The base container grows more sane!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title mentions updating PHP, Swoole, and ImageMagick, and adding CI scanning, but does not clearly convey the primary change and ends with '...' which reduces clarity and specificity. Revise the title to be more specific and comprehensive, for example: 'Update PHP 8.5.3, Swoole 6.2.0, ImageMagick 7.1.2 and enhance CI/CD workflows' or provide a single clear focus area without ellipsis.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch clo-4095-cve-maintenance-01
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-advanced-security
Copy link

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@davidjeddy davidjeddy force-pushed the clo-4095-cve-maintenance-01 branch from d15cdbf to 955d067 Compare March 10, 2026 15:35
@coderabbitai
Copy link

coderabbitai bot commented Mar 11, 2026

Note

Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it.


Generating unit tests... This may take up to 20 minutes.

@coderabbitai
Copy link

coderabbitai bot commented Mar 11, 2026

✅ Created PR with unit tests: #64

@davidjeddy davidjeddy force-pushed the clo-4095-cve-maintenance-01 branch from ad41bf1 to 6ac7505 Compare March 11, 2026 08:45
@davidjeddy davidjeddy force-pushed the clo-4095-cve-maintenance-01 branch from 6ac7505 to fa5df86 Compare March 11, 2026 08:47
@davidjeddy davidjeddy self-assigned this Mar 11, 2026
@davidjeddy davidjeddy added the enhancement New feature or request label Mar 11, 2026
@davidjeddy davidjeddy marked this pull request as ready for review March 11, 2026 08:49
… they fail. But main branch should always be successful.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 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/pr-scan.yml:
- Around line 3-24: The workflow currently uses pull_request_target which causes
the Build job to checkout the base branch (actions/checkout@v4) and perform a
Docker build with elevated permissions (see job "build" and permissions like
security-events: write); change the trigger for the scanning/build path from
pull_request_target to pull_request (or split the workflow so privileged
publish/SARIF upload remains under pull_request_target while the untrusted
Docker build runs under pull_request) and ensure you do not checkout the PR head
with write-scoped permissions in the pull_request_target job.

In `@CHANGES.md`:
- Line 5: The changelog entry currently references the wrong filename
"CHANGELOG.md"; update the text in CHANGES.md to point to the correct file name
"CHANGES.md" (replace the string "CHANGELOG.md" with "CHANGES.md") so links and
references resolve to the existing changelog file.

In `@Dockerfile`:
- Line 1: The Dockerfile hardcodes the PHP ABI suffix
"no-debug-non-zts-20240924" in the COPY operations (the entries that reference
that string) which will mismatch the declared BASEIMAGE
"php:8.5.3-cli-alpine3.23"; change the Dockerfile to compute the extension
directory dynamically (e.g., run php -r 'echo ini_get("extension_dir");' or
similar at build-time and store it in a shell variable/ARG/ENV like EXT_DIR) and
use that variable in the COPY targets instead of the literal
"no-debug-non-zts-20240924", updating the COPY lines that reference the
hardcoded suffix so they write into the actual extension directory returned by
PHP.

In `@README.md`:
- Line 74: Fix the typo in the README note by changing the sentence "Note: Build
of the image and push to the registry shoudl be handle by automation." to use
correct wording: "Note: Build of the image and push to the registry should be
handled by automation." Ensure only the misspelled words "shoudl" -> "should"
and "handle" -> "handled" are corrected in that sentence.
- Around line 39-41: Update the README examples that pipe commands into tee so
the pipeline exit status reflects the first command (e.g., docker build, trivy,
container-structure-test, docker push): add set -o pipefail at the start of each
example and redirect stderr to stdout for timed commands (e.g., time docker
build ... 2>&1 | tee ...) so failures are not masked by tee. Also correct the
repeated typo "shoudl be handle" to "should be handled" in the specified
occurrences.

In `@tests.yaml`:
- Line 7: The test is asserting an exact ImageMagick patch version in tests.yaml
(expectedOutput: [".*ImageMagick 7.1.2.15"]) which will break when Alpine
updates packages; either loosen the matcher or pin the package in Dockerfile:
update tests.yaml's expectedOutput to a more tolerant regex (e.g.,
".*ImageMagick 7\\.1\\.2\\." or ".*ImageMagick 7\\.1\\.") to allow patch
updates, or modify the Dockerfile entries that install imagemagick (both the
build and final stage apk add/install and any apk upgrade invocations) to pin
the package to the exact version you want (use the Alpine package release
suffix) so the installed ImageMagick stays deterministic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c961301c-1344-4213-8bac-e71afd856b68

📥 Commits

Reviewing files that changed from the base of the PR and between 7700869 and fa5df86.

📒 Files selected for processing (8)
  • .dockerignore
  • .github/workflows/pr-scan.yml
  • .github/workflows/trivy.yml
  • .gitignore
  • CHANGES.md
  • Dockerfile
  • README.md
  • tests.yaml

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/trivy.yml (1)

37-42: ⚠️ Potential issue | 🟠 Major

Use format: sarif instead of template syntax.

The format: 'template' with template: '@/contrib/sarif.tpl' is not compatible with trivy-action 0.35.0. The correct SARIF configuration is:

format: sarif
output: 'trivy-results.sarif'

Remove the template field—it is not a valid input for this version.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/trivy.yml around lines 37 - 42, Update the Trivy action
step that uses aquasecurity/trivy-action@0.35.0: remove the invalid "template"
input and set "format" to sarif (i.e., replace format: 'template' + template:
'@/contrib/sarif.tpl' with format: sarif) while keeping output:
'trivy-results.sarif'; locate the step by the uses:
aquasecurity/trivy-action@0.35.0 entry to make the change.
🧹 Nitpick comments (2)
TODO.md (1)

12-12: Minor: Remove leading space in code span.

The code span has a space before the pipe character which triggers MD038.

Suggested fix
-- capture build logs via ` | tee "build-$(date +%s).log"`
+- capture build logs via `| tee "build-$(date +%s).log"`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@TODO.md` at line 12, Remove the leading space inside the inline code span
containing the pipe command so the code reads `| tee "build-$(date +%s).log"`
(no space before the |); locate the inline snippet shown as ` | tee
"build-$(date +%s).log"` in the TODO.md content and edit it to eliminate the
extra space within the backticks to satisfy MD038.
README.md (1)

3-3: Travis CI badge may be outdated.

The repository uses GitHub Actions workflows (test.yml, trivy.yml, build-and-push.yml, pr-scan.yml), not Travis CI. Consider updating to a GitHub Actions badge.

Suggested replacement
-[![Build Status](https://img.shields.io/travis/com/appwrite/docker-base?style=flat-square)](https://travis-ci.com/appwrite/docker-base)
+[![Build Status](https://github.com/appwrite/docker-base/actions/workflows/test.yml/badge.svg)](https://github.com/appwrite/docker-base/actions/workflows/test.yml)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 3, Replace the outdated Travis CI badge markdown reference
"[![Build
Status](https://img.shields.io/travis/com/appwrite/docker-base?style=flat-square)](https://travis-ci.com/appwrite/docker-base)"
in README.md with a GitHub Actions badge that points to the appropriate
workflow(s) (e.g., test.yml or build-and-push.yml); remove the travis URL and
add the corresponding actions badge(s) using the GitHub Actions badge URL
pattern for the workflow names so the README reflects the current CI system.
🤖 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/pr-scan.yml:
- Around line 1-3: The workflow titled "PR Security Scan" currently triggers on
all pushes (the `on: push` stanza) which mismatches the name and may duplicate
scans from `trivy.yml`; update this workflow by either changing its trigger from
`push` to `pull_request` (if it should run only on PRs), removing the workflow
entirely if `trivy.yml` already covers the same scans, or renaming the workflow
title to reflect branch push scanning; look for the workflow name "PR Security
Scan", the `on: push` trigger, and the separate `trivy.yml` workflow to make the
appropriate change.

In `@CHANGES.md`:
- Line 9: Fix the typo in CHANGES.md by updating the line that reads "base_image
and php_build_date to containber labels" to use the correct word "container" so
it reads "base_image and php_build_date to container labels"; modify the exact
string in CHANGES.md to correct "containber" → "container".

In `@Dockerfile`:
- Around line 194-195: The Dockerfile uses $TZ in the RUN ln -snf
/usr/share/zoneinfo/$TZ ... command but TZ is never declared; add a build-time
ARG (e.g., ARG TZ="UTC") and/or ENV TZ="${TZ}" before that RUN step so $TZ is
defined (or remove the two lines if timezone is configured elsewhere); update
the Dockerfile around the RUN ln -snf invocation to reference the declared
ARG/ENV TZ to ensure the symlink and /etc/timezone write succeed.
- Around line 22-59: The RUN layer in the Dockerfile reuses the ineffective
pattern "apk add --no-cache --virtual .deps" followed by a separate "&& apk add
--no-cache" which prevents packages from being installed into the ".deps"
virtual package; change the RUN sequence so the packages intended as build
dependencies are passed directly to the same "apk add --no-cache --virtual
.deps" invocation (i.e., move the list of build tools/libraries such as
autoconf, automake, g++, gcc, make, openssl-dev, postgresql-dev, etc. into the
.deps invocation) and only keep a separate non-virtual apk add for runtime
packages if needed, ensuring the virtual name ".deps" actually contains the
build-time packages to be removable later.
- Around line 197-235: The .deps virtual package is created empty and later
deleted, so fix the RUN block in the Dockerfile: either remove the "apk add
--no-cache --virtual .deps" and the trailing "&& apk del .deps" if all listed
packages are needed at runtime, or group true build-only packages (e.g., g++,
gcc, make, autoconf, automake, libstdc++, pkg-config-like build deps) into the
virtual package and keep runtime packages outside it so that "apk del .deps"
actually removes only build-only dependencies; update the RUN command
accordingly to reflect the chosen option.

In `@README.md`:
- Around line 39-41: Update the README entry that currently shows the incorrect
Docker command string "docker-buildx build --no-cache --tag appwrite/base:latest
." to the correct invocation "docker buildx build --no-cache --tag
appwrite/base:latest ."; locate the shell block in README.md (the code fence
containing the time docker-buildx line) and replace the hyphenated command with
the space-separated "docker buildx" form so the example uses the proper Docker
Buildx CLI.

---

Outside diff comments:
In @.github/workflows/trivy.yml:
- Around line 37-42: Update the Trivy action step that uses
aquasecurity/trivy-action@0.35.0: remove the invalid "template" input and set
"format" to sarif (i.e., replace format: 'template' + template:
'@/contrib/sarif.tpl' with format: sarif) while keeping output:
'trivy-results.sarif'; locate the step by the uses:
aquasecurity/trivy-action@0.35.0 entry to make the change.

---

Nitpick comments:
In `@README.md`:
- Line 3: Replace the outdated Travis CI badge markdown reference "[![Build
Status](https://img.shields.io/travis/com/appwrite/docker-base?style=flat-square)](https://travis-ci.com/appwrite/docker-base)"
in README.md with a GitHub Actions badge that points to the appropriate
workflow(s) (e.g., test.yml or build-and-push.yml); remove the travis URL and
add the corresponding actions badge(s) using the GitHub Actions badge URL
pattern for the workflow names so the README reflects the current CI system.

In `@TODO.md`:
- Line 12: Remove the leading space inside the inline code span containing the
pipe command so the code reads `| tee "build-$(date +%s).log"` (no space before
the |); locate the inline snippet shown as ` | tee "build-$(date +%s).log"` in
the TODO.md content and edit it to eliminate the extra space within the
backticks to satisfy MD038.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 931d0f90-4158-4617-a98c-5038190e049b

📥 Commits

Reviewing files that changed from the base of the PR and between a2e5fd8 and 5b993d0.

📒 Files selected for processing (8)
  • .github/workflows/build-and-push.yml
  • .github/workflows/pr-scan.yml
  • .github/workflows/test.yml
  • .github/workflows/trivy.yml
  • CHANGES.md
  • Dockerfile
  • README.md
  • TODO.md

@davidjeddy davidjeddy force-pushed the clo-4095-cve-maintenance-01 branch 2 times, most recently from 8f5bf13 to 2ebf44f Compare March 11, 2026 13:48
@davidjeddy davidjeddy force-pushed the clo-4095-cve-maintenance-01 branch 2 times, most recently from 8fe4f52 to 0f7f450 Compare March 11, 2026 13:52
@davidjeddy davidjeddy force-pushed the clo-4095-cve-maintenance-01 branch 4 times, most recently from ede58b2 to 1bff3bd Compare March 12, 2026 13:45
@davidjeddy davidjeddy force-pushed the clo-4095-cve-maintenance-01 branch from 1bff3bd to 9ff192d Compare March 12, 2026 13:56
@davidjeddy davidjeddy marked this pull request as ready for review March 12, 2026 14:39
@davidjeddy davidjeddy changed the title DRAFT: ADD pr-scan.yml pipeline, UPDATE PHP to 8.5.3. UPDATE .gitignore, REA… Updated PHP, Swoole, Imagemagick, Add more CI scanning, ... Mar 12, 2026
@greptile-apps
Copy link

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR upgrades the Appwrite base Docker image to PHP 8.5.3 (via the phpswoole/swoole:php8.5-alpine base), bumps ImageMagick to 7.1.2.15 and Swoole to 6.2.0, and introduces a significant CI/CD overhaul — replacing QEMU-based cross-compilation with native GitHub ARM runners, adding Trivy scanning workflows, a Dive image-efficiency check, a container-structure-test workflow, and a lifecycle-policy job. The quality improvements are meaningful, but there are several blocking issues before this should merge:

  • CI won't deliver images on main: manifest_build_and_push_on_main depends on build_and_push, which is gated to run only on non-main branches. When a push lands on main, build_and_push is skipped, which causes manifest_build_and_push_on_main to also be skipped — meaning no production multi-arch manifest is ever published.
  • Empty release tag: The manifest_build_and_push_on_main job uses github.event.release.tag_name as the image tag, but the workflow trigger is on: push (not on: release). In a push context this expression is always an empty string, resulting in an invalidly-named manifest.
  • Build tools left in the final image: The apk add --no-cache --virtual .deps pattern in both the compile and final stages creates an empty virtual group — all packages (including gcc, g++, make, autoconf, automake, linux-headers, openssl-dev) land in the non-virtual layer and are never cleaned up by apk del .deps. In the final image this unnecessarily increases the attack surface.
  • Lifecycle policy is a no-op: The scheduled pruning workflow authenticates to DockerHub but its actual deletion logic is fully commented out.

Confidence Score: 1/5

  • Not safe to merge — the main-branch build/publish pipeline is broken and build tools are permanently embedded in the production image.
  • Two independent logic bugs in build-and-push.yml mean that no correctly-tagged image is ever pushed to DockerHub when code lands on main. Additionally, the Dockerfile's broken virtual-deps pattern leaves compiler toolchain packages in the final image, which both increases image size and widens the attack surface — directly counter to the PR's CVE-reduction goal.
  • .github/workflows/build-and-push.yml (broken main-branch delivery pipeline) and Dockerfile (build tools retained in final image due to empty virtual deps group).

Important Files Changed

Filename Overview
.github/workflows/build-and-push.yml Refactored to native ARM runners with matrix builds, but contains two critical logic bugs: manifest_build_and_push_on_main will always be skipped due to a broken needs dependency chain, and github.event.release.tag_name is empty in a push event context.
Dockerfile Base image switched to phpswoole/swoole, PHP bumped to 8.5.3, ImageMagick to 7.1.2.15, and extension paths parameterised via PHP_BUILD_DATE — but the apk --virtual .deps group is empty in both compile and final stages, leaving build tools (gcc, make, autoconf, etc.) permanently in the final image.
.github/workflows/lifecycle-policy.yml New scheduled workflow for pruning SHA-tagged images, but the actual deletion logic is entirely commented out — the job only authenticates to DockerHub and exits.
.github/workflows/trivy.yml Updated to run on all pushes, upgraded action versions, switched to SARIF format, and uses sarif_file: '.' to upload all SARIF files from the working directory — straightforward and correct.
.github/workflows/scheduled-trivy.yml New workflow for scheduled Trivy scans on push/PR to main and weekly on Saturdays; logic is correct and well-structured.
tests.yaml Updated expected ImageMagick version to 7.1.2.*, added PHP 8.5.3 version check and Swoole 6.2.0 version check — good alignment with Dockerfile version bumps. Missing newline at end of file.

Sequence Diagram

sequenceDiagram
    participant GH as GitHub Push Event
    participant BAP as build_and_push<br/>(if: ref != main)
    participant MFF as manifest_build_and_push_on_feature<br/>(if: ref != main)
    participant MFM as manifest_build_and_push_on_main<br/>(if: ref == main)
    participant DH as DockerHub

    Note over GH,DH: Feature branch push (ref != main)
    GH->>BAP: trigger (condition met)
    BAP->>BAP: Build amd64 + arm64 images
    BAP->>DH: Push sha-amd64 + sha-arm64
    BAP->>MFF: needs satisfied → trigger
    MFF->>DH: Create + push sha manifest

    Note over GH,DH: Main branch push (ref == main) — BROKEN
    GH->>BAP: trigger (condition FALSE → skipped)
    BAP--xMFM: needs: build_and_push skipped → MFM also skipped
    Note over MFM: manifest_build_and_push_on_main<br/>never runs on main
    Note over MFM: github.event.release.tag_name<br/>also empty in push context
Loading

Comments Outside Diff (1)

  1. .github/workflows/lifecycle-policy.yml, line 199-216 (link)

    Lifecycle policy job body is entirely commented out

    The prune_sha_tagged_images job currently only logs in to DockerHub — the actual pruning logic is commented out (lines 208–216). As a result, this scheduled workflow runs daily, authenticates, and then does nothing. Consider either:

    • Implementing the pruning step (or a DockerHub Hub API call equivalent), or
    • Keeping the workflow disabled / not merged until the logic is ready

    The comment at line 208 (# TODO pull all the images in the registry before running this) also suggests this is incomplete work.

Last reviewed commit: 36c11ed

@davidjeddy
Copy link
Contributor Author

@coderabbitai resume

@coderabbitai
Copy link

coderabbitai bot commented Mar 12, 2026

✅ Actions performed

Reviews resumed.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (5)
README.md (2)

82-84: ⚠️ Potential issue | 🟡 Minor

Pipeline exit code may be masked by tee.

Without set -o pipefail, the shell returns tee's exit status rather than docker push's. If the push fails, the pipeline may still report success.

Suggested fix
 ```bash
+set -o pipefail
 docker push appwrite/base:latest | tee "push-$(date +%s).log"
 ```
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 82 - 84, The pipeline may mask docker push failures
because the command uses a pipe to tee; add shell strictness so failures
propagate by inserting set -o pipefail (or set -euo pipefail) before the
pipeline line that runs docker push appwrite/base:latest | tee "push-$(date
+%s).log" so the shell returns docker push's exit code instead of tee's; ensure
this change is added in README examples or any script snippets that run this
pipeline.

80-80: ⚠️ Potential issue | 🟡 Minor

Fix grammar: "should be handle" → "should be handled".

Suggested fix
-Pushing a built image to a repository should be handle by automation.
+Pushing a built image to a repository should be handled by automation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 80, Fix the grammar in the README by changing the phrase
"Pushing a built image to a repository should be handle by automation." to
"Pushing a built image to a repository should be handled by automation." —
update the sentence text in the README (search for the exact phrase "should be
handle") and replace it with the corrected "should be handled".
.github/workflows/lifecycle-policy.yml (1)

18-36: ⚠️ Potential issue | 🟡 Minor

Workflow has no functional cleanup action.

A previous reviewer noted this workflow doesn't do anything currently. The entire cleanup logic (lines 28-36) is commented out, leaving only a DockerHub login step. Either implement the cleanup action or remove this workflow until it's ready.

Also, line 3 has a typo: "ordred" should be "ordered".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/lifecycle-policy.yml around lines 18 - 36, The workflow
job prune_sha_tagged_images currently only logs into DockerHub and the actual
cleanup steps (the actions/delete-package-versions step and associated
ignore-versions/min-versions-to-keep/package settings) are commented out, and
there is also a typo in the workflow metadata ("ordred" → "ordered"); either
implement the cleanup by uncommenting and validating the
actions/delete-package-versions step and its inputs (ignore-versions,
min-versions-to-keep, package-name, package-type) and adding any required
pre-pull/backup steps, or remove the prune_sha_tagged_images job entirely until
ready; also fix the typo by replacing "ordred" with "ordered" wherever it
appears in the workflow.
Dockerfile (2)

185-223: ⚠️ Potential issue | 🟠 Major

Final-stage .deps cleanup is ineffective and leaves build tools in runtime image.

The pattern at Line 188 (--virtual .deps with no packages) followed by a separate apk add means .deps is empty; Line 222 removes nothing. This keeps compiler/build packages in the final image.

Suggested fix
 RUN \
   apk update && \
   apk upgrade && \
-  apk add --no-cache --virtual .deps \
-    && apk add --no-cache \
+  apk add --no-cache --virtual .build-deps \
     autoconf \
     automake \
+    g++ \
+    gcc \
+    make \
+    linux-headers \
+    openssl-dev \
+    postgresql-dev \
+    curl-dev \
+    zstd-dev \
+    yaml-dev \
+    lz4-dev \
+    imagemagick-dev \
+    libjpeg-turbo-dev \
+    libpng-dev \
+    libjxl-dev \
+    libmaxminddb-dev \
+    jpeg-dev \
+  && apk add --no-cache \
     brotli-dev \
     certbot \
-    curl-dev \
     docker-cli \
     docker-cli-compose \
-    g++ \
-    gcc \
     git \
     imagemagick \
     imagemagick-heic \
-    jpeg-dev \
     libavif \
     libgomp \
     libheif \
-    libjpeg-turbo-dev \
-    libjxl-dev \
-    libmaxminddb-dev \
-    libpng-dev \
     libstdc++ \
     libwebp \
-    linux-headers \
-    lz4-dev \
-    make \
-    openssl-dev \
-    postgresql-dev \
     rsync \
-    yaml-dev \
     zip \
-    zstd-dev \
-  && apk del .deps \
+  && apk del .build-deps \
   && rm -rf /var/cache/apk/*
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 185 - 223, The RUN block defines a virtual package
`.deps` with no packages then installs all packages in a separate apk add, so
`apk del .deps` is a no-op and build tools remain in the final image; fix by
grouping build-only packages into a named virtual package (e.g., `.build-deps`)
when installing (include compilers and dev libs like g++, gcc, make, autoconf,
automake, *-dev, etc.) and install runtime packages separately, then run `apk
del .build-deps` before cleanup; update the RUN block that contains `--virtual
.deps` and the subsequent `apk add` so the virtual name actually wraps the
build-only packages or use a two-step install where `.build-deps` is removed
prior to `rm -rf /var/cache/apk/*`.

182-183: ⚠️ Potential issue | 🟠 Major

TZ is used before declaration.

Line 182 and Line 183 reference $TZ, but no ARG TZ/ENV TZ is declared in this file, so timezone setup is unreliable.

Suggested fix
+ARG TZ="UTC"
+ENV TZ="${TZ}"
+
 RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && \
   echo $TZ > /etc/timezone
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 182 - 183, The RUN uses $TZ before it's declared;
add a build ARG and/or environment variable for TZ and set a sensible default so
the ln/echo steps use a defined value—e.g., declare ARG TZ (with a default like
UTC) and then set ENV TZ=$TZ (or at minimum ENV TZ=UTC) before the RUN that
references $TZ so the timezone setup in RUN ln -snf /usr/share/zoneinfo/$TZ
/etc/localtime && echo $TZ > /etc/timezone reliably uses a defined TZ.
🧹 Nitpick comments (3)
.github/workflows/trivy.yml (2)

4-4: Trivy scan triggers on all branches.

on: push without branch filters will run scans on every push to any branch, which may consume significant CI resources. Consider restricting to main/release branches if full-branch scanning isn't required:

on:
  push:
    branches: [ "main" ]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/trivy.yml at line 4, The workflow currently triggers Trivy
scans on every push via the top-level "on: push" event; restrict this to
specific branches by replacing the bare "on: push" trigger with a push event
that includes a branches filter (e.g., only "main" or release branches) so scans
run only for those branches and avoid running on all branch pushes.

37-38: Same sarif_file: '.' concern as scheduled-trivy.yml.

Consider using the explicit filename trivy-image-results.sarif for clarity and predictability.

Suggested fix
       - name: Upload Trivy scan results
         uses: github/codeql-action/upload-sarif@v4
         with:
-          sarif_file: '.'
+          sarif_file: 'trivy-image-results.sarif'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/trivy.yml around lines 37 - 38, Replace the ambiguous
sarif_file value (currently set to '.') with an explicit filename to make
outputs predictable; update the key sarif_file to "trivy-image-results.sarif" in
the workflow step (the with: block that contains sarif_file) so the Trivy action
writes a named SARIF file, and do the same change in any sibling workflow
(scheduled-trivy.yml) if present to keep behavior consistent.
.github/workflows/scheduled-trivy.yml (1)

43-44: Use explicit SARIF filename for clarity.

The Trivy step outputs trivy-image-results.sarif, so using sarif_file: 'trivy-image-results.sarif' instead of '.' makes the intention explicit and improves maintainability by matching the declared output filename.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/scheduled-trivy.yml around lines 43 - 44, The workflow
currently sets the Trivy action output with sarif_file: '.' which is ambiguous;
update the Trivy step to explicitly reference the generated SARIF filename
(e.g., set sarif_file: 'trivy-image-results.sarif') so the step consuming the
artifact matches the actual output name and improves clarity in the
scheduled-trivy.yml 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.yml:
- Around line 16-17: The workflow prevents the manifest job from running on main
and uses a release-only field in a push event; update the job dependency and
event usage: ensure the job manifest_build_and_push_on_main does not depend only
on build_and_push when running on refs/heads/main (e.g., remove or conditionally
gate the needs: build_and_push for main or add a separate job path that runs on
main), and change the workflow trigger or payload references so
github.event.release.tag_name is only used when the workflow is triggered by
release events (either switch to on: release or guard access to
github.event.release.tag_name with a conditional that reads from the push ref
when on: push). Adjust the conditional at the top (the github.ref check) and the
manifest steps to use the correct tag variable for each event to make the
manifest creation reachable and avoid undefined tag values.

In @.github/workflows/dive.yml:
- Around line 23-26: The Dive step uses yuichielectric/dive-action@0.0.4 but
doesn't pass the repository's .dive-ci policy file, so thresholds won't be
enforced; update the Dive job step (the step named "Dive" that currently sets
the with.image input) to include the action input that specifies the config file
and point it to ".dive-ci" (e.g., add a with entry like config: ".dive-ci" or
config-file: ".dive-ci" depending on the action's input name) so the action
loads the repository policy when running against the image.

In `@CHANGES.md`:
- Around line 18-19: Replace all occurrences of the product name spelled
"Github" with the correct, consistent "GitHub" in the changelog entries;
specifically update the two lines shown in the diff (“.github/*.yml steps
updated to latest versions” and any other entries noted at lines 24-25 and 39)
so the product name uses the correct capitalization ("GitHub") everywhere in
CHANGES.md.

In `@README.md`:
- Line 3: Replace the Travis CI badge markdown in the README with a GitHub
Actions badge that references the main workflow name build-and-push.yml: locate
the existing badge line containing "travis" or the exact string "[![Build
Status](https://img.shields.io/travis/com/appwrite/docker-base?style=flat-square)](https://travis-ci.com/appwrite/docker-base)"
and update it to use the GitHub Actions badge URL for the repository and
workflow (pointing to the build-and-push.yml workflow) so the README shows the
correct Actions build status and link.

---

Duplicate comments:
In @.github/workflows/lifecycle-policy.yml:
- Around line 18-36: The workflow job prune_sha_tagged_images currently only
logs into DockerHub and the actual cleanup steps (the
actions/delete-package-versions step and associated
ignore-versions/min-versions-to-keep/package settings) are commented out, and
there is also a typo in the workflow metadata ("ordred" → "ordered"); either
implement the cleanup by uncommenting and validating the
actions/delete-package-versions step and its inputs (ignore-versions,
min-versions-to-keep, package-name, package-type) and adding any required
pre-pull/backup steps, or remove the prune_sha_tagged_images job entirely until
ready; also fix the typo by replacing "ordred" with "ordered" wherever it
appears in the workflow.

In `@Dockerfile`:
- Around line 185-223: The RUN block defines a virtual package `.deps` with no
packages then installs all packages in a separate apk add, so `apk del .deps` is
a no-op and build tools remain in the final image; fix by grouping build-only
packages into a named virtual package (e.g., `.build-deps`) when installing
(include compilers and dev libs like g++, gcc, make, autoconf, automake, *-dev,
etc.) and install runtime packages separately, then run `apk del .build-deps`
before cleanup; update the RUN block that contains `--virtual .deps` and the
subsequent `apk add` so the virtual name actually wraps the build-only packages
or use a two-step install where `.build-deps` is removed prior to `rm -rf
/var/cache/apk/*`.
- Around line 182-183: The RUN uses $TZ before it's declared; add a build ARG
and/or environment variable for TZ and set a sensible default so the ln/echo
steps use a defined value—e.g., declare ARG TZ (with a default like UTC) and
then set ENV TZ=$TZ (or at minimum ENV TZ=UTC) before the RUN that references
$TZ so the timezone setup in RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime
&& echo $TZ > /etc/timezone reliably uses a defined TZ.

In `@README.md`:
- Around line 82-84: The pipeline may mask docker push failures because the
command uses a pipe to tee; add shell strictness so failures propagate by
inserting set -o pipefail (or set -euo pipefail) before the pipeline line that
runs docker push appwrite/base:latest | tee "push-$(date +%s).log" so the shell
returns docker push's exit code instead of tee's; ensure this change is added in
README examples or any script snippets that run this pipeline.
- Line 80: Fix the grammar in the README by changing the phrase "Pushing a built
image to a repository should be handle by automation." to "Pushing a built image
to a repository should be handled by automation." — update the sentence text in
the README (search for the exact phrase "should be handle") and replace it with
the corrected "should be handled".

---

Nitpick comments:
In @.github/workflows/scheduled-trivy.yml:
- Around line 43-44: The workflow currently sets the Trivy action output with
sarif_file: '.' which is ambiguous; update the Trivy step to explicitly
reference the generated SARIF filename (e.g., set sarif_file:
'trivy-image-results.sarif') so the step consuming the artifact matches the
actual output name and improves clarity in the scheduled-trivy.yml workflow.

In @.github/workflows/trivy.yml:
- Line 4: The workflow currently triggers Trivy scans on every push via the
top-level "on: push" event; restrict this to specific branches by replacing the
bare "on: push" trigger with a push event that includes a branches filter (e.g.,
only "main" or release branches) so scans run only for those branches and avoid
running on all branch pushes.
- Around line 37-38: Replace the ambiguous sarif_file value (currently set to
'.') with an explicit filename to make outputs predictable; update the key
sarif_file to "trivy-image-results.sarif" in the workflow step (the with: block
that contains sarif_file) so the Trivy action writes a named SARIF file, and do
the same change in any sibling workflow (scheduled-trivy.yml) if present to keep
behavior consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e204670c-3eae-4161-8bbb-d99fe0904e2d

📥 Commits

Reviewing files that changed from the base of the PR and between 5b993d0 and 36c11ed.

📒 Files selected for processing (14)
  • .dive-ci
  • .github/workflows/build-and-push.yml
  • .github/workflows/dive.yml
  • .github/workflows/lifecycle-policy.yml
  • .github/workflows/scheduled-trivy.yml
  • .github/workflows/structure-test.yml
  • .github/workflows/test.yml
  • .github/workflows/trivy.yml
  • CHANGES.md
  • Dockerfile
  • README.md
  • SECURITY.md
  • TODO.md
  • tests.yaml
💤 Files with no reviewable changes (1)
  • .github/workflows/test.yml

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
CHANGES.md (2)

23-24: ⚠️ Potential issue | 🟡 Minor

Use consistent product naming: GitHub (capital H).

The official product name uses a capital "H". Also at line 38.

,

📝 Suggested fix
-* Github action for container-structure-test now uses a marketplace action
-* Github action runners pinned to Ubuntu 24.04
+* GitHub Action for container-structure-test now uses a marketplace action
+* GitHub Action runners pinned to Ubuntu 24.04
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CHANGES.md` around lines 23 - 24, Replace the inconsistent product name
"Github" with the correct "GitHub" in the CHANGES.md entries shown (e.g., update
the strings "Github action for container-structure-test now uses a marketplace
action" and "Github action runners pinned to Ubuntu 24.04") and also fix the
other occurrence noted around line 38 so all mentions use "GitHub".

38-38: ⚠️ Potential issue | 🟡 Minor

Fix grammar and capitalization.

"Github" should be "GitHub", and "Setup" as a verb should be lowercase "set up".

📝 Suggested fix
-* Github action to Setup QEMU as GitHub now provides native ARM runners
+* GitHub Action to set up QEMU as GitHub now provides native ARM runners
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CHANGES.md` at line 38, Update the CHANGES.md entry to fix capitalization and
grammar: change "Github" to "GitHub" and use the verb form "set up" instead of
"Setup" so the line reads something like "GitHub action to set up QEMU as GitHub
now provides native ARM runners." Ensure only the two words are corrected and
preserve the rest of the sentence.
.github/workflows/build-and-push.yml (1)

67-87: ⚠️ Potential issue | 🔴 Critical

manifest_build_and_push_on_main uses github.event.release.tag_name but will only run on push events.

The job condition if: github.ref == 'refs/heads/main' (line 68) matches push events to main, but lines 81 and 87 reference github.event.release.tag_name, which is undefined for push events. This will cause the manifest creation and push steps to fail with empty tag values.

Additionally, for release events, github.ref is typically the tag ref (e.g., refs/tags/v1.0.0), not refs/heads/main, so this job won't run on releases either.

,

🐛 Suggested fix: Use conditional tagging based on event type
   manifest_build_and_push_on_main:
-    if: github.ref == 'refs/heads/main'
+    if: github.event_name == 'release'
     needs: build_and_push
     runs-on: ubuntu-24.04
     steps:

Alternatively, if you want to publish to main on every push, use github.sha instead of the release tag:

       - name: Create manifest
         run: |
             docker manifest create \
-              ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.event.release.tag_name }} \
+              ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest \
               ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.sha }}-amd64 \
               ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.sha }}-arm64

       - name: Push manifest
         run: |
-            docker manifest push ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ github.event.release.tag_name }}
+            docker manifest push ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build-and-push.yml around lines 67 - 87, The
manifest_build_and_push_on_main job is using github.event.release.tag_name but
is gated with if: github.ref == 'refs/heads/main' so release tag is undefined on
push; change the job to compute a TAG variable based on the event (e.g., if
github.event_name == 'release' use github.event.release.tag_name else use
github.sha or github.ref name) and use that TAG in the docker manifest create
and push commands, or alternatively broaden the job condition to run on both
push and release events and branch the steps accordingly; update references to
github.event.release.tag_name in the Create manifest and Push manifest steps to
use the computed TAG (and keep github.sha-based fallbacks) so the manifest
commands never receive an empty tag.
🧹 Nitpick comments (1)
.github/workflows/trivy.yml (1)

38-45: Consider using the explicit SARIF filename in the upload step.

The Trivy step outputs to trivy-image-results.sarif (line 38), but the upload step uses sarif_file: '.' which uploads all SARIF files in the current directory. While this works, using the explicit filename would be more precise and avoid accidentally uploading unintended files.

♻️ Suggested fix
       - name: Upload Trivy scan results
         uses: github/codeql-action/upload-sarif@v4
         with:
-          sarif_file: '.'
+          sarif_file: 'trivy-image-results.sarif'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/trivy.yml around lines 38 - 45, Update the "Upload Trivy
scan results" step to reference the explicit SARIF filename instead of using the
directory wildcard; change the sarif_file input for the upload step (the step
named "Upload Trivy scan results" that uses
github/codeql-action/upload-sarif@v4) to point to 'trivy-image-results.sarif' so
only that specific file is uploaded.
🤖 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/trivy.yml:
- Line 27: The workflow references a non-existent runner version "uses:
actions/checkout@v6.0.2"; replace that tag with a valid one such as "uses:
actions/checkout@v5" to track the latest v5.x release or pin explicitly to
"uses: actions/checkout@v5.0.1" to fix the invalid version; update the uses
entry in the Trivy workflow where "actions/checkout@v6.0.2" appears.

---

Duplicate comments:
In @.github/workflows/build-and-push.yml:
- Around line 67-87: The manifest_build_and_push_on_main job is using
github.event.release.tag_name but is gated with if: github.ref ==
'refs/heads/main' so release tag is undefined on push; change the job to compute
a TAG variable based on the event (e.g., if github.event_name == 'release' use
github.event.release.tag_name else use github.sha or github.ref name) and use
that TAG in the docker manifest create and push commands, or alternatively
broaden the job condition to run on both push and release events and branch the
steps accordingly; update references to github.event.release.tag_name in the
Create manifest and Push manifest steps to use the computed TAG (and keep
github.sha-based fallbacks) so the manifest commands never receive an empty tag.

In `@CHANGES.md`:
- Around line 23-24: Replace the inconsistent product name "Github" with the
correct "GitHub" in the CHANGES.md entries shown (e.g., update the strings
"Github action for container-structure-test now uses a marketplace action" and
"Github action runners pinned to Ubuntu 24.04") and also fix the other
occurrence noted around line 38 so all mentions use "GitHub".
- Line 38: Update the CHANGES.md entry to fix capitalization and grammar: change
"Github" to "GitHub" and use the verb form "set up" instead of "Setup" so the
line reads something like "GitHub action to set up QEMU as GitHub now provides
native ARM runners." Ensure only the two words are corrected and preserve the
rest of the sentence.

---

Nitpick comments:
In @.github/workflows/trivy.yml:
- Around line 38-45: Update the "Upload Trivy scan results" step to reference
the explicit SARIF filename instead of using the directory wildcard; change the
sarif_file input for the upload step (the step named "Upload Trivy scan results"
that uses github/codeql-action/upload-sarif@v4) to point to
'trivy-image-results.sarif' so only that specific file is uploaded.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b08e1977-2324-4587-b5a3-bba05809c8cb

📥 Commits

Reviewing files that changed from the base of the PR and between 36c11ed and 82a41e6.

📒 Files selected for processing (3)
  • .github/workflows/build-and-push.yml
  • .github/workflows/trivy.yml
  • CHANGES.md

@davidjeddy davidjeddy force-pushed the clo-4095-cve-maintenance-01 branch from 82a41e6 to 60392ba Compare March 12, 2026 21:57
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (5)
Dockerfile (2)

188-223: ⚠️ Potential issue | 🟠 Major

apk del .deps is ineffective with the current install pattern.

The --virtual .deps install is empty, while actual packages are installed in a separate command, so Line 222 removes nothing from the final image.

Suggested fix
 RUN \
   apk update && \
   apk upgrade && \
-  apk add --no-cache --virtual .deps \
-    && apk add --no-cache \
+  apk add --no-cache --virtual .deps \
     autoconf \
     automake \
     brotli-dev \
     certbot \
     curl-dev \
     docker-cli \
     docker-cli-compose \
     g++ \
     gcc \
     git \
     imagemagick \
     imagemagick-dev \
     imagemagick-heic \
     jpeg-dev \
     libavif \
     libgomp \
     libheif \
     libjpeg-turbo-dev \
     libjxl-dev \
     libmaxminddb-dev \
     libpng-dev \
     libstdc++ \
     libwebp \
     linux-headers \
     lz4-dev \
     make \
     openssl-dev \
     postgresql-dev \
     rsync \
     yaml-dev \
     zip \
     zstd-dev \
   && apk del .deps \
   && rm -rf /var/cache/apk/*
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 188 - 223, The .deps virtual package is never
populated because --virtual .deps is applied to an empty apk add command, so apk
del .deps removes nothing; update the Dockerfile so the large apk add that
installs build packages uses --virtual .deps (i.e., move the --virtual .deps
token to the apk add that lists autoconf automake ... zstd-dev) so that those
packages are grouped under the .deps virtual package and can be removed with apk
del .deps, or alternatively remove the apk del .deps line if you intend to keep
those packages in the final image; refer to the apk add/ del commands and the
.deps virtual name when making the change.

182-183: ⚠️ Potential issue | 🟠 Major

Declare TZ before timezone setup.

$TZ is used on Line 182/183 without being declared in this stage, so timezone configuration can be incorrect.

Suggested fix
+ARG TZ="UTC"
+ENV TZ=${TZ}
 RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && \
   echo $TZ > /etc/timezone
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 182 - 183, The Dockerfile uses the TZ variable in
the RUN command that performs timezone setup (ln -snf /usr/share/zoneinfo/$TZ
/etc/localtime && echo $TZ > /etc/timezone) but TZ is not declared in this
stage; declare TZ earlier (e.g., add an ARG TZ=UTC or ENV TZ=UTC before that
RUN) or pass it via build ARG and then ENV (ARG TZ; ENV TZ=$TZ) so the RUN can
expand $TZ reliably, providing a sensible default like UTC.
CHANGES.md (1)

38-38: ⚠️ Potential issue | 🟡 Minor

Reword the QEMU removal note for readability.

Line 38 is hard to parse. Suggested wording: GitHub action to set up QEMU removed because GitHub now provides native ARM runners.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CHANGES.md` at line 38, Replace the hard-to-read changelog entry "GitHub
action to Setup QEMU as GitHub now provides native ARM runners" with the clearer
wording "GitHub action to set up QEMU removed because GitHub now provides native
ARM runners" (update the exact line in CHANGES.md that currently contains that
sentence).
README.md (1)

3-3: ⚠️ Potential issue | 🟡 Minor

Replace the stale Travis badge with a GitHub Actions badge.

Line 3 still points to Travis CI, which no longer reflects this repo’s CI status.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 3, Replace the stale Travis CI badge markdown in README.md
(the existing "[![Build
Status](https://img.shields.io/travis.com/appwrite/docker-base?style=flat-square)](https://travis-ci.com/appwrite/docker-base)"
snippet) with the GitHub Actions badge for this repository: update both the
image URL and the link target to point to your repository's workflow badge and
workflow page (e.g., use
"https://github.com/<OWNER>/<REPO>/actions/workflows/<WORKFLOW>.yml/badge.svg"
for the image and
"https://github.com/<OWNER>/<REPO>/actions/workflows/<WORKFLOW>.yml" for the
link), preserving the same "[![Build Status](...)](...)" markdown structure.
.github/workflows/build-and-push.yml (1)

45-87: ⚠️ Potential issue | 🔴 Critical

Fix manifest job gating for push vs release events.

Current conditions mix branch checks with release-only context fields. On push to main, github.event.release.tag_name is undefined; on release events, the on_main job won’t run because the ref is a tag, not refs/heads/main.

Suggested fix
   manifest_build_and_push_on_feature:
-    if: github.ref != 'refs/heads/main'
+    if: github.event_name == 'push' && github.ref != 'refs/heads/main'
     needs: build_and_push
     runs-on: ubuntu-24.04
@@
   manifest_build_and_push_on_main:
-    if: github.ref == 'refs/heads/main'
+    if: github.event_name == 'release' && github.event.action == 'published'
     needs: build_and_push
     runs-on: ubuntu-24.04
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build-and-push.yml around lines 45 - 87, The manifest jobs
mix branch-based gating and release-only context fields; update the two jobs so
the feature job keeps its branch check (manifest_build_and_push_on_feature with
if: github.ref != 'refs/heads/main') but never references release fields, while
the release job uses the release event condition (change
manifest_build_and_push_on_main to trigger when github.event_name == 'release'
or if: github.event_name == 'release' && github.ref startsWith 'refs/tags/') and
use github.event.release.tag_name for the manifest create/push there; for the
main-branch push path keep a separate job with if: github.ref ==
'refs/heads/main' that uses github.sha (or github.ref_name) for tags and does
not reference github.event.release.tag_name so release-only fields are only
accessed in the release-scoped job.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@TODO.md`:
- Line 5: Fix the typo in the TODO line that currently reads "cache it, the use
it downstream": change "the" to "then" so the line reads "cache it, then use it
downstream"; locate the TODO text containing "cache it, the use it downstream"
and update it accordingly.

---

Duplicate comments:
In @.github/workflows/build-and-push.yml:
- Around line 45-87: The manifest jobs mix branch-based gating and release-only
context fields; update the two jobs so the feature job keeps its branch check
(manifest_build_and_push_on_feature with if: github.ref != 'refs/heads/main')
but never references release fields, while the release job uses the release
event condition (change manifest_build_and_push_on_main to trigger when
github.event_name == 'release' or if: github.event_name == 'release' &&
github.ref startsWith 'refs/tags/') and use github.event.release.tag_name for
the manifest create/push there; for the main-branch push path keep a separate
job with if: github.ref == 'refs/heads/main' that uses github.sha (or
github.ref_name) for tags and does not reference github.event.release.tag_name
so release-only fields are only accessed in the release-scoped job.

In `@CHANGES.md`:
- Line 38: Replace the hard-to-read changelog entry "GitHub action to Setup QEMU
as GitHub now provides native ARM runners" with the clearer wording "GitHub
action to set up QEMU removed because GitHub now provides native ARM runners"
(update the exact line in CHANGES.md that currently contains that sentence).

In `@Dockerfile`:
- Around line 188-223: The .deps virtual package is never populated because
--virtual .deps is applied to an empty apk add command, so apk del .deps removes
nothing; update the Dockerfile so the large apk add that installs build packages
uses --virtual .deps (i.e., move the --virtual .deps token to the apk add that
lists autoconf automake ... zstd-dev) so that those packages are grouped under
the .deps virtual package and can be removed with apk del .deps, or
alternatively remove the apk del .deps line if you intend to keep those packages
in the final image; refer to the apk add/ del commands and the .deps virtual
name when making the change.
- Around line 182-183: The Dockerfile uses the TZ variable in the RUN command
that performs timezone setup (ln -snf /usr/share/zoneinfo/$TZ /etc/localtime &&
echo $TZ > /etc/timezone) but TZ is not declared in this stage; declare TZ
earlier (e.g., add an ARG TZ=UTC or ENV TZ=UTC before that RUN) or pass it via
build ARG and then ENV (ARG TZ; ENV TZ=$TZ) so the RUN can expand $TZ reliably,
providing a sensible default like UTC.

In `@README.md`:
- Line 3: Replace the stale Travis CI badge markdown in README.md (the existing
"[![Build
Status](https://img.shields.io/travis.com/appwrite/docker-base?style=flat-square)](https://travis-ci.com/appwrite/docker-base)"
snippet) with the GitHub Actions badge for this repository: update both the
image URL and the link target to point to your repository's workflow badge and
workflow page (e.g., use
"https://github.com/<OWNER>/<REPO>/actions/workflows/<WORKFLOW>.yml/badge.svg"
for the image and
"https://github.com/<OWNER>/<REPO>/actions/workflows/<WORKFLOW>.yml" for the
link), preserving the same "[![Build Status](...)](...)" markdown structure.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 117abfc4-191e-47da-a8ae-56a57d9016b0

📥 Commits

Reviewing files that changed from the base of the PR and between 82a41e6 and 60392ba.

📒 Files selected for processing (7)
  • .dive-ci.yml
  • .github/workflows/build-and-push.yml
  • .github/workflows/dive.yml
  • CHANGES.md
  • Dockerfile
  • README.md
  • TODO.md

@davidjeddy davidjeddy requested a review from stnguyen90 March 13, 2026 07:16
@davidjeddy davidjeddy requested a review from loks0n March 13, 2026 10:12
@loks0n loks0n dismissed stnguyen90’s stale review March 13, 2026 10:31

following up later

@davidjeddy davidjeddy merged commit b4fd48d into main Mar 13, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants