fix: leak of request into pools (#1206)

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>

Author: ferhatelmas

src/internal/database/pg-connection.test.ts

@@ -8,10 +8,12 @@ import { vi } from 'vitest'
 import {
   getPgCancelConnectionTarget,
   PgPoolExecutor,
+  PgPoolManager,
   PgPoolStrategy,
   PgTenantConnection,
   PgTransaction,
 } from './pg-connection'
+import type { TenantConnectionOptions } from './pool'
 
 class TestablePgPoolStrategy extends PgPoolStrategy {
   getCurrentPoolForTest(): Pool {
@@ -24,8 +26,8 @@ class TestablePgPoolStrategy extends PgPoolStrategy {
 }
 
 function createPoolStrategySettings(
-  overrides: Partial<ConstructorParameters<typeof PgPoolStrategy>[0]> = {}
-): ConstructorParameters<typeof PgPoolStrategy>[0] {
+  overrides: Partial<TenantConnectionOptions> = {}
+): TenantConnectionOptions {
   return {
     tenantId: 'pg-pool-strategy-test',
     dbUrl: 'postgres://postgres:postgres@localhost:5432/postgres',
@@ -741,6 +743,38 @@ describe('PgTenantConnection', () => {
   })
 })
 
+describe('PgPoolManager', () => {
+  it('caches strategies without retaining request-scoped options', async () => {
+    const manager = new PgPoolManager()
+    const tenantId = 'pg-pool-manager-prune-test'
+    const request = { operation: { type: 'upload' } }
+
+    const strategy = manager.getPool(
+      createPoolStrategySettings({
+        tenantId,
+        headers: { authorization: 'Bearer secret' },
+        method: 'POST',
+        path: '/object/bucket/key',
+        operation: () => request.operation.type,
+      })
+    )
+
+    try {
+      const retained = (strategy as unknown as { options: Record<string, unknown> }).options
+      expect(Object.keys(retained).sort()).toEqual([
+        'clusterSize',
+        'dbUrl',
+        'isExternalPool',
+        'maxConnections',
+        'numWorkers',
+        'tenantId',
+      ])
+    } finally {
+      await manager.destroy(tenantId)
+    }
+  })
+})
+
 describe('PgPoolStrategy', () => {
   it('logs idle pg pool errors without rethrowing them', async () => {
     const strategy = new TestablePgPoolStrategy(createPoolStrategySettings())

src/internal/database/pg-connection.ts

@@ -9,6 +9,7 @@ import {
   PoolManager,
   PoolRebalanceOptions,
   PoolStats,
+  PoolStrategySettings,
   searchPath,
   TenantConnectionOptions,
 } from './pool'
@@ -95,7 +96,7 @@ export class PgPoolStrategy {
   protected pool?: Pool
   protected tlsSession?: TlsSessionSlot
 
-  constructor(protected readonly options: TenantConnectionOptions) {}
+  constructor(protected readonly options: PoolStrategySettings) {}
 
   acquire(): PgPoolExecutor {
     return new PgPoolExecutor(this.getPool())
@@ -303,7 +304,7 @@ function pulsePgPoolQueue(pool: Pool): void {
 }
 
 export class PgPoolManager extends PoolManager<PgPoolStrategy> {
-  protected newPool(settings: TenantConnectionOptions): PgPoolStrategy {
+  protected newPool(settings: PoolStrategySettings): PgPoolStrategy {
     return new PgPoolStrategy(settings)
   }
 }

src/internal/database/pool.ts

@@ -40,6 +40,16 @@ export interface TenantConnectionOptions {
   operation?: () => string | undefined
 }
 
+// Pool cache entries are long-lived:
+//  * minimum TENANT_POOL_CACHE_TTL_MS
+//  * refreshed on access
+// Strategies must retain only pool settings and can't capture the request
+// that created them (headers, the operation closure over the whole Fastify request).
+export type PoolStrategySettings = Pick<
+  TenantConnectionOptions,
+  'tenantId' | 'dbUrl' | 'isExternalPool' | 'maxConnections' | 'clusterSize' | 'numWorkers'
+>
+
 export interface User {
   jwt: string
   payload: { role?: string } & JWTPayload
@@ -263,7 +273,14 @@ export abstract class PoolManager<TPool extends PoolStrategy = PoolStrategy> {
       return existingPool as TPool
     }
 
-    const newPool = this.newPool({ ...settings, numWorkers: this.numWorkers })
+    const newPool = this.newPool({
+      tenantId: settings.tenantId,
+      dbUrl: settings.dbUrl,
+      isExternalPool: settings.isExternalPool,
+      maxConnections: settings.maxConnections,
+      clusterSize: settings.clusterSize,
+      numWorkers: this.numWorkers,
+    })
 
     tenantPools.set(settings.tenantId, newPool)
     return newPool
@@ -296,5 +313,5 @@ export abstract class PoolManager<TPool extends PoolStrategy = PoolStrategy> {
     return Promise.allSettled(promises)
   }
 
-  protected abstract newPool(settings: TenantConnectionOptions): TPool
+  protected abstract newPool(settings: PoolStrategySettings): TPool
 }

src/test/pg-connection.test.ts

@@ -4,15 +4,16 @@ import {
   PgPoolStrategy,
   PgTenantConnection,
 } from '@internal/database'
+import type { TenantConnectionOptions } from '@internal/database/pool'
 import { getConfig } from '../config'
 
 const { databaseURL, databasePoolURL, tenantId } = getConfig()
 
 describe('Pg database foundation', () => {
   function createConnectionSettings(
     superUser: Awaited<ReturnType<typeof getServiceKeyUser>>,
-    overrides?: Partial<ConstructorParameters<typeof PgPoolStrategy>[0]>
-  ): ConstructorParameters<typeof PgPoolStrategy>[0] {
+    overrides?: Partial<TenantConnectionOptions>
+  ): TenantConnectionOptions {
     return {
       tenantId,
       isExternalPool: true,

src/test/storage-pg-db.test.ts

@@ -7,6 +7,7 @@ import {
   PgTransaction,
 } from '@internal/database'
 import { runMigrationsOnTenant } from '@internal/database/migrations'
+import type { TenantConnectionOptions } from '@internal/database/pool'
 import { logger, logSchema } from '@internal/monitoring'
 import { dbQueryPerformance } from '@internal/monitoring/metrics'
 import { StoragePgDB } from '@storage/database'
@@ -22,7 +23,7 @@ describe('StoragePgDB bucket metadata', () => {
   let db: StoragePgDB
   let runId: string
   let superUser: Awaited<ReturnType<typeof getServiceKeyUser>>
-  let connectionSettings: ConstructorParameters<typeof PgPoolStrategy>[0]
+  let connectionSettings: TenantConnectionOptions
 
   beforeAll(async () => {
     await runMigrationsOnTenant({

src/test/vector-pg-store.test.ts

@@ -5,6 +5,7 @@ import {
   PgTransaction,
 } from '@internal/database'
 import { runMigrationsOnTenant } from '@internal/database/migrations'
+import type { TenantConnectionOptions } from '@internal/database/pool'
 import { logger, logSchema } from '@internal/monitoring'
 import { PgVectorMetadataDB } from '@storage/protocols/vector'
 import { DatabaseError, type PoolClient } from 'pg'
@@ -25,14 +26,15 @@ describe('PgVectorMetadataDB', () => {
     })
 
     const superUser = await getServiceKeyUser(tenantId)
-    pool = new PgPoolStrategy({
+    const connectionSettings: TenantConnectionOptions = {
       tenantId,
       dbUrl: databaseURL!,
       isExternalPool: false,
       maxConnections: 2,
       user: superUser,
       superUser,
-    })
+    }
+    pool = new PgPoolStrategy(connectionSettings)
     db = new PgVectorMetadataDB(pool.acquire())
   })