sqlite: reject busy statement finalization in authorizer

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>

doc/api/sqlite.md

@@ -1128,7 +1128,7 @@ Finalizes the prepared statement. An exception is thrown if the statement is
 already finalized. An [`ERR_INVALID_STATE`][] error is thrown if this statement
 is currently executing, which happens when the method is called from a callback
 that the statement itself triggered, such as a user-defined function, an
-aggregate function, or a [`'sqlite.db.query'`][] subscriber. Other statements
+aggregate function, or a [`'sqlite.db.query'`][] subscriber. Idle statements
 on the same connection can be finalized from such a callback. This method is a
 wrapper around [`sqlite3_finalize()`][].
 

src/node_sqlite.cc

@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
       (db)->IsInAuthorizerCallback(),                                          \
       "database cannot be accessed from an authorizer callback")
 
+// Finalizing a busy statement from an authorizer can release its locks and
+// change the outer statement's outcome.
+#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt)                      \
+  THROW_AND_RETURN_ON_BAD_STATE(                                               \
+      (env),                                                                   \
+      (stmt)->db_->IsInAuthorizerCallback() &&                                 \
+          sqlite3_stmt_busy((stmt)->statement_.get()),                         \
+      "database cannot be accessed from an authorizer callback")
+
 // A statement's virtual machine cannot be reentered while sqlite3_step() is
 // running it. Finalizing it frees the VM outright, and re-running it resets the
 // VM mid-execution; both are use-after-free rather than merely a contract
@@ -2915,6 +2924,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
   THROW_AND_RETURN_ON_BAD_STATE(
       env, stmt->IsFinalized(), "statement has been finalized");
   THROW_AND_RETURN_IF_STEPPING(env, stmt);
+  THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
   stmt->Close();
 }
 
@@ -2928,6 +2938,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
     return;
   }
   THROW_AND_RETURN_IF_STEPPING(env, stmt);
+  THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
   stmt->Close();
 }
 

src/node_sqlite.h

@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
   void DecrementAuthorizerDepth() { --authorizer_depth_; }
   bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
 
-  // Finalizing a statement frees its virtual machine, so a callback that
-  // SQLite invokes from inside sqlite3_step() must not finalize the statement
-  // being stepped. Other statements on the connection are safe to finalize.
+  // A callback must not finalize the statement being stepped. Other statements
+  // are safe unless they are busy during an authorizer callback.
   void PushSteppingStatement(sqlite3_stmt* stmt) {
     stepping_statements_.push_back(stmt);
   }

test/parallel/test-sqlite-authz.js

@@ -397,10 +397,8 @@ suite('authorizer callback reentrancy', () => {
     assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
   });
 
-  // Only the statement being stepped is unsafe to finalize. Other statements
-  // on the connection have their own virtual machines, so finalizing them from
-  // a callback is allowed.
-  it('allows finalizing a statement that is not being executed', () => {
+  // An idle statement has no virtual-machine state or locks to release.
+  it('allows finalizing an idle statement', () => {
     const db = new DatabaseSync(':memory:');
     db.exec('CREATE TABLE t (x INTEGER)');
     db.exec('INSERT INTO t VALUES (1)');
@@ -417,6 +415,43 @@ suite('authorizer callback reentrancy', () => {
     });
   });
 
+  // A paused iterator is busy and may hold locks between sqlite3_step() calls.
+  it('rejects finalizing another active statement', () => {
+    for (const method of ['close', 'dispose']) {
+      const db = new DatabaseSync(':memory:');
+      db.exec('CREATE TABLE t (x INTEGER)');
+      db.exec('INSERT INTO t VALUES (1), (2), (3)');
+      const stmt = db.prepare('SELECT x FROM t');
+      const iter = stmt.iterate();
+      iter.next();
+      let outcome = 'authorizer callback did not run';
+
+      db.setAuthorizer((actionCode) => {
+        if (actionCode === constants.SQLITE_DROP_TABLE) {
+          try {
+            if (method === 'close') {
+              stmt.close();
+            } else {
+              stmt[Symbol.dispose]();
+            }
+            outcome = 'did not throw';
+          } catch (err) {
+            outcome = `${err.code}: ${err.message}`;
+          }
+        }
+        return constants.SQLITE_OK;
+      });
+
+      assert.throws(() => db.exec('DROP TABLE t'), {
+        code: 'ERR_SQLITE_ERROR',
+        message: 'database table is locked',
+      });
+      assert.strictEqual(outcome, expectedError);
+      db.setAuthorizer(null);
+      iter.return();
+    }
+  });
+
   // Disposal is idempotent, so a statement that is already finalized must stay
   // a no-op even inside a callback. Throwing here would turn a `using` scope's
   // real exception into a SuppressedError.