From 71b3676332d3b9d5cce80b983ad7d78fa16e58c9 Mon Sep 17 00:00:00 2001 From: Trevor Burnham Date: Sat, 8 Aug 2026 20:54:22 -0400 Subject: [PATCH 1/3] sqlite: reject statement-less SQL in SQLTagStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 PR-URL: https://github.com/nodejs/node/pull/65157 Fixes: https://github.com/nodejs/node/issues/65149 Reviewed-By: René Reviewed-By: Trivikram Kamat --- src/node_sqlite.cc | 8 ++++++++ test/parallel/test-sqlite-template-tag.js | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 0898e450a503..7688ba30b899 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -3659,6 +3659,14 @@ BaseObjectPtr SQLTagStore::PrepareStatement( return BaseObjectPtr(); } + // sqlite3_prepare_v2() reports success without producing a statement when + // the input holds no SQL, such as a comment. Such a statement cannot be + // bound or executed, so reject it instead of caching it. + if (s == nullptr) { + THROW_ERR_INVALID_ARG_VALUE(env, "The SQL query contains no statements."); + return BaseObjectPtr(); + } + BaseObjectPtr stmt_obj = StatementSync::Create( env, BaseObjectPtr(session->database_), s); diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 445231bef0bd..20376e199d1b 100644 --- a/test/parallel/test-sqlite-template-tag.js +++ b/test/parallel/test-sqlite-template-tag.js @@ -317,6 +317,29 @@ test('sql error messages are descriptive', () => { }); }); +test('rejects SQL that contains no statements', () => { + const expectedError = { + code: 'ERR_INVALID_ARG_VALUE', + message: /contains no statements/, + }; + + for (const method of ['run', 'get', 'all', 'iterate']) { + assert.throws(() => { + // eslint-disable-next-line no-unused-expressions + sql[method]`-- comment`; + }, expectedError); + + assert.throws(() => { + // eslint-disable-next-line no-unused-expressions + sql[method]``; + }, expectedError); + } + + // A rejected statement must not be cached, so a later valid query with the + // same tag store still works. + assert.strictEqual(sql.run`INSERT INTO foo (text) VALUES (${'bob'})`.changes, 1); +}); + test('a tag store keeps the database alive by itself', () => { const sql = new DatabaseSync(':memory:').createTagStore(); From cb9bb66780936138b360e61d50133fdf080998e3 Mon Sep 17 00:00:00 2001 From: Trevor Burnham Date: Sun, 9 Aug 2026 11:48:27 -0400 Subject: [PATCH 2/3] sqlite: reject statement-less SQL in prepare() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 PR-URL: https://github.com/nodejs/node/pull/65157 Fixes: https://github.com/nodejs/node/issues/65149 Reviewed-By: René Reviewed-By: Trivikram Kamat --- doc/api/sqlite.md | 4 ++++ src/node_sqlite.cc | 15 ++++++++++--- test/parallel/test-sqlite-database-sync.js | 26 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md index ae194ff2acaf..49c1260708d3 100644 --- a/doc/api/sqlite.md +++ b/doc/api/sqlite.md @@ -670,6 +670,10 @@ console.log(query.get()); * `sql` {string} A SQL string to compile to a prepared statement. diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 7688ba30b899..97384bdf449d 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1581,6 +1581,16 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { int r = sqlite3_prepare_v2(db->connection_, *sql, -1, &s, nullptr); CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void()); + + // sqlite3_prepare_v2() reports success without producing a statement when + // the input holds no SQL, such as a comment. Such a statement can never be + // stepped, and tracking it would leave a dangling pointer in statements_ + // because its destructor treats a null statement as already finalized. + if (s == nullptr) { + THROW_ERR_INVALID_ARG_VALUE(env, "The SQL query contains no statements."); + return; + } + BaseObjectPtr stmt = StatementSync::Create(env, BaseObjectPtr(db), s); db->statements_.insert(stmt.get()); @@ -3659,9 +3669,8 @@ BaseObjectPtr SQLTagStore::PrepareStatement( return BaseObjectPtr(); } - // sqlite3_prepare_v2() reports success without producing a statement when - // the input holds no SQL, such as a comment. Such a statement cannot be - // bound or executed, so reject it instead of caching it. + // As in DatabaseSync::Prepare(), reject input that holds no SQL rather + // than caching a statement that can never be bound or stepped. if (s == nullptr) { THROW_ERR_INVALID_ARG_VALUE(env, "The SQL query contains no statements."); return BaseObjectPtr(); diff --git a/test/parallel/test-sqlite-database-sync.js b/test/parallel/test-sqlite-database-sync.js index af7677a3cfc0..08a636c9cbdc 100644 --- a/test/parallel/test-sqlite-database-sync.js +++ b/test/parallel/test-sqlite-database-sync.js @@ -397,6 +397,32 @@ suite('DatabaseSync.prototype.prepare()', () => { message: /The "sql" argument must be a string/, }); }); + + test('throws if sql contains no statements', (t) => { + using db = new DatabaseSync(nextDb()); + + for (const sql of ['', ' ', ';', '-- comment', '/* comment */']) { + t.assert.throws(() => { + db.prepare(sql); + }, { + code: 'ERR_INVALID_ARG_VALUE', + message: /contains no statements/, + }); + } + }); + + test('prepares statements that contain comments', (t) => { + using db = new DatabaseSync(nextDb()); + const queries = [ + '-- lead\nSELECT 1 AS v', + 'SELECT 1 AS v -- trail', + 'SELECT /* mid */ 1 AS v', + ]; + + for (const sql of queries) { + t.assert.strictEqual(db.prepare(sql).get().v, 1); + } + }); }); suite('DatabaseSync.prototype.exec()', () => { From 6a6dee3ed02cdb9510d298c53c6f5072f60916e3 Mon Sep 17 00:00:00 2001 From: Kamal Rawal Date: Tue, 11 Aug 2026 11:18:03 +0530 Subject: [PATCH 3/3] assert: improve documentation wording Signed-off-by: Rawal27 PR-URL: https://github.com/nodejs/node/pull/64953 Reviewed-By: Aviv Keller Reviewed-By: Rich Trott --- doc/api/assert.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 1a99709767ef..c33f4ba82336 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -317,7 +317,7 @@ const assert2 = new Assert({ skipPrototype: true }); assert2.deepStrictEqual(foo, bar); // OK ``` -When destructured, methods lose access to the instance's `this` context and revert to default assertion behavior +When destructured, methods lose access to the instance's `this` context and revert to the default assertion behavior (diff: 'simple', non-strict mode). To maintain custom options when using destructured methods, avoid destructuring and call methods directly on the instance. @@ -423,8 +423,8 @@ are also recursively evaluated by the following rules. ### Comparison details * Primitive values are compared with the [`==` operator][], - with the exception of {NaN}. It is treated as being identical in case - both sides are {NaN}. + except for {NaN}, which is treated as identical when both + sides are {NaN}. * [Type tags][Object.prototype.toString()] of objects should be the same. * Only [enumerable "own" properties][] are considered. * Object constructors are compared when available. @@ -938,7 +938,7 @@ error messages as expressive as possible. If specified, `error` can be a [`Class`][], {RegExp} or a validation function. See [`assert.throws()`][] for more details. -Besides the async nature to await the completion behaves identically to +Aside from asynchronously awaiting completion, it behaves identically to [`assert.doesNotThrow()`][]. ```mjs