v6.0.0 — Bump all dependencies, SQLite, and modernise CI#1857
v6.0.0 — Bump all dependencies, SQLite, and modernise CI#1857jonatansberg merged 3 commits intomasterfrom
Conversation
WalkthroughThis PR releases v6.0.0 and raises the Node engine requirement to >=20.17.0. It updates CI matrices and GitHub Actions versions (including buildx/QEMU), changes artifact-upload conditions to target Node 24, and adjusts Docker build images to Node 24/bookworm. Bundled SQLite is bumped to 3.52.0; deps and devDeps updated (notably node-addon-api, prebuild-install, tar, prebuild, node-gyp). yarn.lock is now tracked. deps/extract.js adds a post-extract cleanup to remove SQLite's VERSION file on extraction. README and prebuilt target listings were updated accordingly. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🧹 Nitpick comments (1)
tools/BinaryBuilder.Dockerfile (1)
1-23: Static analysis note: Container runs as root.Trivy flags that no non-root
USERis specified. This is acceptable for a build-only container that runsnpm install,prebuild, and tests in CI—root access is often needed for package installation. No action required unless this image is ever used in production contexts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/BinaryBuilder.Dockerfile` around lines 1 - 23, Trivy warns the container runs as root; add an explicit non-root USER or document that this image is build-only—either create a dedicated unprivileged user (e.g., create user, chown WORKDIR, then add USER <name>) after RUN npm run test, or add a clear comment near the top (around the FROM/WORKDIR/CMD lines) stating this is a CI-only build image and not intended for production, so reviewers know the root usage is intentional; reference the Dockerfile directives RUN npm install --ignore-scripts, RUN npm run prebuild, RUN npm run test, and CMD ["sh"] to decide where to place the USER change or comment.
🤖 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/ci.yml:
- Around line 121-127: The matrix is inconsistent: the main list uses variant
alpine3.20 but the include entry for the musl x64 build still specifies variant:
alpine3.15; update the include entry (the block containing target: linux/amd64
and variant: alpine3.15) to use variant: alpine3.20 so all Alpine/musl builds
use the same version (unless alpine3.15 was intentionally required, in which
case add a comment explaining that exception).
---
Nitpick comments:
In `@tools/BinaryBuilder.Dockerfile`:
- Around line 1-23: Trivy warns the container runs as root; add an explicit
non-root USER or document that this image is build-only—either create a
dedicated unprivileged user (e.g., create user, chown WORKDIR, then add USER
<name>) after RUN npm run test, or add a clear comment near the top (around the
FROM/WORKDIR/CMD lines) stating this is a CI-only build image and not intended
for production, so reviewers know the root usage is intentional; reference the
Dockerfile directives RUN npm install --ignore-scripts, RUN npm run prebuild,
RUN npm run test, and CMD ["sh"] to decide where to place the USER change or
comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb7f93ea-b446-4555-a232-990d6c2d4032
⛔ Files ignored due to path filters (3)
deps/sqlite-autoconf-3450000.tar.gzis excluded by!**/*.gzdeps/sqlite-autoconf-3520000.tar.gzis excluded by!**/*.gzyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
.github/workflows/ci.yml.gitignoreREADME.mddeps/common-sqlite.gypideps/extract.jspackage.jsontools/BinaryBuilder.Dockerfiletools/semver-check.js
💤 Files with no reviewable changes (1)
- .gitignore
🤖 Velo CI Failure AnalysisClassification: 🟠 SOFT FAIL
|
jonatansberg
force-pushed
the
v6.0.0
branch
3 times, most recently
from
179599b to
af49903
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
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)
tools/BinaryBuilder.Dockerfile (1)
4-22:⚠️ Potential issue | 🟠 MajorRun npm build/test steps as a non-root user.
Line 13 onward executes project scripts as root because no
USERis set. This unnecessarily increases risk in CI/containerized builds.🔒 Proposed hardening
WORKDIR /usr/src/build -COPY . . +COPY --chown=node:node . . +USER node RUN npm install --ignore-scripts ENV CFLAGS="${CFLAGS:-} -include ../src/gcc-preinclude.h" ENV CXXFLAGS="${CXXFLAGS:-} -include ../src/gcc-preinclude.h" RUN npm run prebuild🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/BinaryBuilder.Dockerfile` around lines 4 - 22, The Dockerfile runs npm scripts as root (see RUN npm run prebuild and RUN npm run test) — create a non-root build user, chown the WORKDIR (/usr/src/build) and copied files (COPY . .), switch to that user with USER before running npm install/prebuild/test, and ensure any required packages or permissions are granted (e.g., add a group/user via adduser/addgroup or useradd and use chown -R to transfer ownership) so build and test steps run non-root while keeping existing ENV CFLAGS/CXXFLAGS logic intact.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
23-23: Consider pinning the macOS image instead ofmacos-latest.GitHub currently maps
macos-latestto an arm64 runner, and GitHub notes that-latestlabels can change as images migrate. For release-binary CI, pinning to a concrete image once the toolchain is validated would make these jobs more reproducible. (docs.github.com)Also applies to: 39-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 23, Replace any uses of the floating runner label "macos-latest" with a concrete macOS image (for example "macos-13" or "macos-14" once you've validated the toolchain) to make release-binary CI reproducible; update every job that currently lists "macos-latest" (including the other occurrences noted around lines 39-50) to the chosen pinned image and ensure any matrix entries or job definitions using "macos-latest" are changed consistently.
🤖 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/ci.yml:
- Around line 157-162: The artifact name in the GitHub Actions step using
actions/upload-artifact@v7 currently interpolates matrix.target (name:
prebuilt-binaries-${{ matrix.variant }}-${{ matrix.target }}) which can contain
slashes; update that interpolation to sanitize matrix.target by replacing
forward slashes with a safe character (e.g., '-' or '_') before composing the
artifact name so the upload step accepts it; modify the step that defines name:
prebuilt-binaries-${{ matrix.variant }}-${{ matrix.target }} to use a sanitized
expression of matrix.target (replace '/' with '-' or '_') when building the
artifact name.
---
Outside diff comments:
In `@tools/BinaryBuilder.Dockerfile`:
- Around line 4-22: The Dockerfile runs npm scripts as root (see RUN npm run
prebuild and RUN npm run test) — create a non-root build user, chown the WORKDIR
(/usr/src/build) and copied files (COPY . .), switch to that user with USER
before running npm install/prebuild/test, and ensure any required packages or
permissions are granted (e.g., add a group/user via adduser/addgroup or useradd
and use chown -R to transfer ownership) so build and test steps run non-root
while keeping existing ENV CFLAGS/CXXFLAGS logic intact.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 23: Replace any uses of the floating runner label "macos-latest" with a
concrete macOS image (for example "macos-13" or "macos-14" once you've validated
the toolchain) to make release-binary CI reproducible; update every job that
currently lists "macos-latest" (including the other occurrences noted around
lines 39-50) to the chosen pinned image and ensure any matrix entries or job
definitions using "macos-latest" are changed consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42849b7d-d383-4574-8a93-ff9ebb136a13
⛔ Files ignored due to path filters (3)
deps/sqlite-autoconf-3450000.tar.gzis excluded by!**/*.gzdeps/sqlite-autoconf-3520000.tar.gzis excluded by!**/*.gzyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
.github/workflows/ci.yml.gitignoreREADME.mddeps/common-sqlite.gypideps/extract.jspackage.jsontools/BinaryBuilder.Dockerfiletools/semver-check.js
💤 Files with no reviewable changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- deps/extract.js
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af49903da0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fixes #1824 — prebuild-install failing on Node 22.14.0 due to N-API version string comparison bug in napi-build-utils < 2.0.0. Dependencies: - prebuild-install: ^7.1.1 -> ^7.1.3 (fixes N-API version detection) - prebuild: 12.1.0 -> 13.0.1 - tar: ^6.1.11 -> ^7.5.10 (fixes 6 HIGH CVEs) - node-addon-api: ^7.0.0 -> ^8.0.0 - node-gyp: 8.x -> 12.x (peer + optional) - Added engines field: node >=20.17.0 - Added yarn.lock CI: - Test matrix: Node 20, 22, 24 (all current LTS lines) - ubuntu-20.04 -> ubuntu-24.04, alpine3.15 -> alpine3.20 - Replaced macos-m1 self-hosted with macos-latest (now ARM64) - Dropped win32-ia32 (Node 24 no longer ships 32-bit Windows) - Excluded macos x64 + Node 20 (Rosetta 2 async hooks bug) - Removed pip install setuptools workaround - Updated actions (checkout v6, setup-node v6, upload-artifact v7, setup-qemu-action v4, setup-buildx-action v4) - Sanitised artifact names to avoid slashes from platform targets - Dockerfile: Node 18 -> 24, bullseye -> bookworm
Two years of upstream improvements including bug fixes, performance enhancements, and security hardening. The extract script now removes the VERSION file that SQLite >= 3.49 ships, which conflicts with the C++20 <version> header on case-insensitive filesystems (macOS/Windows).
2 participants
Summary
Fixes #1824 —
prebuild-installfailing on Node 22.14.0 due to N-API version string comparison bug.This is a major version bump because the minimum supported Node.js version is raised from v10 to v20.17.0 (all current LTS lines: 20, 22, 24).
NPM dependency bumps
prebuild-install: ^7.1.1 → ^7.1.3 (fixes N-API version detection vianapi-build-utils2.0.0)prebuild: 12.1.0 → 13.0.1tar: ^6.1.11 → ^7.5.10 (fixes 6 HIGH CVEs)node-addon-api: ^7.0.0 → ^8.0.0node-gyp: 8.x → 12.x (peer + optional)engines.node: ">=20.17.0"yarn.lock(removed from.gitignore)SQLite bump
VERSIONfile to avoid C++20<version>header conflict on case-insensitive filesystems (macOS/Windows)CI modernisation
macos-m1self-hosted runner withmacos-latest(now ARM64 by default)win32-ia32builds (Node 24 no longer ships 32-bit Windows)pip install setuptoolsworkaround (node-gyp 12 handles modern Python)README
win32-ia32from supported targetsTest plan
node tools/semver-check.jspassesnode-gyp rebuildsucceeds