sqlite: reject statement-less SQL in SQLTagStore by TrevorBurnham · Pull Request #65157 · nodejs/node · GitHub

sqlite3_prepare_v2() returns SQLITE_OK without producing a statement when its input holds no SQL, such as a comment or an empty string. Neither caller checked for this.

SQLTagStore cached a StatementSync wrapping the null sqlite3_stmt. Executing it reached sqlite3_clear_bindings(), which only guards against a null statement under SQLITE_ENABLE_API_ARMOR, and segfaulted:

const db = new DatabaseSync(':memory:');
const store = db.createTagStore();
store.run`-- comment`;  // segfault

All four query methods (run, get, all, iterate) were affected.

db.prepare() returned a StatementSync whose statement_ was null. Every method on it threw statement has been finalized, which was misleading because nothing had been finalized. It was also still inserted into statements_, and because IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer that a later close() would finalize.

Both now throw ERR_INVALID_ARG_VALUE at preparation. SQL that merely contains comments is unaffected.

Fixes: #65149

Review requested:

@nodejs/sqlite

Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. 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: nodejs#65157 (comment) Signed-off-by: Trevor Burnham <trevorburnham@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: nodejs#65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@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: nodejs#65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (b2b7405) to head (d7ad352).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65157      +/-   ##
==========================================
- Coverage   90.33%   90.33%   -0.01%     
==========================================
  Files         760      760              
  Lines      248523   248529       +6     
  Branches    46906    46895      -11     
==========================================
- Hits       224511   224509       -2     
+ Misses      15451    15450       -1     
- Partials     8561     8570       +9

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.49% <100.00%> (+0.25%)	⬆️

... and 23 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.

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: nodejs#65149 Signed-off-by: Trevor Burnham <trevorburnham@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: nodejs#65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

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

Landed in a576f1c...cb9bb66

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: #65149 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157 Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> 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: #65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157 Fixes: #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: #65149 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157 Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> 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: #65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157 Fixes: #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: #65149 Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157 Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> 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: #65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157 Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Aug 9, 2026

Renegade334 mentioned this pull request Aug 9, 2026

sqlite: reject statement-less SQLTagStore queries #65152

Closed

Renegade334 reviewed Aug 9, 2026

View reviewed changes

Comment thread src/node_sqlite.cc Outdated

TrevorBurnham force-pushed the sqlite/fix-tagstore-null-stmt branch from af2725f to 3d2d495 Compare August 9, 2026 15:55

Renegade334 approved these changes Aug 9, 2026

View reviewed changes

Renegade334 reviewed Aug 9, 2026

View reviewed changes

Comment thread doc/api/sqlite.md Outdated

TrevorBurnham force-pushed the sqlite/fix-tagstore-null-stmt branch from 3d2d495 to 45be4bd Compare August 9, 2026 19:18

TrevorBurnham marked this pull request as ready for review August 9, 2026 23:47

trivikr approved these changes Aug 10, 2026

View reviewed changes

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 10, 2026

This comment was marked as resolved.

Sign in to view

TrevorBurnham added 2 commits August 9, 2026 21:41

TrevorBurnham force-pushed the sqlite/fix-tagstore-null-stmt branch from 45be4bd to d7ad352 Compare August 10, 2026 01:49

trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 10, 2026

trivikr approved these changes Aug 10, 2026

View reviewed changes

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 10, 2026

nodejs-github-bot closed this Aug 11, 2026

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 11, 2026

github-actions Bot mentioned this pull request Aug 25, 2026

2026-08-26, Version 26.8.0 (Current) #65551

Merged

sqlite: reject statement-less SQL in SQLTagStore by TrevorBurnham · Pull Request #65157 · nodejs/node · GitHub

Uh oh!

Uh oh!

sqlite: reject statement-less SQL in SQLTagStore - #65157

sqlite: reject statement-less SQL in SQLTagStore#65157
TrevorBurnham wants to merge 2 commits into
nodejs:mainfrom
TrevorBurnham:sqlite/fix-tagstore-null-stmt

TrevorBurnham commented Aug 9, 2026 •
edited
Loading

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 9, 2026

Uh oh!

Uh oh!

Renegade334 left a comment

Uh oh!

Uh oh!

This comment was marked as resolved.

codecov Bot commented Aug 10, 2026 •
edited
Loading

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 10, 2026

Uh oh!

nodejs-github-bot commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sqlite: reject statement-less SQL in SQLTagStore by TrevorBurnham · Pull Request #65157 · nodejs/node · GitHub

Uh oh!

Uh oh!

sqlite: reject statement-less SQL in SQLTagStore - #65157

sqlite: reject statement-less SQL in SQLTagStore#65157TrevorBurnham wants to merge 2 commits intonodejs:mainnodejs/node:mainfrom TrevorBurnham:sqlite/fix-tagstore-null-stmtTrevorBurnham/node:sqlite/fix-tagstore-null-stmtCopy head branch name to clipboard

Conversation

TrevorBurnham commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 9, 2026

Uh oh!

Uh oh!

Renegade334 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as resolved.

codecov Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Aug 10, 2026

Uh oh!

nodejs-github-bot commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sqlite: reject statement-less SQL in SQLTagStore#65157
TrevorBurnham wants to merge 2 commits into
nodejs:mainfrom
TrevorBurnham:sqlite/fix-tagstore-null-stmt

TrevorBurnham commented Aug 9, 2026 •
edited
Loading

codecov Bot commented Aug 10, 2026 •
edited
Loading