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>
Expose SQLite's virtual table API through a new
`database.createModule(name, options)` method, wrapping
`sqlite3_create_module_v2()`. This enables read-only virtual tables
backed by JavaScript data sources, usable either as an eponymous table
(`SELECT * FROM module_name`) or via `CREATE VIRTUAL TABLE t USING
module_name`. Hidden columns pass parameters using table-valued
function syntax (`SELECT * FROM module_name(param1, param2)`).
`options` accepts `columns`, `rows`, `directOnly`, and
`useBigIntArguments`. Column types are validated against INTEGER,
TEXT, REAL, BLOB, and ANY, and column names are quoted when building
the `sqlite3_declare_vtab()` schema.
Rebased from https://github.com/nodejs/node/pull/61544, which was
opened by byteforge38 and became inactive. Changes on top of that
work:
- xColumn reports the value each hidden column was constrained to,
rather than NULL. SQLite treats xBestIndex's `omit` as a hint, so it
may recheck a constraint it already handed to xFilter; against NULL
that recheck rejected every row, and `gs(1, 3) WHERE start = 1`
returned no rows.
- xBestIndex lowers estimatedCost as it consumes constraints. With a
constant cost the planner was free to pick the unconstrained plan and
recheck afterwards, so a correlated parameter such as
`FROM t, gs(t.a, t.a + 1)` also returned no rows.
- Violations of the iteration protocol report a SQLite error instead of
calling PropagateJSError with no JavaScript exception pending. That
left `.all()` returning undefined and `exec()` reporting success.
- xBestIndex passes the constrained hidden-column indices to xFilter
through idxStr rather than an int bitmask, which previously aliased
for parameter indices at or above the width of an int.
- xFilter, xNext, and xColumn take a CallbackDepthGuard. Without it
close() from inside rows(), an iterator's next(), or a row getter
finalized the statement that SQLite was still stepping, crashing the
process.
- xClose calls the iterator's return() method so generator `finally`
blocks run when SQLite stops stepping early, as it does for LIMIT or
a `break` out of a for...of loop. It is skipped while tearing down
from ~StatementSync or ~DatabaseSync, which run from garbage
collection callbacks where JavaScript cannot be executed; an
abandoned generator does not run `finally` in JavaScript either. It
is also skipped when an error is already pending, so that error still
reaches the caller.
- VirtualTableModule holds a BaseObjectWeakPtr<DatabaseSync> to match
UserDefinedFunction instead of a raw pointer.
- createModule() rejects being called from an authorizer callback.
- Documents that values yielded by rows() follow the usual conversion
rules, so a number is stored as REAL and a BigInt as INTEGER even
when a column declares INTEGER, since virtual tables do not apply
column affinity to the values they return.
Refs: https://github.com/nodejs/node/pull/61544
Refs: https://github.com/nodejs/node/issues/63826
Fixes: https://github.com/nodejs/node/issues/61539
Co-authored-by: byteforge38 <stormcraft318@gmail.com>
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
Assisted-by: Claude Opus 5
PR-URL: https://github.com/nodejs/node/pull/65787
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ValidateDatabasePath() treats any object exposing a string `href` as a
URL, then asserts the parse result with CHECK(ada::can_parse(location)).
Whether that string parses depends on user input rather than on an
invariant the code guarantees, so a value such as { href: 'not a url' }
aborted the process instead of throwing.
Replace the CHECK with an ERR_INVALID_URL exception, matching how
node_file.cc reports an unparsable URL. Both DatabaseSync() and backup()
validate their path through this function, so both paths are covered.
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Assisted-by: Claude Code
PR-URL: https://github.com/nodejs/node/pull/66026
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
A second Environment created from the same `IsolateData`, which
embedding.md allows and the cctest fixture does, aborted as soon as it
loaded `stream_wrap`, `tcp_wrap`, `pipe_wrap`, `tty_wrap`, `http2` or
the crypto key classes, i.e. on `require('net')`: those bindings
created their per-isolate templates from the per-Environment binding
initializer and stored them with a setter that CHECKs the slot is
empty. Templates that were already cached per isolate (ffi, sqlite,
dtls) instead hit "FunctionTemplate already instantiated" when
`SetConstructorFunction()` renamed them.
Build these templates once per isolate and reuse them, set class names
when the template is created, and store the `ShutdownWrap`, `WriteWrap`
and `Http2Stream` function templates rather than their instance
templates so the second Environment can expose the same constructors.
Refs: https://github.com/nodejs/node/pull/43802
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65978
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
SQLite can invoke JavaScript through user-defined SQL functions while
applying a changeset, even when no filter or conflict callback is set.
Copy non-empty changesets unconditionally so JavaScript cannot detach
or modify the input while SQLite is still reading it.
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex
PR-URL: https://github.com/nodejs/node/pull/65870
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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: https://github.com/nodejs/node/pull/62237
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
prepare(), function(), aggregate(), deserialize(), applyChangeset() and
backup() validated the connection, then read their options bag with
Object::Get(). A property getter runs arbitrary JavaScript at that
point, so a getter calling close() invalidates what was just checked.
Five of the six then passed a null sqlite3* to SQLite and crashed;
prepare() reported a spurious "out of memory".
Re-check IsOpen() after option parsing, immediately before the SQLite
call, keeping the early check so invalid calls still fail before any
user code runs. IsOpen() is the only condition a getter can change:
authorizer and callback depths are RAII-managed. createSession()
already parsed options first, so it only gains the early check.
deserialize() also latched the buffer length before reading
options.dbName. A getter that shrank the backing store left the length
too large; CopyContents() then handed the uninitialized remainder to
SQLite, from where serialize() returned it to JavaScript. Check the
CopyContents() result instead of discarding it.
function() and aggregate() cast the callback's length property with
As<Int32>() and no IsInt32() guard. length is configurable, so any type
reached the cast and produced a silently wrong arity.
Fixes: https://github.com/nodejs/node/issues/65586
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65595
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Run SQLite backup after-work callbacks inside an internal callback
scope. This drains promise reactions and next ticks before the event
loop can become idle.
Add a child-process regression test where backup is the final active
request.
Assisted-by: Codex
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65666
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
SQLite runs "PRAGMA table_xinfo" from inside its pre-update hook, while
it is still walking the connection's session list. Deleting a session
from a callback that the PRAGMA triggers frees memory that walk is still
using. Both an authorizer callback and a 'sqlite.db.query' subscriber
reach that window, and either one segfaults.
Reject session.close() and Symbol.dispose when the connection is inside
any callback. An already-closed session stays a no-op so that disposal
remains idempotent.
Fixes: https://github.com/nodejs/node/issues/65428
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65454
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Session objects are weak and nothing else holds a strong reference to
them, so a garbage collection can free one while SQLite is still using
it. SQLite runs "PRAGMA table_xinfo" from inside its pre-update hook,
which reaches JavaScript, so a GC during a callback invoked from there
can collect a session the hook is still walking.
Hold a strong reference to every attached session for the duration of
each callback SQLite invokes. The trace callback now enters that guard
before building its payload, since the allocation can itself trigger a
garbage collection.
Fixes: https://github.com/nodejs/node/issues/65460
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65465
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
c4b755ee00 tracks the statements currently being stepped and rejects
reentry into them, but the guard is established inside the execution
helpers, after the entry point has already reset the statement and
bound its parameters. Binding reads properties off the supplied
object, so a named-parameter getter runs JavaScript in that window.
Reentering the same statement there resets it a second time and, for
iterate(), hands out a second iterator; both iterators record the
same reset generation, so neither is invalidated and they interleave
rows from one virtual machine.
Establish the guard at the four StatementSync entry points instead,
before the reset, so it spans binding as well as stepping. The
existing guards inside the helpers are left in place; the stepping
set is a stack, so the nested acquisition is balanced.
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
Assisted-by: claude:opus-5
PR-URL: https://github.com/nodejs/node/pull/65294
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
An iterator can leave a statement active between sqlite3_step() calls.
Finalizing that statement from an authorizer callback can release its
locks and change the outcome of the statement being authorized.
Reject close() and Symbol.dispose when the statement is busy and the
connection is in an authorizer callback. Continue allowing idle
statements to be finalized.
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
PR-URL: https://github.com/nodejs/node/pull/65369
Fixes: https://github.com/nodejs/node/issues/65368
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
SQLite requires that an authorizer callback not modify the connection
that invoked it, and counts sqlite3_prepare_v2() and sqlite3_step() as
modifications. node:sqlite let the callback call prepare(), exec(), the
statement execution methods, and other connection-mutating APIs on the
same DatabaseSync. Track authorizer depth on DatabaseSync with an RAII
guard around the callback and throw ERR_INVALID_STATE from the affected
entry points while it is on the stack. Covering every authorizer
invocation, including the re-prepare that SQLite can run during
sqlite3_step(), exposed a second and distinct hazard: reentering a
statement that is currently being stepped is a use-after-free rather
than a contract violation, since finalizing it frees the virtual machine
under sqlite3_step() and re-running it resets that machine
mid-execution. Any callback SQLite invokes during execution can reach
it, so a user-defined function is enough. Track the statements currently
being stepped and reject reentry into only those, which leaves a
user-defined function free to prepare, run, and finalize its own helper
statements.
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
Fixes: https://github.com/nodejs/node/issues/63207
Assisted-by: claude:opus-5
PR-URL: https://github.com/nodejs/node/pull/65156
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Apply the same check to DatabaseSync::Prepare() so that statement-less
SQL is rejected at preparation instead of on first use. This matches
SQLite's own oo1 JavaScript API, which throws when the SQL contains no
statements rather than exposing the C API's null statement pointer.
Previously db.prepare('-- comment') returned a StatementSync whose
statement_ was null. Every method on it threw "statement has been
finalized", which was misleading because nothing had been finalized, and
the object was still inserted into statements_. Since IsFinalized() is
true for a null statement, its destructor skipped UntrackStatement() and
left a dangling pointer in the set that a later close() would finalize.
Refs: https://github.com/nodejs/node/pull/65157#discussion_r3742903347
Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65157
Fixes: https://github.com/nodejs/node/issues/65149
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
sqlite3_prepare_v2() returns SQLITE_OK without producing a statement
when its input holds no SQL, such as a comment. PrepareStatement() only
checked the return code, so it cached a StatementSync wrapping a null
sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only
guards against a null statement under SQLITE_ENABLE_API_ARMOR, and
segfaulted.
Reject such input instead of caching it. The StatementSync methods
already avoid the crash because their IsFinalized() guard treats a null
statement as finalized.
Fixes: https://github.com/nodejs/node/issues/65149
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65157
Fixes: https://github.com/nodejs/node/issues/65149
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
sqlite3_column_text() can return nullptr on failure which
was not handled.
sqlite3_column_blob() can return nullptr for zero-length BLOBs,
which is then passed to memcpy() which is UB.
Avoid this by checking for a nullptr.
Signed-off-by: ndossche <nora.dossche@ugent.be>
PR-URL: https://github.com/nodejs/node/pull/63288
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This extends explicit resource management support to prepared
statements, allowing a StatementSync to be deterministically
finalized via a `using` declaration, mirroring the existing
DatabaseSync and Session dispose methods.
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/64232
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
DatabaseSync::DeleteSessions() deleted native sqlite3_session handles
without clearing the pointers held by their Session wrappers. Reopening
the database allowed stale session handles to be used, crashing the
process.
Track Session wrappers and delete sessions through Session::Delete() so
the wrappers are marked closed.
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
PR-URL: https://github.com/nodejs/node/pull/64783
Fixes: https://github.com/nodejs/node/issues/64782
Reviewed-By: Aviv Keller <me@aviv.sh>
SQLTagStore Run/Iterate/Get/All reset the shared cached statement via
raw sqlite3_reset(), bypassing ResetStatement() and its reset_generation_
bump, so live iterators were never invalidated. Also mark the iterator
done on SQLITE_DONE so it cannot restart the statement.
PR-URL: https://github.com/nodejs-private/node-private/pull/896
Refs: https://hackerone.com/reports/3564941
Reviewed-By: Robert Nagy <ronagy@icloud.com>
CVE-ID: CVE-2026-58041
When sqlite3_exec() or sqlite3changeset_apply() call JavaScript
callbacks (user-defined functions, conflict handlers, or filter
callbacks), the DatabaseSync object could be garbage-collected
if the JavaScript code drops all references to it. Both methods
only held a raw DatabaseSync* pointer on the C++ stack, which
V8 GC does not track.
Add a BaseObjectPtr<DatabaseSync> guard that keeps the database
alive for the duration of these SQLite API calls, preventing a
use-after-free when the JavaScript callback triggers GC.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/64535
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
When sqlite3_open_v2() fails, SQLite still assigns a database handle
in a "sick" state. Such a handle may only be used to retrieve the
error and must then be released with sqlite3_close(). Keeping it in
connection_ meant that after a failed open():
- isOpen incorrectly reported true,
- every method passed the "database is not open" check and called
SQLite APIs on the sick handle, which is an API misuse,
- registering a user-defined function leaked its user data in builds
with SQLITE_ENABLE_API_ARMOR, because SQLite rejects the call
before taking ownership of the user data,
- open() could not be retried since the database appeared open.
Close and reset the connection handle when open() fails at any point
so that the database remains closed and open() can be retried.
Fixes: https://github.com/nodejs/node/issues/63831
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/63854
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Guilherme Araújo