Skip to content

src: support building with the V8 sandbox - #62237

Closed
codebytere wants to merge 2 commits into
nodejs:mainfrom
codebytere:sandbox-tweak
Closed

src: support building with the V8 sandbox#62237
codebytere wants to merge 2 commits into
nodejs:mainfrom
codebytere:sandbox-tweak

Conversation

@codebytere

@codebytere codebytere commented Mar 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

With V8_ENABLE_SANDBOX every ArrayBuffer backing store has to be allocated inside the sandbox, so memory that Node.js or a library allocated itself can't be wrapped and has to be copied in. Several places already special-case this, each with its own #ifdef, but a sandbox build still doesn't compile (kMaxSafeBufferSizeForSandbox), aborts in crypto.randomUUID() (SecureBuffer is UNREACHABLE()), throws from every caller of the malloc-owning Buffer::New() (v8.serialize(), string transcoding, IPC, the public Buffer::New(isolate, data, length)), and trace_events, SEA assets and FFI still wrap outside memory unconditionally.

Reworked since the first round to go through the isolate's allocator: AdoptIntoBackingStore() wraps the memory as before in regular builds and, with the sandbox, copies it into a store from ArrayBuffer::NewBackingStore(isolate, ...) and runs the deleter right away. All of the sites above use it, so there is one #ifdef. Two things can't be copied since sharing the memory is the point: the trace category flag, where lib now falls back to isTraceCategoryEnabled(), and FFI's copy: false views, which throw ERR_OPERATION_FAILED. The second commit turns the sandbox on for --experimental-pointer-compression-shared-cage, matching V8's own default, so the configuration is reachable from configure.

