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).
|
Thanks for doing this. I guess my question is: before I install this, are there any plans to remove the deprecation notice? Is there any process change that will get provide any support for this release? Trying to figure out whether or not to move to another sqlite library or accept the dependabot PR that upgrades my release. |
|
@kberg we don't have the bandwidth to actively maintain this library on our end. Ghost moved away from sqlite a while ago, and our current usage is mostly contained to legacy tests in CI. Unless someone else wants to step up and maintain this library I'd still consider moving to a different sqlite client. Modern Node versions have a built-in sqlite client, and if you rely on the async nature of this client, then I'd consider libsql/turso. |
|
Thanks for your thorough response. |
3 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