Skip to content

fix: install latest npm version for node 22 #348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 22, 2025
Merged

fix: install latest npm version for node 22 #348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d95e346
fix: install latest npm version for node 22
beevelop Aug 22, 2025
d484b1d
fix: docker build configuration
beevelop Aug 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/docker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ on:
schedule:
- cron: "0 10 * * *" # everyday at 10am
workflow_dispatch:
pull_request_target:
pull_request:
branches: ["latest"]
Comment on lines +7 to 8
Copy link

Choose a reason for hiding this comment

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

💡 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.

Suggested change
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.

push:
branches: ["latest"]
Expand All @@ -20,6 +20,8 @@ jobs:
steps:
- name: Check out the repo
uses: actions/checkout@v5
with:
ref: ${{ github.head_ref || github.ref }}
- name: Set imageName based on the repository name
Comment on lines 21 to 25
Copy link

Choose a reason for hiding this comment

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

💡 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: 0

You 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"
fi

Length 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: 0

This 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.

id: step_one
run: |
Expand All @@ -46,5 +48,6 @@ jobs:
with:
platforms: ${{ env.platforms }}
push: ${{ github.event_name != 'pull_request' }}
pull: true
tags: ${{ steps.docker_meta.outputs.tags }}
labels: ${{ steps.docker_meta.outputs.labels }}
9 changes: 5 additions & 4 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ LABEL maintainer="[email protected]" \
org.opencontainers.image.documentation="https://github.com/beevelop/docker-android-nodejs/blob/latest/README.md" \
org.opencontainers.image.source="https://github.com/beevelop/docker-android-nodejs.git"

# Install Node.js using NodeSource repository
RUN apt-get update && apt-get install -y curl ca-certificates && \
curl -fsSL https://deb.nodesource.com/setup_lts.x | bash - && \
apt-get install -y nodejs && \
# 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 && \
Copy link

Choose a reason for hiding this comment

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

🛠️ 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.

Suggested change
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.

npm install -g npm@latest && \
npm install -g yarn && \
apt-get clean && \
rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* && \
Expand Down