Tests: cctest and the default, ffi, sqlite and embedding suites pass in a shared-cage sandbox build (on top of #65464, which that configuration needs to compile) with the same results as a regular build of this branch; a few tests that wrapped stack or foreign memory for convenience are adjusted, and common.hasV8Sandbox gates the FFI zero-copy and --secure-heap tests.

Disclosure: the code and this description were written by Claude Code, directed and reviewed by @codebytere.

@codebytere
codebytere requested a review from jasnell March 13, 2026 10:45
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Mar 13, 2026
@codecov

codecov Bot commented Mar 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.62637% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.23%. Comparing base (b0d56c7) to head (5e89763).
⚠️ Report is 30 commits behind head on main.

Files with missing lines Patch % Lines
src/node_buffer.cc 66.66% 4 Missing and 5 partials ⚠️
lib/internal/trace_events.js 82.35% 3 Missing ⚠️
src/crypto/crypto_dh.cc 25.00% 2 Missing and 1 partial ⚠️
src/crypto/crypto_util.cc 66.66% 1 Missing and 2 partials ⚠️
src/crypto/crypto_x509.cc 25.00% 2 Missing and 1 partial ⚠️
src/ffi/data.cc 81.81% 0 Missing and 2 partials ⚠️
src/node_sea.cc 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #62237      +/-   ##
==========================================
+ Coverage   90.19%   90.23%   +0.03%     
==========================================
  Files         770      770              
  Lines      264410   264464      +54     
  Branches    50243    50279      +36     
==========================================
+ Hits       238490   238641     +151     
+ Misses      16924    16841      -83     
+ Partials     8996     8982      -14
Files with missing lines Coverage Δ
lib/ffi.js 96.07% <100.00%> (-0.02%) ⬇️
lib/internal/http.js 92.22% <100.00%> (-0.11%) ⬇️
lib/internal/util/debuglog.js 95.32% <100.00%> (ø)
src/node_buffer.h 100.00% <ø> (ø)
src/node_sqlite.cc 82.14% <100.00%> (-0.07%) ⬇️
src/node_trace_events.cc 82.30% <ø> (ø)
src/util.cc 87.63% <100.00%> (+0.05%) ⬆️
src/util.h 90.66% <ø> (ø)
src/node_sea.cc 89.66% <66.66%> (-0.12%) ⬇️
src/ffi/data.cc 75.59% <81.81%> (-0.12%) ⬇️
... and 5 more

... and 38 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

addaleax
addaleax reviewed Mar 15, 2026
View reviewed changes

@addaleax addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reading through the diff ... why doesn't the version of ArrayBuffer::NewBackingStore() that takes a size argument use the default allocator if that's a requirement anyway? Wouldn't it be a bug in V8 if that variant of ArrayBuffer::NewBackingStore() is effectively unusable?

If we indeed need to manually reach out to the default allocator, it would probably be better to create a helper that covers all of these cases and handles the V8_ENABLE_SANDBOX switching in one location instead of those being spread out throughout the codebase

addaleax
addaleax previously requested changes Mar 16, 2026
View reviewed changes

@addaleax addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This definitely fixes issues building Node.js with sandbox mode enabled, but it's still a bit unclear how this interacts with fb21829 and why specifically ArrayBuffer::NewBackingStore(isolate, size) doesn't return a backing store that is valid for the given isolate

Comment thread src/node_buffer.cc Outdated
Comment thread src/api/environment.cc
Comment thread src/node_serdes.cc Outdated
@addaleax
addaleax dismissed their stale review March 16, 2026 15:27

Memory leak is only real in specific scenarios

legendecas
legendecas reviewed Mar 17, 2026
View reviewed changes
Comment thread src/node_trace_events.cc Outdated
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

@github-actions github-actions Bot added the stale Issues and PRs marked stale due to inactivity and scheduled for automatic closure. label Jul 28, 2026
@codebytere codebytere changed the title src: support V8 sandbox memory cage in allocators src: support building with the V8 sandbox Aug 21, 2026
@codebytere codebytere removed the stale Issues and PRs marked stale due to inactivity and scheduled for automatic closure. label Aug 21, 2026
@codebytere

codebytere commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

@addaleax @legendecas rebased and reworked - everything goes through ArrayBuffer::NewBackingStore(isolate, …) in one helper now, as suggested, and the description is rewritten to match. #65464 is split out since shared-cage builds currently don't compile at all.

legendecas
legendecas reviewed Aug 28, 2026
View reviewed changes
Comment thread src/crypto/crypto_util.cc
legendecas
legendecas reviewed Aug 28, 2026
View reviewed changes
Comment thread src/node_buffer.cc Outdated
legendecas
legendecas reviewed Aug 28, 2026
View reviewed changes

@legendecas legendecas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for fixing up! LGTM with minor comments

@nodejs-github-bot

nodejs-github-bot commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/76764/

legendecas
legendecas reviewed Aug 31, 2026
View reviewed changes
Comment thread src/node_buffer.cc
@codebytere
codebytere force-pushed the sandbox-tweak branch 2 times, most recently from 3325452 to 895bda3 Compare August 31, 2026 23:56
@nodejs nodejs deleted a comment from nodejs-github-bot Sep 5, 2026
@nodejs nodejs deleted a comment from nodejs-github-bot Sep 5, 2026
@codebytere
src: support building with the V8 sandbox
abb6a1d 
With V8_ENABLE_SANDBOX every ArrayBuffer backing store has to be
allocated inside the sandbox, so memory that Node.js or a library
allocated itself cannot be wrapped and has to be copied in. Several
places already special-cased this, each with its own #ifdef, but a
build with the sandbox enabled still failed to compile
(`kMaxSafeBufferSizeForSandbox`), aborted in `crypto.randomUUID()`
(`SecureBuffer` was UNREACHABLE) and threw from every caller of the
malloc-owning `Buffer::New()` (v8.serialize, string transcoding, IPC,
the public `Buffer::New(isolate, data, length)`), and trace_events,
SEA assets and FFI still wrapped outside memory unconditionally.

Add `AdoptIntoBackingStore()`, which wraps the memory as before in
regular builds and copies it into an isolate-allocated backing store,
running the deleter right away, when the sandbox is enabled, and use it
at all of those sites so the #ifdef lives in one place. The trace
category flag cannot be copied since JS polls it, so under the sandbox
`getCategoryEnabledBuffer()` returns nothing and lib falls back to
`isTraceCategoryEnabled()`; FFI zero-copy views throw
ERR_OPERATION_FAILED there for the same reason.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
@codebytere
build: enable the V8 sandbox in shared-cage builds
5e89763 
V8 defaults `v8_enable_sandbox` to on whenever the shared pointer
compression cage and the external code space are enabled, and that is
the configuration embedders that use the sandbox build with. Now that
the sandbox builds and passes the tests, follow that default for
`--experimental-pointer-compression-shared-cage` so the configuration
is reachable from `configure`. Multi-cage pointer compression builds
stay without it: there every IsolateGroup gets its own sandbox, and
`NodeArrayBufferAllocator` always allocates from the default one.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
@nodejs-github-bot

nodejs-github-bot commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/77071/

@nodejs-github-bot

nodejs-github-bot commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/77094/

@codebytere codebytere added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 7, 2026
codebytere added a commit that referenced this pull request Sep 7, 2026
@codebytere
src: support building with the V8 sandbox
48c90ff 
With V8_ENABLE_SANDBOX every ArrayBuffer backing store has to be
allocated inside the sandbox, so memory that Node.js or a library
allocated itself cannot be wrapped and has to be copied in. Several
places already special-cased this, each with its own #ifdef, but a
build with the sandbox enabled still failed to compile
(`kMaxSafeBufferSizeForSandbox`), aborted in `crypto.randomUUID()`
(`SecureBuffer` was UNREACHABLE) and threw from every caller of the
malloc-owning `Buffer::New()` (v8.serialize, string transcoding, IPC,
the public `Buffer::New(isolate, data, length)`), and trace_events,
SEA assets and FFI still wrapped outside memory unconditionally.

Add `AdoptIntoBackingStore()`, which wraps the memory as before in
regular builds and copies it into an isolate-allocated backing store,
running the deleter right away, when the sandbox is enabled, and use it
at all of those sites so the #ifdef lives in one place. The trace
category flag cannot be copied since JS polls it, so under the sandbox
`getCategoryEnabledBuffer()` returns nothing and lib falls back to
`isTraceCategoryEnabled()`; FFI zero-copy views throw
ERR_OPERATION_FAILED there for the same reason.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
codebytere added a commit that referenced this pull request Sep 7, 2026
@codebytere
build: enable the V8 sandbox in shared-cage builds
c7d80a2 
V8 defaults `v8_enable_sandbox` to on whenever the shared pointer
compression cage and the external code space are enabled, and that is
the configuration embedders that use the sandbox build with. Now that
the sandbox builds and passes the tests, follow that default for
`--experimental-pointer-compression-shared-cage` so the configuration
is reachable from `configure`. Multi-cage pointer compression builds
stay without it: there every IsolateGroup gets its own sandbox, and
`NodeArrayBufferAllocator` always allocates from the default one.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@codebytere

codebytere commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Landed in 695f01f...c7d80a2

@codebytere codebytere closed this Sep 7, 2026
aduh95 pushed a commit that referenced this pull request Sep 7, 2026
@codebytere @aduh95
build: enable the V8 sandbox in shared-cage builds
c58d762 
V8 defaults `v8_enable_sandbox` to on whenever the shared pointer
compression cage and the external code space are enabled, and that is
the configuration embedders that use the sandbox build with. Now that
the sandbox builds and passes the tests, follow that default for
`--experimental-pointer-compression-shared-cage` so the configuration
is reachable from `configure`. Multi-cage pointer compression builds
stay without it: there every IsolateGroup gets its own sandbox, and
`NodeArrayBufferAllocator` always allocates from the default one.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@github-actions github-actions Bot mentioned this pull request Sep 7, 2026
Merged
aduh95 pushed a commit that referenced this pull request Sep 7, 2026
@codebytere @aduh95
build: enable the V8 sandbox in shared-cage builds
773c7d7 
V8 defaults `v8_enable_sandbox` to on whenever the shared pointer
compression cage and the external code space are enabled, and that is
the configuration embedders that use the sandbox build with. Now that
the sandbox builds and passes the tests, follow that default for
`--experimental-pointer-compression-shared-cage` so the configuration
is reachable from `configure`. Multi-cage pointer compression builds
stay without it: there every IsolateGroup gets its own sandbox, and
`NodeArrayBufferAllocator` always allocates from the default one.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Sep 15, 2026
@codebytere @aduh95
build: enable the V8 sandbox in shared-cage builds
55213fd 
V8 defaults `v8_enable_sandbox` to on whenever the shared pointer
compression cage and the external code space are enabled, and that is
the configuration embedders that use the sandbox build with. Now that
the sandbox builds and passes the tests, follow that default for
`--experimental-pointer-compression-shared-cage` so the configuration
is reachable from `configure`. Multi-cage pointer compression builds
stay without it: there every IsolateGroup gets its own sandbox, and
`NodeArrayBufferAllocator` always allocates from the default one.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Sep 16, 2026
@codebytere @aduh95
src: support building with the V8 sandbox
a0f99f1 
With V8_ENABLE_SANDBOX every ArrayBuffer backing store has to be
allocated inside the sandbox, so memory that Node.js or a library
allocated itself cannot be wrapped and has to be copied in. Several
places already special-cased this, each with its own #ifdef, but a
build with the sandbox enabled still failed to compile
(`kMaxSafeBufferSizeForSandbox`), aborted in `crypto.randomUUID()`
(`SecureBuffer` was UNREACHABLE) and threw from every caller of the
malloc-owning `Buffer::New()` (v8.serialize, string transcoding, IPC,
the public `Buffer::New(isolate, data, length)`), and trace_events,
SEA assets and FFI still wrapped outside memory unconditionally.

Add `AdoptIntoBackingStore()`, which wraps the memory as before in
regular builds and copies it into an isolate-allocated backing store,
running the deleter right away, when the sandbox is enabled, and use it
at all of those sites so the #ifdef lives in one place. The trace
category flag cannot be copied since JS polls it, so under the sandbox
`getCategoryEnabledBuffer()` returns nothing and lib falls back to
`isTraceCategoryEnabled()`; FFI zero-copy views throw
ERR_OPERATION_FAILED there for the same reason.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Sep 19, 2026
@codebytere @aduh95
build: enable the V8 sandbox in shared-cage builds
c0bec1a 
V8 defaults `v8_enable_sandbox` to on whenever the shared pointer
compression cage and the external code space are enabled, and that is
the configuration embedders that use the sandbox build with. Now that
the sandbox builds and passes the tests, follow that default for
`--experimental-pointer-compression-shared-cage` so the configuration
is reachable from `configure`. Multi-cage pointer compression builds
stay without it: there every IsolateGroup gets its own sandbox, and
`NodeArrayBufferAllocator` always allocates from the default one.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Sep 19, 2026
@codebytere @aduh95
build: enable the V8 sandbox in shared-cage builds
4b23a44 
V8 defaults `v8_enable_sandbox` to on whenever the shared pointer
compression cage and the external code space are enabled, and that is
the configuration embedders that use the sandbox build with. Now that
the sandbox builds and passes the tests, follow that default for
`--experimental-pointer-compression-shared-cage` so the configuration
is reachable from `configure`. Multi-cage pointer compression builds
stay without it: there every IsolateGroup gets its own sandbox, and
`NodeArrayBufferAllocator` always allocates from the default one.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: #62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@github-actions github-actions Bot mentioned this pull request Sep 20, 2026
Merged
@github-actions github-actions Bot mentioned this pull request Oct 11, 2026
Merged
MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026
@MarshallOfSound
fix(patch): enable the V8 sandbox in shared-cage builds
43a8269 
Upstream now enables the V8 sandbox only for shared-cage builds. Keep
Electron's behaviour of enabling it whenever pointer compression is on,
matching Chromium's V8. The unofficial.gni x86 hunk landed upstream.

Ref: nodejs/node#62237
Ref: nodejs/node#65491
MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026
@MarshallOfSound
fix(patch): enable the V8 sandbox in shared-cage builds
6b358b6 
Upstream now enables the V8 sandbox only for shared-cage builds. Keep
Electron's behaviour of enabling it whenever pointer compression is on,
matching Chromium's V8. The unofficial.gni x86 hunk landed upstream.

Ref: nodejs/node#62237
Ref: nodejs/node#65491
MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026
@MarshallOfSound
fix(patch): enable the V8 sandbox in shared-cage builds
86e642a 
Upstream now enables the V8 sandbox only for shared-cage builds. Keep
Electron's behaviour of enabling it whenever pointer compression is on,
matching Chromium's V8. The unofficial.gni x86 hunk landed upstream.

Ref: nodejs/node#62237
Ref: nodejs/node#65491
MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026
@MarshallOfSound
fix(patch): enable the V8 sandbox in shared-cage builds
9ac0bee 
Upstream now enables the V8 sandbox only for shared-cage builds. Keep
Electron's behaviour of enabling it whenever pointer compression is on,
matching Chromium's V8. The unofficial.gni x86 hunk landed upstream.

Ref: nodejs/node#62237
Ref: nodejs/node#65491
MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026
@MarshallOfSound
fix(patch): enable the V8 sandbox in shared-cage builds
4927ba5 
Upstream now enables the V8 sandbox only for shared-cage builds. Keep
Electron's behaviour of enabling it whenever pointer compression is on,
matching Chromium's V8. The unofficial.gni x86 hunk landed upstream.

Ref: nodejs/node#62237
Ref: nodejs/node#65491
MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026
@electron-roller @MarshallOfSound
chore: bump node to v24.22.0 (44-x-y) (#55004)
e67a209 
* chore: bump node in DEPS to v24.22.0

* fix(patch): enable the V8 sandbox in shared-cage builds

Upstream now enables the V8 sandbox only for shared-cage builds. Keep
Electron's behaviour of enabling it whenever pointer compression is on,
matching Chromium's V8. The unofficial.gni x86 hunk landed upstream.

Ref: nodejs/node#62237
Ref: nodejs/node#65491

* fix(patch): use names for asymmetric key algorithms

Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed
ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h>
from env.h, so those hunks are no longer needed.

Ref: nodejs/node#65966
Ref: nodejs/node#66108
Ref: nodejs/node#65484

* fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL

The skip is now upstream.

Ref: nodejs/node#65808

* fix(patch): detach cppgc wrappers from their Realm before it is freed

Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the
wrapper owns and frees its list node, so the local PopFront/delete hunk
is dropped.

Ref: nodejs/node#65778

* fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY

Ref: nodejs/node#66082

* chore: remove upstreamed patches

- nodejs/node#63985
- nodejs/node#65630
- nodejs/node#64453
- nodejs/node#65778

* chore: update patches (trivial only)

* chore: update filenames.auto.gni

* fix(patch): discover hashes from OpenSSL providers

ncrypto's DigestCache::aliases() now has a function-local static, so
ncrypto needs the no_exit_time_destructors config too.

Ref: nodejs/node#65484

* fix(patch): read the peer certificate chain without consuming it

sk_X509_num() returns size_t in BoringSSL.

Ref: nodejs/node#65602

* fix(patch): fix shared buffer growability validation

Ref: nodejs/node#65845

* chore: update patches (trivial only)

* node#65820: src: fix null pointer call when running without a startup snapshot

SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the
embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData().

Ref: nodejs/node#65820

* node#65407: src: apply IsolateSettings when using a snapshot

node::CreateEnvironment() no longer installs the per-isolate message
listener when it builds the environment from a snapshot, so stop
compensating for that and let SetIsolateUpForNode() add it.

Ref: nodejs/node#65407

* test: drop removed wasm rab integration flag from webidl buffer source test

Chromium's V8 removed --experimental-wasm-rab-integration after shipping
it; the new test-internal-webidl-buffer-source.js passes it.

Ref: nodejs/node#65845
Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297

* chore: remove obsolete TLS external-memory accounting patch

Upstream moved the SecureContext external-memory Increase() into Init()
and guards the Decrease() on ctx_, so the leak this patch fixed is gone.
The patch's removal of that guard now double-decrements when v24.22.0's
TLSSocket closes a single-use context before the wrapper is destroyed,
tripping V8's ExternalMemoryAccounter DCHECK in every TLS test. Already
dropped on main in #53524.

* test: skip new rmSync permission error test when the Node suite runs as root

test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and
relies on a read-only directory blocking removal, which root bypasses;
the Linux Node.js test job runs as root.

Ref: nodejs/node#64453

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Sam Attard <sattard@anthropic.com>
MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026
@electron-roller @MarshallOfSound
chore: bump node to v24.22.0 (45-x-y) (#55005)
33f217a 
* chore: bump node in DEPS to v24.22.0

* fix(patch): enable the V8 sandbox in shared-cage builds

Upstream now enables the V8 sandbox only for shared-cage builds. Keep
Electron's behaviour of enabling it whenever pointer compression is on,
matching Chromium's V8. The unofficial.gni x86 hunk landed upstream.

Ref: nodejs/node#62237
Ref: nodejs/node#65491

* fix(patch): use names for asymmetric key algorithms

Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed
ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h>
from env.h, so those hunks are no longer needed.

Ref: nodejs/node#65966
Ref: nodejs/node#66108
Ref: nodejs/node#65484

* fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL

The skip is now upstream.

Ref: nodejs/node#65808

* fix(patch): detach cppgc wrappers from their Realm before it is freed

Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the
wrapper owns and frees its list node, so the local PopFront/delete hunk
is dropped.

Ref: nodejs/node#65778

* fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY

Ref: nodejs/node#66082

* chore: remove upstreamed patches

- nodejs/node#63985
- nodejs/node#65630
- nodejs/node#64453
- nodejs/node#65778

* chore: update patches (trivial only)

* chore: update filenames.auto.gni

* fix(patch): discover hashes from OpenSSL providers

ncrypto's DigestCache::aliases() now has a function-local static, so
ncrypto needs the no_exit_time_destructors config too.

Ref: nodejs/node#65484

* fix(patch): read the peer certificate chain without consuming it

sk_X509_num() returns size_t in BoringSSL.

Ref: nodejs/node#65602

* fix(patch): fix shared buffer growability validation

Ref: nodejs/node#65845

* chore: update patches (trivial only)

* node#65820: src: fix null pointer call when running without a startup snapshot

SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the
embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData().

Ref: nodejs/node#65820

* node#65407: src: apply IsolateSettings when using a snapshot

node::CreateEnvironment() no longer installs the per-isolate message
listener when it builds the environment from a snapshot, so stop
compensating for that and let SetIsolateUpForNode() add it.

Ref: nodejs/node#65407

* test: drop removed wasm rab integration flag from webidl buffer source test

Chromium's V8 removed --experimental-wasm-rab-integration after shipping
it; the new test-internal-webidl-buffer-source.js passes it.

Ref: nodejs/node#65845
Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297

* test: skip new rmSync permission error test when the Node suite runs as root

test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and
relies on a read-only directory blocking removal, which root bypasses;
the Linux Node.js test job runs as root.

Ref: nodejs/node#64453

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Sam Attard <sattard@anthropic.com>
MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026
@electron-roller @MarshallOfSound
chore: bump node to v24.22.0 (43-x-y) (#55003)
6bb4a9f 
* chore: bump node in DEPS to v24.22.0

* fix(patch): enable the V8 sandbox in shared-cage builds

Upstream now enables the V8 sandbox only for shared-cage builds. Keep
Electron's behaviour of enabling it whenever pointer compression is on,
matching Chromium's V8. The unofficial.gni x86 hunk landed upstream.

Ref: nodejs/node#62237
Ref: nodejs/node#65491

* fix(patch): use names for asymmetric key algorithms

Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed
ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h>
from env.h, so those hunks are no longer needed.

Ref: nodejs/node#65966
Ref: nodejs/node#66108
Ref: nodejs/node#65484

* fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL

The skip is now upstream.

Ref: nodejs/node#65808

* fix(patch): detach cppgc wrappers from their Realm before it is freed

Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the
wrapper owns and frees its list node, so the local PopFront/delete hunk
is dropped.

Ref: nodejs/node#65778

* fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY

Ref: nodejs/node#66082

* chore: remove upstreamed patches

- nodejs/node#63985
- nodejs/node#65630
- nodejs/node#64453
- nodejs/node#65778

* chore: update patches (trivial only)

* chore: update filenames.auto.gni

* fix(patch): discover hashes from OpenSSL providers

ncrypto's DigestCache::aliases() now has a function-local static, so
ncrypto needs the no_exit_time_destructors config too.

Ref: nodejs/node#65484

* fix(patch): read the peer certificate chain without consuming it

sk_X509_num() returns size_t in BoringSSL.

Ref: nodejs/node#65602

* chore: update patches (trivial only)

* node#65820: src: fix null pointer call when running without a startup snapshot

SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the
embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData().

Ref: nodejs/node#65820

* node#65407: src: apply IsolateSettings when using a snapshot

node::CreateEnvironment() no longer installs the per-isolate message
listener when it builds the environment from a snapshot, so stop
compensating for that and let SetIsolateUpForNode() add it.

Ref: nodejs/node#65407

* test: drop removed wasm rab integration flag from webidl buffer source test

Chromium's V8 removed --experimental-wasm-rab-integration after shipping
it; the new test-internal-webidl-buffer-source.js passes it.

Ref: nodejs/node#65845
Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297

* chore: remove obsolete TLS external-memory accounting patch

Upstream moved the SecureContext external-memory Increase() into Init()
and guards the Decrease() on ctx_, so the leak this patch fixed is gone.
The patch's removal of that guard now double-decrements when v24.22.0's
TLSSocket closes a single-use context before the wrapper is destroyed,
tripping V8's ExternalMemoryAccounter DCHECK in every TLS test. Already
dropped on main in #53524.

* test: skip new rmSync permission error test when the Node suite runs as root

test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and
relies on a read-only directory blocking removal, which root bypasses;
the Linux Node.js test job runs as root.

Ref: nodejs/node#64453

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Sam Attard <sattard@anthropic.com>
MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026
@electron-roller @MarshallOfSound
chore: bump node to v24.22.0 (main) (#55006)
1f3d86b 
* chore: bump node in DEPS to v24.22.0

* fix(patch): enable the V8 sandbox in shared-cage builds

Upstream now enables the V8 sandbox only for shared-cage builds. Keep
Electron's behaviour of enabling it whenever pointer compression is on,
matching Chromium's V8. The unofficial.gni x86 hunk landed upstream.

Ref: nodejs/node#62237
Ref: nodejs/node#65491

* fix(patch): use names for asymmetric key algorithms

Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed
ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h>
from env.h, so those hunks are no longer needed.

Ref: nodejs/node#65966
Ref: nodejs/node#66108
Ref: nodejs/node#65484

* fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL

The skip is now upstream.

Ref: nodejs/node#65808

* fix(patch): detach cppgc wrappers from their Realm before it is freed

Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the
wrapper owns and frees its list node, so the local PopFront/delete hunk
is dropped.

Ref: nodejs/node#65778

* fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY

Ref: nodejs/node#66082

* chore: remove upstreamed patches

- nodejs/node#63985
- nodejs/node#65630
- nodejs/node#64453
- nodejs/node#65778

* chore: update patches (trivial only)

* chore: update filenames.auto.gni

* fix(patch): discover hashes from OpenSSL providers

ncrypto's DigestCache::aliases() now has a function-local static, so
ncrypto needs the no_exit_time_destructors config too.

Ref: nodejs/node#65484

* fix(patch): read the peer certificate chain without consuming it

sk_X509_num() returns size_t in BoringSSL.

Ref: nodejs/node#65602

* fix(patch): fix shared buffer growability validation

Ref: nodejs/node#65845

* chore: update patches (trivial only)

* node#65820: src: fix null pointer call when running without a startup snapshot

SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the
embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData().

Ref: nodejs/node#65820

* node#65407: src: apply IsolateSettings when using a snapshot

node::CreateEnvironment() no longer installs the per-isolate message
listener when it builds the environment from a snapshot, so stop
compensating for that and let SetIsolateUpForNode() add it.

Ref: nodejs/node#65407

* test: drop removed wasm rab integration flag from webidl buffer source test

Chromium's V8 removed --experimental-wasm-rab-integration after shipping
it; the new test-internal-webidl-buffer-source.js passes it.

Ref: nodejs/node#65845
Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297

* test: skip new rmSync permission error test when the Node suite runs as root

test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and
relies on a read-only directory blocking removal, which root bypasses;
the Linux Node.js test job runs as root.

Ref: nodejs/node#64453

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Sam Attard <sattard@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@legendecas legendecas legendecas approved these changes
@jasnell jasnell jasnell approved these changes
@addaleax addaleax Awaiting requested review from addaleax

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@codebytere @nodejs-github-bot @jasnell @addaleax @legendecas