[task] the pg-mode DB client takes a pooled connection per statement, so withTenant's transaction scope is not pinned to one connection #14

New Issue

2026-10-27T07:00:25Z

coreci-chat-po commented

2026-10-27 07:00:25 +00:00

TL;DR - the real-Postgres client takes a pooled connection per statement, so withTenant does not run its transaction on one connection - the tenant scope it sets can land on a different connection than the statement it must scope. Found while fixing #12 and #13 in the same lane. Owner: coreci-chat-po (this line).

Affected repo version - coreci/coreci-chat, branch po/ci-fix-invalid-workflow-yaml, head db158255c04. Files: packages/db/src/create-db.ts (pg mode), packages/db/src/withTenant.ts, packages/db/src/db-client.ts.

Repro steps

Read packages/db/src/create-db.ts (pg mode): query() calls pool.connect() per statement and releases the client when that one statement is done.
Read packages/db/src/withTenant.ts: the transaction is four separate client calls - BEGIN, set_config('app.tenant_id', ...), the scoped statement, COMMIT - and DbClient has no transaction-scoped method, so those four calls may take four different pooled connections.
The module's own docstring says "In pg (pool), withTenant acquires a dedicated client for the transaction". The implementation does not; there is no dedicated-client path to acquire.
The suite passes today only because the pool hands back the same connection for those sequential statements. Two concurrent tenant-scoped writes can interleave across connections, and then the statement runs without its tenant scope: a write fails RLS with "new row violates row-level security policy" and a read sees zero rows.

Severity - amber. It is the transaction guarantee behind the REQ-039 isolation backstop, and it is a concurrency-dependent correctness and isolation risk in the prod path (the pg Pool), not in the PGlite dev path. Not a confirmed outage: this seat has no prod deployment and no concurrent-load run, so the runtime consequence is read from the code, not observed.

Territory match - coreci-chat product row (coreci/company org/products.md), packages/db (the RLS client) exercised by apps/control-plane and by the R-009 pen test.

Suggested action - give DbClient a transaction-scoped variant - transaction<T>(fn: (tx: DbClient) => Promise<T>): Promise<T> - implement it in create-db.ts (pg: one client held across BEGIN/COMMIT/ROLLBACK; PGlite: the single in-process connection), and have withTenant use it when it is present and fall back to the current BEGIN/COMMIT path otherwise. Keep the RLS posture as it is: no role elevation, no ownership grant for the app role, and the app role still reads and writes only inside withTenant.

Due date - 2026-10-31 (amber tier).

Dependency refs - coreci-chat#9 (the gate merge request this defect was found through).

Related issue - #12 and #13 (the two defects the same lane fixed on this branch; #13's fix also moves the membership lookup inside the tenant scope).

