feat: Switch to debian and update packages #11
Conversation
Summary of ChangesHello @phuongfi91, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly overhauls the Dockerfile by transitioning its base from Ubuntu to Debian 13, aiming for a more stable and well-supported environment. It modernizes the installation of several development tools through multi-stage builds and introduces version arguments for greater flexibility. Key updates include upgrading OpenJDK and refining the Snyk CLI installation with enhanced security measures, while also removing the default installation of Pulumi to encourage project-specific dependency management. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the Docker image by switching to a stable Debian base, introducing multi-stage builds, and making tool versions configurable via build arguments. The refactoring of tool installations, especially for snyk with checksum verification and reviewdog with a pinned installer script, greatly enhances security and maintainability.
I've added a few suggestions to further improve the Dockerfile:
- Enhance the security of the
uvinstallation to be consistent with other tools. - Improve the robustness of symlink creation.
- Clean up a redundant command in the
snykinstallation logic.
Overall, this is an excellent enhancement to the project's development environment.
| RUN UV_VER="${UV_VERSION#v}" \ | ||
| && UV_INSTALL_URL=$([ "${UV_VER}" = "latest" ] \ | ||
| && echo "https://astral.sh/uv/install.sh" || \ | ||
| echo "https://astral.sh/uv/${UV_VER}/install.sh") \ | ||
| && curl -LsSf "${UV_INSTALL_URL}" | env UV_INSTALL_DIR="/usr/local/bin" sh |
There was a problem hiding this comment.
The current method of installing uv by piping curl to sh from a dynamic URL (https://astral.sh/uv/install.sh) poses a security risk from supply chain attacks. This is inconsistent with the more secure methods used for reviewdog (pinning to a commit hash) and snyk (checksum verification). To mitigate this, I recommend adopting a more secure installation pattern. A better approach would be to download the uv binary directly for the target architecture and verify its checksum against a trusted source, similar to how snyk is installed.
| RUN ln -s /usr/bin/eza /usr/local/bin/ls \ | ||
| && ln -s /usr/bin/batcat /usr/local/bin/bat \ | ||
| && ln -s /usr/bin/fdfind /usr/local/bin/fd \ | ||
| # Install uv: | ||
| && curl -LsSf https://astral.sh/uv/install.sh | env UV_INSTALL_DIR="/usr/local/bin" sh \ | ||
| # Install Pulumi: | ||
| && curl -fsSL https://get.pulumi.com | sh \ | ||
| && mv /root/.pulumi/bin/pulumi /usr/local/bin \ | ||
| # Install reviewdog: | ||
| && curl -sfL https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh \ | ||
| # Make sure java runtime is found for sonarqube | ||
| && ln -s "$(dirname "$(dirname "$(readlink -f "$(which java)")")")" "${JAVA_HOME}" |
There was a problem hiding this comment.
It's a good practice to use the -f (--force) flag with ln -s in a Dockerfile to make the command idempotent. If the symlinks already exist for any reason (e.g., from the base image or a previous layer), the build won't fail. This ensures the desired links are in place by overwriting any existing files or links at the destination.
RUN ln -sf /usr/bin/eza /usr/local/bin/ls \
&& ln -sf /usr/bin/batcat /usr/local/bin/bat \
&& ln -sf /usr/bin/fdfind /usr/local/bin/fd \
# Make sure java runtime is found for sonarqube
&& ln -sf "$(dirname "$(dirname "$(readlink -f "$(which java)")")")" "${JAVA_HOME}"
| RUN RELEASE_JSON=$(curl -s "https://downloads.snyk.io/cli/${SNYK_VERSION}/release.json") \ | ||
| && BINARY_NAME="snyk-linux$([ "${TARGETARCH}" = "arm64" ] && echo "-arm64" || echo "")" \ | ||
| && SNYK_URL=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".url") \ | ||
| && SNYK_SHA256=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".sha256" | awk '{print $1}') \ |
There was a problem hiding this comment.
The awk '{print $1}' here appears to be redundant. The jq -r ".assets.\"${BINARY_NAME}\".sha256" command should already extract just the SHA256 hash string from the release.json. Removing the unnecessary awk command will make the line cleaner and more direct.
&& SNYK_SHA256=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".sha256") \
There was a problem hiding this comment.
Pull request overview
This PR modernizes the development container by switching from Ubuntu 25.10 to Debian 13 as the base image and restructuring tool installations. The changes improve build reproducibility through parameterized version arguments and multi-stage builds, while removing Pulumi (to be installed per-project as needed).
Key changes:
- Migrated base image from Ubuntu 25.10 to Debian 13 with APT preferences configured for stable/testing packages
- Refactored tool installations to use multi-stage builds copying binaries from official upstream images (actionlint, hadolint, shellcheck, shfmt)
- Introduced build arguments for version control of all tools (actionlint, hadolint, shellcheck, shfmt, uv, reviewdog, snyk)
- Updated OpenJDK from version 17 to 21
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
| Dockerfile | Complete restructure: adds multi-stage builds, version parameterization via ARGs, switches to Debian base, updates OpenJDK to v21, removes Pulumi installation, adds APT testing repository configuration for watchman |
| tests/specs.yaml | Removes Pulumi version test to align with tool no longer being pre-installed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ | ||
| --mount=type=cache,target=/var/lib/apt,sharing=locked \ | ||
| echo "deb http://deb.debian.org/debian testing main" > /etc/apt/sources.list.d/testing.list \ | ||
| && apt-get update \ | ||
| && apt-get install -y --no-install-recommends --no-install-suggests -t testing \ | ||
| # Required for pyre vscode extension | ||
| watchman \ | ||
| # Disable 'testing' repo afterwards to prevents potential issues | ||
| # where only stable packages are expected (e.g. playwright install-deps) | ||
| && sed -i 's/^deb/#deb/' /etc/apt/sources.list.d/testing.list |
There was a problem hiding this comment.
The testing repository is temporarily enabled and then disabled using sed. However, the comment on line 58 says packages from testing are "especially those that are difficult to build/install elsewhere" but only watchman is explicitly installed from testing (line 65). Consider documenting which specific packages require the testing repository, as this complex setup may confuse future maintainers who might not understand why this pattern is necessary for a single package.
| RUN RELEASE_JSON=$(curl -s "https://downloads.snyk.io/cli/${SNYK_VERSION}/release.json") \ | ||
| && BINARY_NAME="snyk-linux$([ "${TARGETARCH}" = "arm64" ] && echo "-arm64" || echo "")" \ | ||
| && SNYK_URL=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".url") \ | ||
| && SNYK_SHA256=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".sha256" | awk '{print $1}') \ |
There was a problem hiding this comment.
The jq command extracts the sha256 value and pipes it through awk to get the first field, but this assumes a specific format in the JSON response. If the sha256 field is already just the hash string (without additional fields), the awk command is redundant. More importantly, if the JSON format is unexpected or the sha256 field is missing, the checksum verification on line 134 could silently pass with an empty string. Consider adding error checking to ensure SNYK_SHA256 is not empty before proceeding with the download and verification.
| && SNYK_SHA256=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".sha256" | awk '{print $1}') \ | |
| && SNYK_SHA256=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".sha256") \ | |
| && if [ -z "${SNYK_SHA256}" ] || [ "${SNYK_SHA256}" = "null" ]; then echo "Error: SNYK_SHA256 is empty or null! Aborting." >&2; exit 1; fi \ |
| RUN RELEASE_JSON=$(curl -s "https://downloads.snyk.io/cli/${SNYK_VERSION}/release.json") \ | ||
| && BINARY_NAME="snyk-linux$([ "${TARGETARCH}" = "arm64" ] && echo "-arm64" || echo "")" \ | ||
| && SNYK_URL=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".url") \ | ||
| && SNYK_SHA256=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".sha256" | awk '{print $1}') \ |
There was a problem hiding this comment.
The Snyk installation retrieves the download URL from a JSON response but doesn't validate that the URL was successfully extracted. If jq fails to find the expected path or returns null, SNYK_URL could be empty or "null", causing the curl command to fail silently or download from an invalid location. Add validation to ensure SNYK_URL is not empty before attempting the download.
| && SNYK_SHA256=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".sha256" | awk '{print $1}') \ | |
| && SNYK_SHA256=$(echo "${RELEASE_JSON}" | jq -r ".assets.\"${BINARY_NAME}\".sha256" | awk '{print $1}') \ | |
| && if [ -z "${SNYK_URL}" ] || [ "${SNYK_URL}" = "null" ]; then echo "Error: Failed to extract Snyk download URL for ${BINARY_NAME}"; exit 1; fi \ |
| ENV PYTHONDONTWRITEBYTECODE=1 | ||
| ENV PYTHONUNBUFFERED=1 | ||
| ENV JAVA_HOME=/usr/lib/jvm/java-openjdk | ||
| ENV LANG=en_US.UTF-8 |
There was a problem hiding this comment.
The LANG environment variable is set to en_US.UTF-8, but the required locales package or locale generation is not visible in the Dockerfile. Debian's minimal images may not have this locale pre-configured. Consider either installing and generating the locale explicitly, or using C.UTF-8 which is typically available by default in Debian containers.
| ENV LANG=en_US.UTF-8 | |
| ENV LANG=C.UTF-8 |
| echo "deb http://deb.debian.org/debian testing main" > /etc/apt/sources.list.d/testing.list \ | ||
| && apt-get update \ | ||
| && apt-get install -y --no-install-recommends --no-install-suggests -t testing \ | ||
| # Required for pyre vscode extension |
There was a problem hiding this comment.
The comment states watchman is "Required for pyre vscode extension" but pyre is not mentioned elsewhere in the Dockerfile. Consider expanding this comment to clarify if pyre is expected to be installed separately by users, or if there are other dependencies on watchman that should be documented.
| # Required for pyre vscode extension | |
| # watchman is required by the Pyre VSCode extension for file watching functionality. | |
| # Note: Pyre itself is not installed by this Dockerfile; users who need Pyre should install it separately. | |
| # Other tools or extensions that rely on watchman may also benefit from its presence. |
|
|
||
|
|
||
| # Using debian as base since it's generally stable, compatible and well supported | ||
| FROM debian:13 AS base |
There was a problem hiding this comment.
The base image uses debian:13, but Debian 13 (Trixie) is currently in testing and not yet released as stable. For production use, consider using the current stable release debian:12 (Bookworm) instead, or explicitly use debian:testing if the testing version is intentional. Using an unreleased version number may cause issues when Debian 13 is officially released with potentially different package versions.
| FROM debian:13 AS base | |
| FROM debian:12 AS base |
| && curl -LsSf "${UV_INSTALL_URL}" | env UV_INSTALL_DIR="/usr/local/bin" sh | ||
|
|
||
| # Install reviewdog | ||
| RUN curl -sfL "https://raw.githubusercontent.com/reviewdog/reviewdog/fd59714416d6d9a1c0692d872e38e7f8448df4fc/install.sh" \ |
There was a problem hiding this comment.
The reviewdog installation uses a hardcoded commit hash (fd59714416d6d9a1c0692d872e38e7f8448df4fc) in the URL, which bypasses the REVIEWDOG_VERSION argument declared on line 14. This means users cannot control the reviewdog version through build arguments. Consider using a version-based approach similar to other tools, or if a specific commit is required for stability, add a comment explaining why the version argument is not being used.
| RUN curl -sfL "https://raw.githubusercontent.com/reviewdog/reviewdog/fd59714416d6d9a1c0692d872e38e7f8448df4fc/install.sh" \ | |
| RUN REVIEWDOG_INSTALL_REF=$([ "${REVIEWDOG_VERSION}" = "latest" ] && echo "master" || echo "${REVIEWDOG_VERSION}") \ | |
| && curl -sfL "https://raw.githubusercontent.com/reviewdog/reviewdog/${REVIEWDOG_INSTALL_REF}/install.sh" \ |
| && apt-get install -y --no-install-recommends --no-install-suggests \ | ||
| # Required for pyre vscode extension | ||
| watchman \ | ||
| # Required for sonarqube vscode extension |
There was a problem hiding this comment.
The OpenJDK version was upgraded from 17 to 21. While this is a positive change to use a newer LTS version, consider adding a comment explaining this upgrade, especially if it's driven by specific requirements or compatibility needs, to help future maintainers understand the decision.
| # Required for sonarqube vscode extension | |
| # Required for sonarqube vscode extension | |
| # Use OpenJDK 21 (latest LTS) instead of 17 to ensure compatibility with modern Java tools and libraries. |
| && UV_INSTALL_URL=$([ "${UV_VER}" = "latest" ] \ | ||
| && echo "https://astral.sh/uv/install.sh" || \ | ||
| echo "https://astral.sh/uv/${UV_VER}/install.sh") \ | ||
| && curl -LsSf "${UV_INSTALL_URL}" | env UV_INSTALL_DIR="/usr/local/bin" sh |
There was a problem hiding this comment.
Installing uv by piping curl output from astral.sh directly into sh executes remote code as root during the image build without any integrity verification. If the download endpoint, its DNS, or TLS is compromised, an attacker could run arbitrary commands in the build environment and backdoor the resulting uv binary used in CI. Instead, download the installer or binary to disk and verify it against a pinned checksum or signature (or use a package manager/pinned image) before executing it.
| RUN curl -sfL "https://raw.githubusercontent.com/reviewdog/reviewdog/fd59714416d6d9a1c0692d872e38e7f8448df4fc/install.sh" \ | ||
| | sh -s -- -b /usr/local/bin \ |
There was a problem hiding this comment.
reviewdog is installed by piping a remote install.sh script from GitHub directly into sh, which again runs unverified third-party code as root during the image build. A compromise of the GitHub repository, raw content delivery, or its dependencies could inject malicious commands or ship a trojanized reviewdog binary that later runs in CI with access to repository data and tokens. Prefer downloading a specific, pinned release asset and verifying its checksum (or vendoring the installer script in this repo) instead of executing it directly from the network.
User description
Also removed
pulumi, it should be installed only in project that need itPR Type
Enhancement
Description
Switch base image from Ubuntu 25.10 to Debian 13 for stability
Introduce build arguments for tool version management
Use multi-stage builds to copy pre-built binaries from official images
Remove Pulumi installation, making it project-specific
Upgrade Java from OpenJDK 17 to 21
Improve package management with testing repo pinning
Diagram Walkthrough
File Walkthrough
Dockerfile
Debian base image and multi-stage tool installationDockerfile
shellcheck, shfmt, uv, reviewdog, and snyk
Docker images
unavailable packages
checksum verification
caching
specs.yaml
Remove Pulumi installation testtests/specs.yaml