fix: install latest npm version for node 22 #348
Conversation
WalkthroughSwitched Dockerfile Node.js installation from NodeSource/apt to extracting official Node tar.xz binaries (ENV NODE_VERSION=22.18.0), added xz-utils, updated npm, installed yarn, added cleanup and version checks. Workflow changed pull_request_target → pull_request, ensured checkout of PR ref, and enabled image pull in docker build step. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / PR
participant GH as GitHub Actions
participant Checkout as checkout action
participant Builder as docker/build-push-action
participant Image as Docker build context
Dev->>GH: open PR (pull_request)
GH->>Checkout: checkout ref (${{ github.head_ref || github.ref }})
Checkout-->>GH: repo at PR ref
GH->>Builder: build image (pull: true)
Builder->>Image: RUN steps (apt update -> install xz-utils -> download node tar.xz -> tar -xJ --strip-components=1 -> npm -g npm@latest -> npm -g yarn -> cleanup)
Image-->>Builder: build artifact
Builder-->>GH: push/result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
23-30: Add checksums and robust shell flags to harden the supply chain and fail-fast on errors.Pipe-to-tar without verification is risky and can mask transient failures. Add set -euxo pipefail and verify the tarball against SHASUMS256.txt before extracting.
Apply this diff:
-RUN apt-get update && apt-get install -y curl ca-certificates xz-utils && \ - curl -fsSL https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-x64.tar.xz | tar -xJ -C /usr/local --strip-components=1 && \ - npm install -g npm@latest && \ - npm install -g yarn && \ - apt-get clean && \ - rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* && \ - node -v && npm -v && yarn -v && \ - mvn -v && gradle -v && java -version && ant -version +ARG TARGETARCH +RUN set -euxo pipefail; \ + apt-get update; \ + apt-get install -y --no-install-recommends curl ca-certificates xz-utils; \ + arch="${TARGETARCH:-$(uname -m)}"; \ + case "$arch" in \ + x86_64|amd64) node_arch=x64 ;; \ + aarch64|arm64) node_arch=arm64 ;; \ + armv7l|armv7|armhf) node_arch=armv7l ;; \ + *) echo "Unsupported architecture: $arch" >&2; exit 1 ;; \ + esac; \ + cd /tmp; \ + curl -fsSLO "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${node_arch}.tar.xz"; \ + curl -fsSLO "https://nodejs.org/dist/v${NODE_VERSION}/SHASUMS256.txt"; \ + grep " node-v${NODE_VERSION}-linux-${node_arch}.tar.xz" SHASUMS256.txt | sha256sum -c -; \ + tar -xJf "node-v${NODE_VERSION}-linux-${node_arch}.tar.xz" -C /usr/local --strip-components=1; \ + rm -f "node-v${NODE_VERSION}-linux-${node_arch}.tar.xz" "SHASUMS256.txt"; \ + npm install -g npm@latest; \ + npm install -g yarn; \ + apt-get purge -y --auto-remove xz-utils; \ + rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*
🧹 Nitpick comments (4)
Dockerfile (4)
23-23: Use --no-install-recommends and purge build-only deps to shrink the image.xz-utils is only needed for extraction; purge it afterward. You can also drop apt-get clean (redundant when lists are removed).
The diff above already applies these optimizations.
25-26: Consider installing Yarn via Corepack instead of npm (optional).Node 22 ships with Corepack, which manages Yarn and pnpm versions reproducibly. Installing Yarn with npm gives Yarn 1.x (classic) and is not pinned.
If you prefer Yarn Berry via Corepack, replace the Yarn line:
- npm install -g yarn && \ + corepack enable && corepack prepare yarn@stable --activate && \Please confirm whether this image is expected to provide Yarn 1.x (classic) for backwards compatibility, or if moving to Yarn Berry is acceptable for your user base.
29-30: Drop version-print commands in the build layer to reduce build time and noise.These checks don’t persist artifacts and can slow CI. Prefer a separate CI step or keep just node -v && npm -v if you need a smoke check.
Apply this diff to remove them:
- node -v && npm -v && yarn -v && \ - mvn -v && gradle -v && java -version && ant -version
22-22: Parameterize npm version (optional) for reproducible rebuilds.Using npm@latest is fine for “always latest” but is non-reproducible across rebuilds on different days.
Example:
-ENV NODE_VERSION=22.18.0 +ENV NODE_VERSION=22.18.0 +ARG NPM_VERSION=latestand
- npm install -g npm@latest; \ + npm install -g "npm@${NPM_VERSION}"; \
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Dockerfile(1 hunks)
🔇 Additional comments (2)
Dockerfile (2)
21-26: Good move: pin Node 22 and upgrade npm to latest.Switching to official Node.js binaries and explicitly upgrading npm satisfies the PR goal (“install latest npm version for Node 22”) and makes the toolchain more predictable at build time.
22-24: Node.js artifacts availability confirmed
The verification script succeeded in fetching both the Node.js v22.18.0 binary and its SHASUMS256 checksum, confirming the Dockerfile’s download URL is valid.
| # Install Node.js 22 with compatible npm using official Node.js binaries | ||
| ENV NODE_VERSION=22.18.0 | ||
| RUN apt-get update && apt-get install -y curl ca-certificates xz-utils && \ | ||
| curl -fsSL https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-x64.tar.xz | tar -xJ -C /usr/local --strip-components=1 && \ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hardcoded linux-x64 will break non-amd64 (e.g., arm64) builds — derive arch via TARGETARCH/uname and use it in the URL.
This Dockerfile will fail or produce the wrong binary on arm64 or other platforms. Make it multi-arch friendly by mapping the docker buildx-provided TARGETARCH (or uname -m fallback) to Node’s archive naming.
Apply this diff:
- curl -fsSL https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-x64.tar.xz | tar -xJ -C /usr/local --strip-components=1 && \
+ arch="${TARGETARCH:-$(uname -m)}"; \
+ case "$arch" in \
+ x86_64|amd64) node_arch=x64 ;; \
+ aarch64|arm64) node_arch=arm64 ;; \
+ armv7l|armv7|armhf) node_arch=armv7l ;; \
+ *) echo "Unsupported architecture: $arch" >&2; exit 1 ;; \
+ esac; \
+ curl -fsSL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${node_arch}.tar.xz" \
+ | tar -xJ -C /usr/local --strip-components=1 && \📝 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.
| curl -fsSL https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-x64.tar.xz | tar -xJ -C /usr/local --strip-components=1 && \ | |
| arch="${TARGETARCH:-$(uname -m)}"; \ | |
| case "$arch" in \ | |
| x86_64|amd64) node_arch=x64 ;; \ | |
| aarch64|arm64) node_arch=arm64 ;; \ | |
| armv7l|armv7|armhf) node_arch=armv7l ;; \ | |
| *) echo "Unsupported architecture: $arch" >&2; exit 1 ;; \ | |
| esac; \ | |
| curl -fsSL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-${node_arch}.tar.xz" \ | |
| | tar -xJ -C /usr/local --strip-components=1 && \ |
🤖 Prompt for AI Agents
In Dockerfile around line 24, the curl URL hardcodes "linux-x64" which breaks
non-amd64 builds; change to derive arch from build ARG TARGETARCH (fallback to
uname -m), map common values (amd64 -> x64, arm64 -> arm64, arm -> armv7l, etc.)
into a NODE_ARCH variable, export NODE_ARCH and use it in the download URL
instead of linux-x64 so the correct Node archive is fetched for the target
architecture.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docker.yml (1)
41-45: Gate Docker Hub login so PRs from forks don’t fail due to missing secrets.
docker/login-actionrequiresusernameandpassword. Onpull_requestfrom forks, these secrets aren’t available and the step will error. Skip the login when not pushing.- - name: Login to DockerHub - uses: docker/login-action@v3 - with: - username: ${{ secrets.DOCKERHUB_USERNAME }} - password: ${{ secrets.DOCKERHUB_TOKEN }} + - name: Login to DockerHub + if: ${{ github.event_name != 'pull_request' }} + uses: docker/login-action@v3 + with: + username: ${{ secrets.DOCKERHUB_USERNAME }} + password: ${{ secrets.DOCKERHUB_TOKEN }}Optional hardening: also add
if: ${{ github.event_name != 'pull_request' }}to the “Build and push” step (even thoughpush:is already conditional) to prevent accidental registry interaction during PRs.
🧹 Nitpick comments (1)
.github/workflows/docker.yml (1)
51-51: Enabling pull is a solid choice to refresh base images before build.This helps ensure your scheduled and push builds don’t use stale bases. Consider adding BuildKit cache to speed up repeat builds (optional):
uses: docker/build-push-action@v6 with: platforms: ${{ env.platforms }} push: ${{ github.event_name != 'pull_request' }} pull: true + cache-from: type=gha + cache-to: type=gha,mode=max tags: ${{ steps.docker_meta.outputs.tags }} labels: ${{ steps.docker_meta.outputs.labels }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/docker.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: main
| pull_request: | ||
| branches: ["latest"] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good move: pull_request is safer than pull_request_target, but gate secret-using steps to avoid PR failures.
Switching to pull_request reduces token exposure from forks. However, secrets are not available on pull_request from forks and same-repo PRs may still not need them. Make sure steps that require secrets (e.g., Docker Hub login) are skipped on PRs; otherwise the workflow can fail on forks due to missing inputs.
I’ve added a separate comment with a targeted diff for the Docker Hub login step to gate it properly.
🏁 Script executed:
#!/usr/bin/env bash
set -e
echo "=== Displaying .github/workflows/docker.yml ==="
sed -n '1,200p' .github/workflows/docker.yml || echo "File not found or path incorrect"Length of output: 1641
Gate DockerHub login step to non-PR events
The DockerHub login currently runs on every pull_request, but GitHub does not expose secrets for forked PRs, causing the workflow to fail. Add an if condition to skip the login when the event is pull_request.
• File: .github/workflows/docker.yml
Location: around the “Login to DockerHub” step in jobs.main.steps
Suggested diff:
- name: Login to DockerHub
+ if: github.event_name != 'pull_request'
uses: docker/login-action@v3
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pull_request: | |
| branches: ["latest"] | |
| - name: Login to DockerHub | |
| if: github.event_name != 'pull_request' | |
| uses: docker/login-action@v3 | |
| with: | |
| username: ${{ secrets.DOCKERHUB_USERNAME }} | |
| password: ${{ secrets.DOCKERHUB_TOKEN }} |
🤖 Prompt for AI Agents
.github/workflows/docker.yml around lines 7-8: the "Login to DockerHub" step
currently runs for pull_request events which fails for forked PRs because
secrets are not available; update that step to include an if condition that
skips execution when github.event_name == 'pull_request' (e.g., if:
github.event_name != 'pull_request') so the login only runs on non-PR events
where secrets are available, leaving the rest of the job unchanged.
| - name: Check out the repo | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| ref: ${{ github.head_ref || github.ref }} | ||
| - name: Set imageName based on the repository name |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix checkout for forked PRs (current ref override can break on forks).
Using ref: ${{ github.head_ref || github.ref }} without also setting repository: will try to fetch the head branch from the base repo, which fails for forked PRs. Either rely on the default behavior of actions/checkout for PRs or explicitly set repository and ref in a fork-safe way.
Apply this diff to split checkout into two steps and make it fork-safe while also fetching full history (helps tagging/metadata), and avoiding credentials on PRs:
- - name: Check out the repo
- uses: actions/checkout@v5
- with:
- ref: ${{ github.head_ref || github.ref }}
+ - name: Check out PR (fork-safe)
+ if: ${{ github.event_name == 'pull_request' }}
+ uses: actions/checkout@v5
+ with:
+ repository: ${{ github.event.pull_request.head.repo.full_name }}
+ ref: ${{ github.event.pull_request.head.ref }}
+ fetch-depth: 0
+ persist-credentials: false
+ - name: Check out repo (non-PR)
+ if: ${{ github.event_name != 'pull_request' }}
+ uses: actions/checkout@v5
+ with:
+ ref: ${{ github.ref }}
+ fetch-depth: 0You can validate this by opening a test PR from a fork and confirming the checkout step resolves the fork’s head repository/branch and proceeds to build.
🏁 Script executed:
#!/usr/bin/env bash
set -e
echo "=== First 100 lines of .github/workflows/docker.yml ==="
if [ -f .github/workflows/docker.yml ]; then
sed -n '1,100p' .github/workflows/docker.yml
else
echo "File .github/workflows/docker.yml not found"
fiLength of output: 1699
Ensure fork-safe checkout in Pull Request workflows
Using actions/checkout@v5 with only
with:
ref: ${{ github.head_ref || github.ref }}will default to checking out the specified branch from the base repository (${{ github.repository }}), not the fork. In a forked PR, the head branch doesn’t exist on the base repo, causing the checkout to fail (github.com).
Split the checkout into two conditional steps—one for PRs (explicitly pointing to the fork’s repo and branch, with full history and no credentials) and one for non-PRs—to guarantee correct behavior on forks:
• Check out PR head (fork-safe)
- name: Check out PR (fork-safe)
if: ${{ github.event_name == 'pull_request' }}
uses: actions/checkout@v5
with:
repository: ${{ github.event.pull_request.head.repo.full_name }}
ref: ${{ github.event.pull_request.head.ref }}
fetch-depth: 0
persist-credentials: false• Check out default ref on non-PR events
- name: Check out repo (non-PR)
if: ${{ github.event_name != 'pull_request' }}
uses: actions/checkout@v5
with:
ref: ${{ github.ref }}
fetch-depth: 0This ensures that on a forked pull request you fetch from the correct repository and branch, and for pushes/tags you continue using the workflow’s default behavior (stackoverflow.com).
🤖 Prompt for AI Agents
.github/workflows/docker.yml lines 21-25: the current single checkout step using
ref: ${{ github.head_ref || github.ref }} is not fork-safe and can fail for PRs
from forks; replace it with two conditional checkout steps: one for pull_request
events that uses repository: ${{ github.event.pull_request.head.repo.full_name
}}, ref: ${{ github.event.pull_request.head.ref }}, fetch-depth: 0 and
persist-credentials: false, and one for non-pull_request events that uses ref:
${{ github.ref }} and fetch-depth: 0, ensuring the PR head is checked out from
the fork while preserving correct behavior for pushes/tags.
Summary by CodeRabbit