From e3d7cc494b03fc29242d1c499f4fd59f074fad7e Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 8 Jul 2026 11:32:49 +0000 Subject: [PATCH 1/5] sql: reject tx.unsafe()/tx.file() on a settled transaction or released reserved handle The tagged-template call, savepoint(), connect(), flush() and the runner's own internal statements on a transaction/reserved handle all check connectionState and reject with "Connection closed" once begin() has settled (or the reserved connection has been released). unsafe() and file() on the same handle did not, so a reference retained past the callback could still issue statements on a pooled connection the handle no longer owns. On a real pool that connection may already belong to another caller. Add the same closed/acceptQueries guard to unsafe() and file() on both the transaction and reserved handles. file() re-checks after the async read so a handle closed while reading still rejects. --- src/js/bun/sql.ts | 36 ++++++++++++++++++++++++++++++++++ test/js/sql/sql.test.ts | 35 +++++++++++++++++++++++++++++++++ test/js/sql/sqlite-sql.test.ts | 25 +++++++++++++++++++++++ 3 files changed, 96 insertions(+) diff --git a/src/js/bun/sql.ts b/src/js/bun/sql.ts index c3a74582a25..c1a6c98e5d5 100644 --- a/src/js/bun/sql.ts +++ b/src/js/bun/sql.ts @@ -278,13 +278,31 @@ const SQL: typeof Bun.SQL = function SQL( } reserved_sql.unsafe = (string, args = []) => { + if ( + state.connectionState & ReservedConnectionState.closed || + !(state.connectionState & ReservedConnectionState.acceptQueries) + ) { + return Promise.$reject(pool.connectionClosedError()); + } return unsafeQueryFromTransaction(string, args, pooledConnection, state.queries); }; reserved_sql.file = async (path: string, args = []) => { + if ( + state.connectionState & ReservedConnectionState.closed || + !(state.connectionState & ReservedConnectionState.acceptQueries) + ) { + return Promise.$reject(pool.connectionClosedError()); + } return await Bun.file(path) .text() .then(text => { + if ( + state.connectionState & ReservedConnectionState.closed || + !(state.connectionState & ReservedConnectionState.acceptQueries) + ) { + return Promise.$reject(pool.connectionClosedError()); + } return unsafeQueryFromTransaction(text, args, pooledConnection, state.queries); }); }; @@ -579,12 +597,30 @@ const SQL: typeof Bun.SQL = function SQL( return queryFromTransaction(strings, values, pooledConnection, state.queries); } transaction_sql.unsafe = (string, args = []) => { + if ( + state.connectionState & ReservedConnectionState.closed || + !(state.connectionState & ReservedConnectionState.acceptQueries) + ) { + return Promise.$reject(pool.connectionClosedError()); + } return unsafeQueryFromTransaction(string, args, pooledConnection, state.queries); }; transaction_sql.file = async (path: string, args = []) => { + if ( + state.connectionState & ReservedConnectionState.closed || + !(state.connectionState & ReservedConnectionState.acceptQueries) + ) { + return Promise.$reject(pool.connectionClosedError()); + } return await Bun.file(path) .text() .then(text => { + if ( + state.connectionState & ReservedConnectionState.closed || + !(state.connectionState & ReservedConnectionState.acceptQueries) + ) { + return Promise.$reject(pool.connectionClosedError()); + } return unsafeQueryFromTransaction(text, args, pooledConnection, state.queries); }); }; diff --git a/test/js/sql/sql.test.ts b/test/js/sql/sql.test.ts index 7818fecb5ca..9f362f27954 100644 --- a/test/js/sql/sql.test.ts +++ b/test/js/sql/sql.test.ts @@ -3997,6 +3997,41 @@ CREATE TABLE ${table_name} ( expect(xs.map(x => x.x).join("")).toBe("123"); }); + test("reserved.unsafe() and reserved.file() reject after release()", async () => { + await using sql = postgres({ ...options, max: 1 }); + const reserved = await sql.reserve(); + await reserved.release(); + const outcome = async (p: Promise) => p.then(() => "fulfilled", (e: any) => `rejected: ${e.code}`); + expect({ + template: await outcome(reserved`select 1 as x`), + unsafe: await outcome(reserved.unsafe("select 1 as x")), + file: await outcome(reserved.file(rel("select.sql"))), + }).toEqual({ + template: "rejected: ERR_POSTGRES_CONNECTION_CLOSED", + unsafe: "rejected: ERR_POSTGRES_CONNECTION_CLOSED", + file: "rejected: ERR_POSTGRES_CONNECTION_CLOSED", + }); + }); + + test("tx.unsafe() and tx.file() reject after begin() settles", async () => { + await using sql = postgres({ ...options, max: 1 }); + let leaked: any; + await sql.begin(async tx => { + leaked = tx; + await tx.unsafe("select 1 as x"); + }); + const outcome = async (p: Promise) => p.then(() => "fulfilled", (e: any) => `rejected: ${e.code}`); + expect({ + template: await outcome(leaked`select 1 as x`), + unsafe: await outcome(leaked.unsafe("select 1 as x")), + file: await outcome(leaked.file(rel("select.sql"))), + }).toEqual({ + template: "rejected: ERR_POSTGRES_CONNECTION_CLOSED", + unsafe: "rejected: ERR_POSTGRES_CONNECTION_CLOSED", + file: "rejected: ERR_POSTGRES_CONNECTION_CLOSED", + }); + }); + test("keeps process alive when it should", async () => { const file = path.posix.join(__dirname, "sql-fixture-ref.ts"); const result = await $`DATABASE_URL=${process.env.DATABASE_URL} ${bunExe()} ${file}`; diff --git a/test/js/sql/sqlite-sql.test.ts b/test/js/sql/sqlite-sql.test.ts index f90ebfe3d19..7d2dbe7a6d0 100644 --- a/test/js/sql/sqlite-sql.test.ts +++ b/test/js/sql/sqlite-sql.test.ts @@ -1144,6 +1144,31 @@ describe("Transactions", () => { const accounts = await sql`SELECT * FROM accounts WHERE id = 1`; expect(accounts[0].balance).toBe(1002); }); + + test("tx.unsafe() and tx.file() reject after begin() settles", async () => { + const dir = tempDirWithFiles("sql-tx-closed", { + "q.sql": `INSERT INTO accounts VALUES (98, 0)`, + }); + let leaked: any; + await sql.begin(async tx => { + leaked = tx; + await tx.unsafe(`INSERT INTO accounts VALUES (10, 0)`); + }); + const outcome = async (p: Promise) => p.then(() => "fulfilled", (e: any) => `rejected: ${e.message}`); + expect({ + template: await outcome(leaked`SELECT 1`), + savepoint: await outcome(Promise.resolve().then(() => leaked.savepoint(async () => {}))), + unsafe: await outcome(leaked.unsafe(`INSERT INTO accounts VALUES (99, 0)`)), + file: await outcome(leaked.file(join(dir, "q.sql"))), + }).toEqual({ + template: "rejected: Connection closed", + savepoint: "rejected: Connection closed", + unsafe: "rejected: Connection closed", + file: "rejected: Connection closed", + }); + const rows = await sql`SELECT id FROM accounts WHERE id >= 10 ORDER BY id`; + expect(rows).toEqual([{ id: 10 }]); + }); }); describe("SQLite-specific features", () => { From a271a5ec5d5608cfd6e3ef69f3104f6fb81db756 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 8 Jul 2026 11:35:05 +0000 Subject: [PATCH 2/5] [autofix.ci] apply automated fixes --- test/js/sql/sql.test.ts | 12 ++++++++++-- test/js/sql/sqlite-sql.test.ts | 6 +++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/test/js/sql/sql.test.ts b/test/js/sql/sql.test.ts index 9f362f27954..5e802b0f76e 100644 --- a/test/js/sql/sql.test.ts +++ b/test/js/sql/sql.test.ts @@ -4001,7 +4001,11 @@ CREATE TABLE ${table_name} ( await using sql = postgres({ ...options, max: 1 }); const reserved = await sql.reserve(); await reserved.release(); - const outcome = async (p: Promise) => p.then(() => "fulfilled", (e: any) => `rejected: ${e.code}`); + const outcome = async (p: Promise) => + p.then( + () => "fulfilled", + (e: any) => `rejected: ${e.code}`, + ); expect({ template: await outcome(reserved`select 1 as x`), unsafe: await outcome(reserved.unsafe("select 1 as x")), @@ -4020,7 +4024,11 @@ CREATE TABLE ${table_name} ( leaked = tx; await tx.unsafe("select 1 as x"); }); - const outcome = async (p: Promise) => p.then(() => "fulfilled", (e: any) => `rejected: ${e.code}`); + const outcome = async (p: Promise) => + p.then( + () => "fulfilled", + (e: any) => `rejected: ${e.code}`, + ); expect({ template: await outcome(leaked`select 1 as x`), unsafe: await outcome(leaked.unsafe("select 1 as x")), diff --git a/test/js/sql/sqlite-sql.test.ts b/test/js/sql/sqlite-sql.test.ts index 7d2dbe7a6d0..4ff92b589a5 100644 --- a/test/js/sql/sqlite-sql.test.ts +++ b/test/js/sql/sqlite-sql.test.ts @@ -1154,7 +1154,11 @@ describe("Transactions", () => { leaked = tx; await tx.unsafe(`INSERT INTO accounts VALUES (10, 0)`); }); - const outcome = async (p: Promise) => p.then(() => "fulfilled", (e: any) => `rejected: ${e.message}`); + const outcome = async (p: Promise) => + p.then( + () => "fulfilled", + (e: any) => `rejected: ${e.message}`, + ); expect({ template: await outcome(leaked`SELECT 1`), savepoint: await outcome(Promise.resolve().then(() => leaked.savepoint(async () => {}))), From a99ad43a55406c60eb231ed6fc7ea783225ccde7 Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 8 Jul 2026 11:41:44 +0000 Subject: [PATCH 3/5] sql: extract cannotAcceptQueries() helper for the closed/acceptQueries guard --- src/js/bun/sql.ts | 72 +++++++++++++---------------------------------- 1 file changed, 20 insertions(+), 52 deletions(-) diff --git a/src/js/bun/sql.ts b/src/js/bun/sql.ts index c1a6c98e5d5..066d4984c1a 100644 --- a/src/js/bun/sql.ts +++ b/src/js/bun/sql.ts @@ -27,6 +27,13 @@ interface TransactionState { queries: Set>; } +function cannotAcceptQueries(state: TransactionState) { + return ( + state.connectionState & ReservedConnectionState.closed || + !(state.connectionState & ReservedConnectionState.acceptQueries) + ); +} + function adapterFromOptions(options: Bun.SQL.__internal.DefinedOptions) { switch (options.adapter) { case "postgres": @@ -259,10 +266,7 @@ const SQL: typeof Bun.SQL = function SQL( } function reserved_sql(strings: string | TemplateStringsArray | SQLHelper | Query, ...values: any[]) { - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } if ($isArray(strings)) { @@ -278,29 +282,20 @@ const SQL: typeof Bun.SQL = function SQL( } reserved_sql.unsafe = (string, args = []) => { - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } return unsafeQueryFromTransaction(string, args, pooledConnection, state.queries); }; reserved_sql.file = async (path: string, args = []) => { - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } return await Bun.file(path) .text() .then(text => { - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } return unsafeQueryFromTransaction(text, args, pooledConnection, state.queries); @@ -361,10 +356,7 @@ const SQL: typeof Bun.SQL = function SQL( }; reserved_sql.begin = (options_or_fn: string | TransactionCallback, fn?: TransactionCallback) => { // begin is allowed the difference is that we need to make sure to use the same connection and never release it - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } let callback = fn; @@ -398,10 +390,7 @@ const SQL: typeof Bun.SQL = function SQL( }; reserved_sql.close = async (options?: { timeout?: number }) => { const reserveQueries = state.queries; - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$resolve(undefined); } state.connectionState &= ~ReservedConnectionState.acceptQueries; @@ -444,10 +433,7 @@ const SQL: typeof Bun.SQL = function SQL( return Promise.$resolve(undefined); }; reserved_sql.release = () => { - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } // just release the connection back to the pool @@ -579,10 +565,7 @@ const SQL: typeof Bun.SQL = function SQL( strings: string | TemplateStringsArray | import("internal/sql/shared.ts").SQLHelper | Query, ...values: any[] ) { - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } if ($isArray(strings)) { @@ -597,28 +580,19 @@ const SQL: typeof Bun.SQL = function SQL( return queryFromTransaction(strings, values, pooledConnection, state.queries); } transaction_sql.unsafe = (string, args = []) => { - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } return unsafeQueryFromTransaction(string, args, pooledConnection, state.queries); }; transaction_sql.file = async (path: string, args = []) => { - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } return await Bun.file(path) .text() .then(text => { - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$reject(pool.connectionClosedError()); } return unsafeQueryFromTransaction(text, args, pooledConnection, state.queries); @@ -681,10 +655,7 @@ const SQL: typeof Bun.SQL = function SQL( }; transaction_sql.close = async function (options?: { timeout?: number }) { // we dont actually close the connection here, we just set the state to closed and rollback the transaction - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { return Promise.$resolve(undefined); } state.connectionState &= ~ReservedConnectionState.acceptQueries; @@ -766,10 +737,7 @@ const SQL: typeof Bun.SQL = function SQL( transaction_sql.savepoint = async (fn: TransactionCallback, name?: string): Promise => { let savepoint_callback = fn; - if ( - state.connectionState & ReservedConnectionState.closed || - !(state.connectionState & ReservedConnectionState.acceptQueries) - ) { + if (cannotAcceptQueries(state)) { throw pool.connectionClosedError(); } From baa774c96c60ac31b0ef1839b8aa496f901cd6f8 Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 8 Jul 2026 11:52:36 +0000 Subject: [PATCH 4/5] test: move sqlite closed-handle test to its own file sqlite-sql.test.ts has a pre-existing 10k-iteration test that times out under the debug+ASAN gate build; moving the new test out so the gate only runs the relevant coverage. --- .../sql/sql-transaction-closed-handle.test.ts | 42 +++++++++++++++++++ test/js/sql/sqlite-sql.test.ts | 29 ------------- 2 files changed, 42 insertions(+), 29 deletions(-) create mode 100644 test/js/sql/sql-transaction-closed-handle.test.ts diff --git a/test/js/sql/sql-transaction-closed-handle.test.ts b/test/js/sql/sql-transaction-closed-handle.test.ts new file mode 100644 index 00000000000..a39da698f66 --- /dev/null +++ b/test/js/sql/sql-transaction-closed-handle.test.ts @@ -0,0 +1,42 @@ +import { SQL } from "bun"; +import { expect, test } from "bun:test"; +import { tempDirWithFiles } from "harness"; +import { join } from "node:path"; + +// tx.unsafe()/tx.file() must reject once sql.begin() has settled, the same way the +// tagged-template call and tx.savepoint() already do. Before the fix they reached the +// pooled connection the handle no longer owns and the write was durable. +test("tx.unsafe() and tx.file() reject after begin() settles", async () => { + const dir = tempDirWithFiles("sql-tx-closed", { + "q.sql": `INSERT INTO accounts VALUES (98, 0)`, + }); + const sql = new SQL("sqlite://:memory:"); + try { + await sql`CREATE TABLE accounts (id INTEGER PRIMARY KEY, balance REAL)`; + let leaked: any; + await sql.begin(async tx => { + leaked = tx; + await tx.unsafe(`INSERT INTO accounts VALUES (10, 0)`); + }); + const outcome = async (p: Promise) => + p.then( + () => "fulfilled", + (e: any) => `rejected: ${e.message}`, + ); + expect({ + template: await outcome(leaked`SELECT 1`), + savepoint: await outcome(Promise.resolve().then(() => leaked.savepoint(async () => {}))), + unsafe: await outcome(leaked.unsafe(`INSERT INTO accounts VALUES (99, 0)`)), + file: await outcome(leaked.file(join(dir, "q.sql"))), + }).toEqual({ + template: "rejected: Connection closed", + savepoint: "rejected: Connection closed", + unsafe: "rejected: Connection closed", + file: "rejected: Connection closed", + }); + const rows = await sql`SELECT id FROM accounts ORDER BY id`; + expect(rows).toEqual([{ id: 10 }]); + } finally { + await sql.close(); + } +}); diff --git a/test/js/sql/sqlite-sql.test.ts b/test/js/sql/sqlite-sql.test.ts index 4ff92b589a5..f90ebfe3d19 100644 --- a/test/js/sql/sqlite-sql.test.ts +++ b/test/js/sql/sqlite-sql.test.ts @@ -1144,35 +1144,6 @@ describe("Transactions", () => { const accounts = await sql`SELECT * FROM accounts WHERE id = 1`; expect(accounts[0].balance).toBe(1002); }); - - test("tx.unsafe() and tx.file() reject after begin() settles", async () => { - const dir = tempDirWithFiles("sql-tx-closed", { - "q.sql": `INSERT INTO accounts VALUES (98, 0)`, - }); - let leaked: any; - await sql.begin(async tx => { - leaked = tx; - await tx.unsafe(`INSERT INTO accounts VALUES (10, 0)`); - }); - const outcome = async (p: Promise) => - p.then( - () => "fulfilled", - (e: any) => `rejected: ${e.message}`, - ); - expect({ - template: await outcome(leaked`SELECT 1`), - savepoint: await outcome(Promise.resolve().then(() => leaked.savepoint(async () => {}))), - unsafe: await outcome(leaked.unsafe(`INSERT INTO accounts VALUES (99, 0)`)), - file: await outcome(leaked.file(join(dir, "q.sql"))), - }).toEqual({ - template: "rejected: Connection closed", - savepoint: "rejected: Connection closed", - unsafe: "rejected: Connection closed", - file: "rejected: Connection closed", - }); - const rows = await sql`SELECT id FROM accounts WHERE id >= 10 ORDER BY id`; - expect(rows).toEqual([{ id: 10 }]); - }); }); describe("SQLite-specific features", () => { From 65e0e013eb667a7bb34002f3ac95622ee53b8733 Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 8 Jul 2026 11:59:12 +0000 Subject: [PATCH 5/5] ci: retrigger