ci---
project: home
seat: coreci-chat-po (own PAT)
run: hourly SOP cycle (gate run 55595, job 57855, read 2026-10-27T05:57Z)
sop: sop-hourly v2.0 section 3 ACT (triage - product defect found while fixing #12/#13); owner named

**TL;DR** - the real-Postgres client takes a pooled connection per statement, so `withTenant` does not run its transaction on one connection - the tenant scope it sets can land on a different connection than the statement it must scope. Found while fixing #12 and #13 in the same lane. Owner: coreci-chat-po (this line). **Affected repo version** - coreci/coreci-chat, branch `po/ci-fix-invalid-workflow-yaml`, head `db158255c04`. Files: `packages/db/src/create-db.ts` (pg mode), `packages/db/src/withTenant.ts`, `packages/db/src/db-client.ts`. **Repro steps** 1. Read `packages/db/src/create-db.ts` (pg mode): `query()` calls `pool.connect()` per statement and releases the client when that one statement is done. 2. Read `packages/db/src/withTenant.ts`: the transaction is four separate client calls - `BEGIN`, `set_config('app.tenant_id', ...)`, the scoped statement, `COMMIT` - and `DbClient` has no transaction-scoped method, so those four calls may take four different pooled connections. 3. The module's own docstring says "In pg (pool), withTenant acquires a dedicated client for the transaction". The implementation does not; there is no dedicated-client path to acquire. 4. The suite passes today only because the pool hands back the same connection for those sequential statements. Two concurrent tenant-scoped writes can interleave across connections, and then the statement runs without its tenant scope: a write fails RLS with "new row violates row-level security policy" and a read sees zero rows. **Severity** - amber. It is the transaction guarantee behind the REQ-039 isolation backstop, and it is a concurrency-dependent correctness and isolation risk in the prod path (the pg Pool), not in the PGlite dev path. Not a confirmed outage: this seat has no prod deployment and no concurrent-load run, so the runtime consequence is read from the code, not observed. **Territory match** - coreci-chat product row (`coreci/company` `org/products.md`), `packages/db` (the RLS client) exercised by `apps/control-plane` and by the R-009 pen test. **Suggested action** - give `DbClient` a transaction-scoped variant - `transaction<T>(fn: (tx: DbClient) => Promise<T>): Promise<T>` - implement it in `create-db.ts` (pg: one client held across BEGIN/COMMIT/ROLLBACK; PGlite: the single in-process connection), and have `withTenant` use it when it is present and fall back to the current BEGIN/COMMIT path otherwise. Keep the RLS posture as it is: no role elevation, no ownership grant for the app role, and the app role still reads and writes only inside `withTenant`. **Due date** - 2026-10-31 (amber tier). **Dependency refs** - coreci-chat#9 (the gate merge request this defect was found through). **Related issue** - #12 and #13 (the two defects the same lane fixed on this branch; #13's fix also moves the membership lookup inside the tenant scope). --- ci--- project: home seat: coreci-chat-po (own PAT) run: hourly SOP cycle (gate run 55595, job 57855, read 2026-10-27T05:57Z) sop: sop-hourly v2.0 section 3 ACT (triage - product defect found while fixing #12/#13); owner named

coreci-chat-po added this to the v1 milestone 2026-10-27 07:00:25 +00:00

coreci-chat-po added the type:task label 2026-10-27 07:00:25 +00:00

coreci-chat-po self-assigned this 2026-10-27 07:00:25 +00:00

coreci-chat-po commented

2026-10-27 08:37:46 +00:00

TL;DR - the fix is on the branch: the transaction now runs on one pinned connection, so the tenant scope cannot be set on one connection and read on another.

What

DbClient gains an optional transaction(fn); the pg client holds one pooled connection across BEGIN/COMMIT/ROLLBACK, PGlite uses its single connection.
withTenant uses it when the client has it and keeps its BEGIN/COMMIT path as the fallback.
New test packages/db/tests/withTenant.test.ts: 4 cases - the pinned path, the fallback path, and the rollback on failure in both.

Why - a pool that hands out a different connection per statement breaks the RLS scope: the statements then run unscoped and the row-level security policy sees no tenant.

Action - no close yet: the close rides the merge of PR coreci-chat#9 plus a green test-postgres job. The verification runs for the new head are queued, so nothing is claimed green.

Details - head f8a36b9e29 on https://git.coreci.dev/coreci/coreci-chat/pull/9 (packages/db/src/db-client.ts, create-db.ts, withTenant.ts, tests/withTenant.test.ts)

**TL;DR** - the fix is on the branch: the transaction now runs on one pinned connection, so the tenant scope cannot be set on one connection and read on another. **What** - DbClient gains an optional `transaction(fn)`; the pg client holds one pooled connection across BEGIN/COMMIT/ROLLBACK, PGlite uses its single connection. - withTenant uses it when the client has it and keeps its BEGIN/COMMIT path as the fallback. - New test packages/db/tests/withTenant.test.ts: 4 cases - the pinned path, the fallback path, and the rollback on failure in both. **Why** - a pool that hands out a different connection per statement breaks the RLS scope: the statements then run unscoped and the row-level security policy sees no tenant. **Action** - no close yet: the close rides the merge of PR coreci-chat#9 plus a green test-postgres job. The verification runs for the new head are queued, so nothing is claimed green. **Details** - head f8a36b9e29b3 on https://git.coreci.dev/coreci/coreci-chat/pull/9 (packages/db/src/db-client.ts, create-db.ts, withTenant.ts, tests/withTenant.test.ts)

coreci-chat-po commented

2026-10-27 09:19:00 +00:00

TL;DR - the fix is verified green: the real-Postgres job passed every step, and the new regression test passes in both jobs. Not closing yet - the fix still has to land on main.

What

Run 56039 (head f8a36b9e29b3), job 58322 test-postgres: 15 of 15 steps green, zero error lines in the log.
packages/db/tests/withTenant.test.ts (4 cases: pinned path, fallback path, rollback on failure) passes in BOTH jobs - real Postgres 16 and PGlite.
The whole suite is green under real Postgres 16 with the security policy enforced: control-plane 86 tests, mcp 360, auth 52, db 17 (cross-tenant pen test included).

Why - the tenant scope now runs on one pinned connection, which is what this filing asked for.

Action - @coreci-cpo approve PR coreci-chat#9 at head f8a36b9e29b3; @jonathan merge on protected main. Then this closes.

Details - run 56039, jobs 58321 and 58322; the same shape on run 56040.

**TL;DR** - the fix is verified green: the real-Postgres job passed every step, and the new regression test passes in both jobs. Not closing yet - the fix still has to land on `main`. **What** - Run 56039 (head `f8a36b9e29b3`), job 58322 `test-postgres`: 15 of 15 steps green, zero error lines in the log. - `packages/db/tests/withTenant.test.ts` (4 cases: pinned path, fallback path, rollback on failure) passes in BOTH jobs - real Postgres 16 and PGlite. - The whole suite is green under real Postgres 16 with the security policy enforced: control-plane 86 tests, mcp 360, auth 52, db 17 (cross-tenant pen test included). **Why** - the tenant scope now runs on one pinned connection, which is what this filing asked for. **Action** - @coreci-cpo approve PR coreci-chat#9 at head `f8a36b9e29b3`; @jonathan merge on protected `main`. Then this closes. **Details** - run 56039, jobs 58321 and 58322; the same shape on run 56040.

coreci-chat-po commented

2026-10-27 22:59:16 +00:00

TL;DR — fixed, merged, verified on main: the transaction now holds one pooled connection. Closing.

What — DbClient gained an optional transaction(fn); the pg client holds ONE pooled connection across BEGIN / set_config / the scoped statement / COMMIT / ROLLBACK, PGlite uses its single connection, and withTenant uses the transaction path when it is present and keeps the BEGIN/COMMIT path as the fallback. The regression test packages/db/tests/withTenant.test.ts (4 cases: the pinned path, the fallback path, and the rollback on failure in both) came with it.

Evidence — in main via merge 74902f21 (PR coreci-chat#9, merged 22:38:41Z, parent 2 = the approved head f8a36b9e29b3). Verified on the first run this repo has ever had on main: run 58210, job 60501 (test-postgres, real Postgres 16) — tests/withTenant.test.ts 4 tests passed and the R-009 cross-tenant pen test 6 tests green; job 60500 (test-pglite) green on the same db suite.

Honest limit — this closes the defect on the merged code, not on a prod deployment: this seat has no prod deployment to observe.

**TL;DR — fixed, merged, verified on main: the transaction now holds one pooled connection. Closing.** What — `DbClient` gained an optional `transaction(fn)`; the pg client holds ONE pooled connection across BEGIN / set_config / the scoped statement / COMMIT / ROLLBACK, PGlite uses its single connection, and `withTenant` uses the transaction path when it is present and keeps the BEGIN/COMMIT path as the fallback. The regression test `packages/db/tests/withTenant.test.ts` (4 cases: the pinned path, the fallback path, and the rollback on failure in both) came with it. Evidence — in main via merge `74902f21` (PR coreci-chat#9, merged 22:38:41Z, parent 2 = the approved head `f8a36b9e29b3`). Verified on the first run this repo has ever had on `main`: run 58210, job 60501 (`test-postgres`, real Postgres 16) — `tests/withTenant.test.ts` 4 tests passed and the R-009 cross-tenant pen test 6 tests green; job 60500 (`test-pglite`) green on the same db suite. Honest limit — this closes the defect on the merged code, not on a prod deployment: this seat has no prod deployment to observe.

coreci-chat-po referenced this issue

2026-10-27 22:59:22 +00:00

fix(ci): quote the 7 step names that start with "[" — the gate file was invalid YAML, so no run could ever be created (coreci-chat#1) #9

coreci-chat-po closed this issue

2026-10-30 23:19:59 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: coreci/coreci-chat#14