Necessarily semver-major.
When `--permission` is on, every env var not matched by
`--allow-env` is removed at startup. It takes names,
prefix patterns (`PREFIX_*`), or `*`, repeatable or
comma-sep'd.
There are a range of env vars that Node.js itself uses,
and a default range that are generally known to be safe
in common usage. These are never scrubbed. These include
things like `NODE_OPTIONS`, `NODE_EXTRA_CA_CERTS`, `PATH`,
`HOME`, etc. `NODE_ENV` is not in the defaults and must
be allowed explicitly.
Proxy vars (`HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY`) are
also not in the defaults since they can carry credentials.
When `--use-env-proxy` or `NODE_USE_ENV_PROXY` is set and
any of them were removed, a single warning naming them is
emitted.
Env vars can be dropped at runtime after reading using
`permission.drop()`. This is a stronger protection than
using `process.env.FOO = undefined` because it will
scrub the env var also from the environment block.
On Linux, the removed entries are overwritten in the
initial environment block. fs reads of any other
process's /proc/<pid>/environ, ancestors included, are
denied regardless of `--allow-fs-read`. A process's own
is readable only with `--allow-env=*`. Symlinks are
resolved before the check so paths like
/dev/fd/../../<ppid>/environ are caught. The check only
canonicalizes paths that statfs() reports are on procfs.
On Windows, removal also clears the C runtime's copy
of the environ using _wputenv_s.
Reading a removed name returns undefined, warns once per
name, and publishes to a diagnostics channel.
Env file keys are allowed. If the user had reason to pass
in an env file the assumption is they meant to allow them.
File-source config (node.config.json and NODE_OPTIONS
from a .env file) can only narrow the allow list.
Embedders must call ScrubProcessEnvironment() themselves
on startup. This is left up to the embedder to determine
the exact timing but needs to be called before startup
actually happens.
Child processes are started with `--allow-env=*`. Those
either receive the explicit env they were started with
or only the env they inherit from the parent. Since the
parent process is scrubbed, and the child cannot read
any other process's /proc/<pid>/environ, it should never
see more than the parent can.
Main part of the impl was done by hand. Docs, tests,
verification pass, and cleanup nits were automated.
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
PR-URL: https://github.com/nodejs/node/pull/66132
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Skip the %XX walk in unescapeBuffer when the input has no '%'.
Add a dedicated '&'/'=' scanner for the default parse path so
it does not build separator code arrays or run the multi-char
state machine.
Official benchmark/querystring/querystring-parse.js:
encodemany is about 38% faster, manyblankpairs about 17%,
encodelast about 10%, noencode about 8%.
Official querystring-unescapebuffer.js with no escapes is
about 36% faster.
Assisted-by: a closed-source coding agent
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/66175
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
`isInt32()` accepts -0 because `-0 === (-0 | 0)`, but V8 does not
represent -0 as an Int32 value, so `Value::IsInt32()` rejects it. The
utf8 fast paths of `readFileSync()` and `writeFileSync()` hand the value
straight to the binding, which then took it for a path and aborted on
the null check.
Coerce -0 to 0 before the call, matching `getValidatedFd()` and the rest
of fs, where -0 is a valid way to name file descriptor 0.
Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65888
Fixes: https://github.com/nodejs/node/issues/65886
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
deflateReset starts a new member. Bytes already written out cannot be
taken back, so gunzip/inflate see a truncated member followed by a new
header. Refuse reset in that case. Raw deflate has no wrapper header
and is unchanged.
Signed-off-by: Xia Chao <shapirolutts@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66179
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Original commit message:
[stack-traces] Fix overflow in Error.stackTraceLimit trimming
When stack traces are captured for uncaught exceptions (enabled via
Isolate::SetCaptureStackTraceForUncaughtExceptions, e.g. by the
inspector or by Node.js's --trace-uncaught), CaptureAndSetErrorStack
reuses the simple stack trace and trims it to Error.stackTraceLimit.
Error.stackTraceLimit counts frames, but the raw call site data stores
CallSiteInfo::Fields::kCount slots per frame, so the trim multiplied the
limit by kCount: once in the uint32_t comparison against the array
length and once, as int, to compute the new length. GetStackTraceLimit
clamps the limit to [0, INT_MAX], so for very large limits the uint32_t
product can wrap to a value below the array length. The trim branch is
then taken although the limit exceeds the number of captured frames,
and the int multiplication of the new length overflows.
On main (kCount == 5) the product first wraps at 858993460. That limit
trimmed the raw data to 4 slots (no complete frame) and 858993461 to 9
slots (one frame), so error.stack silently lost frames. Infinity, the
value from the Node.js report, is clamped to INT_MAX; its wrapped
product (2147483643) is not below the array length, so on main it does
not take the trim branch and does not reach the signed overflow.
Fix this by comparing the limit with the number of frames in the raw
data (length / kCount), and only multiplying once the limit is known to
be smaller than the frame count. The resulting length is then bounded
by the existing array length and cannot overflow. Behavior for limits
that did not overflow is unchanged, since the raw data length is always
a multiple of kCount.
This regressed with https://crrev.com/c/7673818 (ebd15783b7b,
"[objects]: Defer CallSiteInfo creation"), which switched from one
CallSiteInfo per frame to kCount raw slots per frame.
This is the underlying cause of Node.js issue 66074. The symptom there
differs from main: Node's V8 14.6 backport of that change has
kCount == 6 and uses int for the comparison and for RightTrim, so the
product overflows for limits above 357913941. For many of those,
including Infinity (INT_MAX * 6 wraps to -6), the result is negative
and fails "Check failed: new_capacity > 0." in RightTrim. Comparing in
frames avoids the overflow in both cases.
The new cctest CaptureStackTraceForUncaughtExceptionHugeStackTraceLimit
enables capture for uncaught exceptions and checks that limits of
858993460, 858993461 and Infinity yield the same error.stack as a limit
of 10, and that a limit of 1 still trims to a single frame. 858993460
and 858993461 are the first limits whose product with kCount wraps
around uint32_t; both fail without this change. The new test and the
existing stack trace tests also pass in a UBSan build, with no
diagnostics.
Bug: 565047704
Refs: https://github.com/nodejs/node/issues/66074
Change-Id: I3422ca1de6a7dd9448c7fd53fb9bc5e40e2a17c1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8426465
Reviewed-by: Patrick Thier <pthier@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: eliau elkouby (אליהו אלקובי) <eliau.elkouby@gmail.com>
Commit-Queue: Patrick Thier <pthier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#110043}
Refs: https://github.com/v8/v8/commit/786c1c2d88d445eecf54efe7ba9cef27a3eef3bb
Fixes: https://github.com/nodejs/node/issues/66074
Assisted-by: a closed-source coding agent
Signed-off-by: Eliau Elkouby <145869377+eliau2005@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/66249
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
Use a dedicated fixture with a pending promise and assertion plan
instead of competing assertion timers. Keep both timeout failure
assertions and clear the keepalive interval after the tests finish.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66255
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Async hook activation can be deferred when the inspector command arrives
during a V8 interrupt. Schedule timeout creation with setImmediate so
the event loop finishes enabling the hooks before recording its async
stack.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66255
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Evaluating expressions against the running target can trigger a V8
scope assertion. Keep the target paused and wait for the initial break
before checking the function formatting. Await debugger teardown.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66231
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
pipe() installs one 'data' listener that calls dest.write() for
every chunk. That repeats encoding, mode, and end checks that stay
the same for a synchronous buffer write.
When that listener is still the only one, hand the Buffer to the
same synchronous write path without those checks. A second
listener, a non-buffer chunk, or a busy writable still goes through
emit('data').
On top of the flowing-read fast path, benchmark/streams/pipe.js is
about 31% faster (20 runs). Object-mode pipe and readable-readall
stay within noise.
Assisted-by: a closed-source coding agent
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/66182
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
flow() pulls one already-buffered chunk and calls _read() for the
next one on every iteration. That goes through the general read()
path, which updates a holey buffer array and then pulls the chunk
back out.
While a synchronous byte-mode flow is in progress, keep that
prefetched chunk on the readable state and emit it directly.
_read() of the next chunk still runs before 'data', and a nested
read() moves the chunk back onto the buffer.
benchmark/streams/pipe.js is about 77% faster (15 runs).
pipe-object-mode, readable-readall, and readable-bigread stay
within noise.
Assisted-by: a closed-source coding agent
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/66182
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
The localStorage backing file is a user-specified path, and the schema
is created with CREATE TABLE IF NOT EXISTS, so a file that already
contains tables of those names is adopted as-is. Its stored values may
then have any SQLite type, but every read asserted the expected type
with CHECK, so a wrong-typed value aborted the process. A bad
schema_version was the worst case: that assertion is in
Storage::Open(), so any access aborted and the application had no
chance to inspect or repair the file.
Report these as ERR_INVALID_STATE instead, matching the throw four
lines below the schema_version assertion for a version that is too new.
Storage::GetAll() has no JavaScript caller to throw at, so it returns
std::nullopt and the DOM storage inspector agent reports a protocol
error.
Now that a failed open returns instead of aborting, Open() has to clean
up after itself: adopt the sqlite3* into a conn_unique_ptr immediately,
so that an error does not leak the connection and leave the next access
to open another one.
Storage::GetAll() also ignored the result of sqlite3_prepare_v2() and
the status its row loop ended on, reporting a malformed file or a
mid-scan error as an empty store. Both now return std::nullopt.
Also drop a redundant second sqlite3_exec() of the init SQL that
clobbered the result of the sqlite3_prepare_v2() above it, hiding
prepare failures behind a misleading "bad parameter or other API
misuse".
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
Assisted-by: Claude Opus 5
PR-URL: https://github.com/nodejs/node/pull/65879
Fixes: https://github.com/nodejs/node/issues/65878
Fixes: https://github.com/nodejs/node/issues/64640
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Putting a time limit on a Node.js process is common in many
use cases, but each of the common approaches are generally
flawed.
OS and CI timeouts are inconsistent and blind. Every OS flavor
handles process timeouts in different ways. Stock macOS and
Windows do not ship coreutils `timeout`, etc. None of the options
can explain why the process was still running or what exactly
was interrupted. Using setTimeout only fires when the event loop
is free so it misses busy loops and thread blocks and cannot
see the stack. Diagnostics triggered by signal have the same
problem.
This introduces a `--process-timeout` that should work
consistently across all runtimes. A native watchdog thread
interrupts the main thread, even in the middle of running
code. It prints the JavaScript stack (if any) and resources
keeping the event loop alive (if any), and can optionally
print a full diagnostic report. The process exits with a
distinct code (124). If the main thread is stuck in native
code, the process still exits.
$ node --process-timeout=10s -e "while(true) {}"
(node:483970) Process timed out after 10s (--process-timeout). Exiting with code 124.
Main thread was executing JavaScript:
at [eval]:1:1
at runScriptInThisContext (node:internal/vm:219:10)
at node:internal/process/execution:485:12
at [eval]-wrapper:6:24
at runScriptInContext (node:internal/process/execution:483:60)
at evalFunction (node:internal/process/execution:317:30)
at evalTypeScript (node:internal/process/execution:329:3)
at node:internal/main/eval_string:71:3
No resources keeping the event loop alive were found.
Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
PR-URL: https://github.com/nodejs/node/pull/66138
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
openAsBlob() and openAsBlobSync() read the MIME type as
options.type || '' before validating it, causing falsy non-string
values to be replaced with the default before reaching validateString().
As a result, { type: 0 }, { type: false }, { type: null }, and
{ type: NaN } are accepted as an empty type, while { type: 1 }
throws ERR_INVALID_ARG_TYPE.
Default the option only when it is undefined so all other values are
validated against the documented string type.
Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66125
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
ffi.toBuffer() and ffi.toArrayBuffer() document copy as a boolean, but
read it with BooleanValue(), which applies JavaScript truthiness. A
falsy non-boolean such as null, 0 or '' therefore selects the
zero-copy mode, which returns a writable view over foreign memory
instead of a copy, while values such as 'false' or 1 select a copy.
Throw ERR_INVALID_ARG_TYPE when copy is neither undefined nor a
boolean. Omitting copy or passing undefined still makes a copy, and
true and false keep their current behavior.
Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66219
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Utf8Stream#write() in 'buffer' content mode only accepted Buffer
instances, even though the underlying implementation only needs
byte-addressable data. This accepts any ArrayBufferView (TypedArray,
DataView) and reinterprets it as a Buffer over the same bytes
(without copying), so callers no longer need to wrap other typed
arrays in Buffer.from() themselves.
Views are normalized to a Buffer at the single entry point
(#writeBuffer), using byteOffset/byteLength rather than the view's
element-count length, so that internal length bookkeeping used by
mergeBuf()/Buffer.concat() and the write-release logic keeps
operating on real byte counts. This mirrors the existing pattern in
zlibBuffer() (lib/zlib.js).
Signed-off-by: agape1225 <49804691+agape1225@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/65301
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Keep the original socket error as the cause of ECONNRESET errors emitted
when an HTTP response closes before completion. Preserve the existing
aborted message, error code, and event order. Leave cause absent when
there is no underlying error.
Allow ConnResetException to accept Error options, document the behavior,
and cover TLS record errors, TCP resets, explicit destruction, and
premature closure without a socket error.
This follows investigation of #66001 and addresses lost error context.
The original TLS decryption failure remains unresolved.
Refs: https://github.com/nodejs/node/issues/66001
Signed-off-by: XadillaX <i@2333.moe>
PR-URL: https://github.com/nodejs/node/pull/66061
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
shared_array_buffers grows via repeated push_back() while iterating
shared_array_buffers_, even though shared_array_buffers_.size() is
already known before the loop starts and is an exact count of how
many items will be pushed (one per iteration, unconditionally).
This reserves capacity upfront to avoid the repeated allocate/copy/
free cycle that happens each time capacity is exceeded, mirroring
the existing array_buffers.reserve() in the sibling
Message::Serialize().
Signed-off-by: agape1225 <49804691+agape1225@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/65304
Reviewed-By: James M Snell <jasnell@gmail.com>
The cpu-prof tests hand-rolled a partial version of what
test/common/child_process already provides: log the child's stderr
when the status is non-zero, then assert the status. That manual
version never shows stdout, the signal, or the command that ran, so
a CI failure gives less information than the helper would.
Replace those blocks with spawnSyncAndExitWithoutError(). Every
existing expectation is preserved -- status 0 is still required, and
signal null is now checked as well, matching the helper defaults.
Only success-path blocks whose assertions map one-to-one onto the
helper defaults are converted.
Refs: https://github.com/nodejs/node/pull/65351
Refs: https://github.com/nodejs/node/pull/65552
Assisted-by: claude:opus-5
Signed-off-by: Junhwan Choi <devjunsday@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65694
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
SQLite re-invokes xFilter on a cursor it already used, as it does
for the inner table of a correlated subquery or join, abandoning the
previous iterator mid-loop. Call its return() method so generator
`finally` blocks still run.
Refactor the xClose cleanup intoCloseIterator() so both paths share it;
on refilter, a throwing cleanup is surfaced as the query error.
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: opencode
PR-URL: https://github.com/nodejs/node/pull/66195
Fixes: https://github.com/nodejs/node/issues/66193
Reviewed-By: Guilherme Araújo <arauujogui@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reserve temporary string storage in InvokeFunction only when the
first string argument is encountered. Calls without string arguments
avoid allocating and freeing an unused vector buffer.
Reserve capacity for all arguments before saving the first string
pointer so subsequent strings cannot invalidate it. Add coverage for
multiple strings, mixed Buffer arguments, and errors during argument
conversion.
Signed-off-by: Soul Lee <alus20x@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66178
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Skip toLowerCase() for the documented ipv4/ipv6 families and add a
V8 Fast API for one-byte address strings. The FastOneByteString
callback takes FastApiCallbackOptions so V8 can use the fast path
reliably (required for string-shaped Fast API arguments).
Overlong IPv6 zone ids are parsed like uv_ip6_addr (unknown zone
is scope_id 0; address parts 40+ chars stay a miss). IPv4 zone
suffixes stay a miss, matching uv_ip4_addr.
Assisted-by: a closed-source coding agent
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/66166
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Skip redundant RSA keygen and PBKDF2 WPT variants during regular CI.
Keep the full matrix when producing a WPT report.
Use fewer scrypt, PBKDF2, and Argon2 derivations in parallel tests.
Reuse RSA key material across OAEP hash and PSS salt tests.
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
PR-URL: https://github.com/nodejs/node/pull/66244
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Object property keys are already unique unless USVString normalization
changes a key. Avoid allocating and updating a SafeMap for ordinary
record keys. Create and backfill it when normalization first changes a
key.
This improves object construction by 10% to 25% in the measured cases
and reduces allocation by about 31% for a ten-property record.
Assisted-by: Amp
Signed-off-by: Romain Lanz <romain.lanz@pm.me>
PR-URL: https://github.com/nodejs/node/pull/65801
Reviewed-By: James M Snell <jasnell@gmail.com>
James M Snell