fix(db): encode Date binds in raw sql, document the fetch_types contract

Author: TheodoreSpeaks

apps/sim/app/api/v1/logs/filters.ts

@@ -32,11 +32,11 @@ export function buildLogFilters(filters: LogFilters): SQL<unknown> {
     const cursorDate = new Date(filters.cursor.startedAt)
     if (filters.order === 'desc') {
       conditions.push(
-        sql`(${workflowExecutionLogs.startedAt}, ${workflowExecutionLogs.id}) < (${cursorDate}, ${filters.cursor.id})`
+        sql`(${workflowExecutionLogs.startedAt}, ${workflowExecutionLogs.id}) < (${sql.param(cursorDate, workflowExecutionLogs.startedAt)}, ${filters.cursor.id})`
       )
     } else {
       conditions.push(
-        sql`(${workflowExecutionLogs.startedAt}, ${workflowExecutionLogs.id}) > (${cursorDate}, ${filters.cursor.id})`
+        sql`(${workflowExecutionLogs.startedAt}, ${workflowExecutionLogs.id}) > (${sql.param(cursorDate, workflowExecutionLogs.startedAt)}, ${filters.cursor.id})`
       )
     }
   }

apps/sim/lib/execution/payloads/large-value-metadata.ts

@@ -390,9 +390,9 @@ async function pruneStaleReferences(
   batchSize: number,
   dbClient: LargeValueMetadataClient
 ): Promise<number> {
-  // `IN ()` is a syntax error; callers chunk a non-empty list, but never rely on that.
+  // Empty input is a valid no-op, and `IN ()` is a syntax error whose failure the
+  // cleanup job swallows — keep these total rather than relying on the caller.
   if (workspaceIds.length === 0) return 0
-
   const rows = await dbClient.execute<{ count: number }>(sql`
     WITH deleted AS (
       DELETE FROM ${executionLargeValueReferences} AS ref
@@ -434,9 +434,9 @@ async function pruneDeletedParentDependencies(
   batchSize: number,
   dbClient: LargeValueMetadataClient
 ): Promise<number> {
-  // `IN ()` is a syntax error; callers chunk a non-empty list, but never rely on that.
+  // Empty input is a valid no-op, and `IN ()` is a syntax error whose failure the
+  // cleanup job swallows — keep these total rather than relying on the caller.
   if (workspaceIds.length === 0) return 0
-
   const rows = await dbClient.execute<{ count: number }>(sql`
     WITH deleted AS (
       DELETE FROM ${executionLargeValueDependencies} AS dependency
@@ -472,9 +472,9 @@ async function pruneDeletedLargeValueTombstones(
   batchSize: number,
   dbClient: LargeValueMetadataClient
 ): Promise<number> {
-  // `IN ()` is a syntax error; callers chunk a non-empty list, but never rely on that.
+  // Empty input is a valid no-op, and `IN ()` is a syntax error whose failure the
+  // cleanup job swallows — keep these total rather than relying on the caller.
   if (workspaceIds.length === 0) return 0
-
   const rows = await dbClient.execute<{ count: number }>(sql`
     WITH deleted AS (
       DELETE FROM ${executionLargeValues} AS value
@@ -483,7 +483,7 @@ async function pruneDeletedLargeValueTombstones(
         FROM ${executionLargeValues} AS value
         WHERE value.workspace_id IN ${workspaceIds}
           AND value.deleted_at IS NOT NULL
-          AND value.deleted_at < ${deletedBefore}
+          AND value.deleted_at < ${sql.param(deletedBefore, executionLargeValues.deletedAt)}
           AND NOT EXISTS (
             SELECT 1
             FROM ${executionLargeValueDependencies} AS dependency

apps/sim/lib/execution/payloads/live-paused-statuses-sql.test.ts

@@ -37,4 +37,13 @@ describe('unreferencedLargeValuePredicate SQL', () => {
     // `status = IN (...)` is a syntax error Postgres only reports at execution time.
     expect(text).not.toMatch(/=\s*IN\s*\(/)
   })
+
+  it('never binds an array as a parameter', () => {
+    // The invariant that matters: the app pools set `fetch_types: false`, so an
+    // array bound as one parameter fails at execution even though the rendered
+    // SQL (`ANY($1::text[])`) is valid. Only the params can reveal it.
+    for (const param of params) {
+      expect(Array.isArray(param)).toBe(false)
+    }
+  })
 })

apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts

@@ -67,12 +67,16 @@ describe('pruneLargeValueMetadata SQL', () => {
         }
       })
 
-      it('never binds an array as a parameter', async () => {
+      it('binds only scalar parameters', async () => {
         const statements = await renderPruneStatements([...ids])
 
         for (const { params } of statements) {
           for (const param of params) {
+            // Arrays fail under fetch_types: false (no serializer, 22P02); Date
+            // objects fail in postgres-js's unsafe path outright
+            // (ERR_INVALID_ARG_TYPE) — both must be pre-encoded to strings.
             expect(Array.isArray(param)).toBe(false)
+            expect(param instanceof Date).toBe(false)
           }
         }
       })

apps/sim/lib/webhooks/polling/config-sql.test.ts

@@ -0,0 +1,54 @@
+/**
+ * @vitest-environment node
+ */
+
+// Renders the real key-removal expression against the real drizzle dialect.
+// utils.test.ts mocks drizzle wholesale, so its assertions passed against BOTH
+// previously-broken forms (`- ${keys}::text[]` and `- ${sql.param(keys)}::text[]`).
+// Only a real render can tell them apart, and only the params reveal an array bind:
+// the app pools set `fetch_types: false` (packages/db/db.ts), which leaves
+// postgres-js unable to serialize an array bound as a single parameter.
+import { describe, expect, it, vi } from 'vitest'
+
+vi.unmock('drizzle-orm')
+vi.unmock('@sim/db')
+vi.unmock('@sim/db/schema')
+
+process.env.DATABASE_URL ??= 'postgresql://user:pass@localhost:5432/test'
+
+const { sql } = await import('drizzle-orm')
+const { PgDialect } = await import('drizzle-orm/pg-core')
+
+/** Mirrors the expression built in `updateWebhookProviderConfig`. */
+function renderKeyRemoval(removedKeys: string[]) {
+  const merged = sql`COALESCE("provider_config"::jsonb, '{}'::jsonb) || ${JSON.stringify({ a: 1 })}::jsonb`
+  return new PgDialect().sqlToQuery(
+    sql`(${merged}) - ARRAY[${sql.join(
+      removedKeys.map((key) => sql`${key}`),
+      sql`, `
+    )}]::text[]`
+  )
+}
+
+describe('provider-config key removal SQL', () => {
+  // Production only ever removes one key (google-drive's pageToken reset), so the
+  // single-element case is the one that actually runs.
+  for (const keys of [['pageToken'], ['a', 'b'], ['a', 'b', 'c']]) {
+    it(`binds ${keys.length} key(s) as scalars inside an ARRAY constructor`, () => {
+      const { sql: text, params } = renderKeyRemoval(keys)
+
+      expect(text).toContain(`ARRAY[${keys.map((_, i) => `$${i + 2}`).join(', ')}]::text[]`)
+      expect(params).toEqual([JSON.stringify({ a: 1 }), ...keys])
+      for (const param of params) {
+        expect(Array.isArray(param)).toBe(false)
+      }
+    })
+  }
+
+  it('never renders a row constructor cast to text[]', () => {
+    // `- ($1, $2)::text[]` — what interpolating the raw JS array produced. Postgres
+    // rejects it: "cannot cast type record to text[]" (and 22P02 at one element).
+    const { sql: text } = renderKeyRemoval(['a', 'b'])
+    expect(text).not.toMatch(/\(\$\d+(, \$\d+)+\)::text\[\]/)
+  })
+})

packages/db/db.ts

@@ -43,18 +43,33 @@ if (!connectionString) {
 }
 
 /**
- * `fetch_types: false` skips postgres.js's `pg_catalog.pg_type` roundtrip, which
- * it otherwise runs on every new connection before that connection's first query.
- * It builds array parsers only — scalar types (including `jsonb` and pgvector)
- * are unaffected — and Drizzle already parses this schema's `text[]` columns
- * itself, so the fetch is pure connection-setup latency.
+ * `fetch_types: false` skips postgres.js's `pg_catalog.pg_type` roundtrip, which it
+ * otherwise runs on every new connection before that connection's first query.
  *
- * The one behavior this changes: a raw `db.execute` that projects an array-typed
- * column yields the wire form `'{a,b}'` rather than `['a','b']`, since nothing maps
- * the result. Verified by differential test that Drizzle-typed selects and
- * `.returning()` are unaffected — `PgArray.mapFromDriverValue` parses the wire form
- * itself, and writes never reach the array serializer because Drizzle serializes
- * arrays before binding.
+ * That roundtrip populates the array type map, which postgres.js uses in BOTH
+ * directions, for every array type — not just `text[]`. Disabling it has two
+ * consequences on every client built from these options (the primary, the replica,
+ * and every `dbFor()` sub-pool):
+ *
+ * 1. Reads: a raw `db.execute` projecting an array column yields the wire form
+ *    `'{a,b}'`, not `['a','b']`. Drizzle-typed selects and `.returning()` are
+ *    unaffected — `PgArray.mapFromDriverValue` parses the wire form itself.
+ * 2. Writes: a JS array bound as a SINGLE parameter fails at execution with
+ *    `22P02 Array value must start with "{"` — there is no serializer to build the
+ *    literal, and neither `prepare: false` nor `sql.array()` avoids it. Drizzle-typed
+ *    column writes ARE safe (`PgArray.mapToDriverValue` stringifies first), as are
+ *    `inArray`/`${jsArray}` in a drizzle `sql` template (both expand to scalar binds).
+ *    Raw binds are NOT: never write `sql.param(someArray)`. Pass an expanded list
+ *    (`IN ${ids}`) or an explicit `ARRAY[${sql.join(...)}]::text[]` of scalars.
+ *
+ * Scalar types — including `jsonb`, pgvector, enums and ranges — are unaffected.
+ *
+ * Note postgres.js's OWN `sql` tag has the opposite semantics: it binds `${array}`
+ * as one array parameter. `packages/db/scripts/migrate.ts` deliberately omits
+ * `fetch_types` for that reason; see the note there before sharing these options.
+ *
+ * Pinned by apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts, which renders
+ * the real statements and asserts no bind parameter is an array.
  */
 const poolOptions = {
   prepare: false,

packages/db/scripts/migrate.ts

@@ -45,6 +45,14 @@ const hasDirectMigrationUrl = Boolean(process.env.MIGRATION_DATABASE_URL)
  * `max_lifetime: null` pins the session for the whole run: the postgres-js
  * default recycles the connection after 30–60 min, silently dropping the
  * session advisory lock and `SET`s.
+ *
+ * Deliberately does NOT set `fetch_types: false`, unlike the app pools in
+ * `packages/db/db.ts`. Script migrations bind JS arrays directly through
+ * postgres-js's own `sql` tag (`= ANY(${ids}::text[])`, `unnest(${ids}::text[])`),
+ * and that binding is powered by the `pg_catalog.pg_type` fetch this option would
+ * skip. Copying the app's options here for consistency fails every registered
+ * script migration with `22P02`, aborting the migration container and blocking
+ * startup on every deploy and every fresh self-hosted install.
  */
 const client = postgres(url, {
   max: 1,

packages/testing/src/mocks/database.mock.ts

@@ -25,12 +25,28 @@ export function createMockSql() {
     }),
   })
 
-  // Binds a value as a single parameter — drizzle's escape hatch for passing an
-  // array to Postgres as an array rather than expanding it into a value list.
-  sqlFn.param = (value: any) => ({
-    value,
-    toSQL: () => ({ sql: '?', params: [value] }),
-  })
+  // Binds a value as a single parameter. Rejecting arrays here is the only place
+  // the unit suite can see this bug class: the app pools set `fetch_types: false`
+  // (packages/db/db.ts), which leaves postgres-js with no array serializer, so an
+  // array bound as ONE parameter fails at execution with `22P02`. Rendered SQL
+  // looks perfect (`ANY($1::text[])`), so no assertion on query text can catch it.
+  sqlFn.param = (value: any, encoder?: any) => {
+    if (encoder === undefined && Array.isArray(value)) {
+      throw new Error(
+        'sql.param(array) binds an array as one parameter, which fails under ' +
+          'fetch_types: false (packages/db/db.ts). Use `IN ${arr}` or ' +
+          'ARRAY[${sql.join(arr.map((v) => sql`${v}`), sql`, `)}]::text[].'
+      )
+    }
+    if (encoder === undefined && value instanceof Date) {
+      throw new Error(
+        'sql.param(date) without an encoder reaches postgres-js as a Date object, ' +
+          'which its unsafe path cannot serialize (ERR_INVALID_ARG_TYPE). Bind ' +
+          'through the matching column: sql.param(date, table.timestampColumn).'
+      )
+    }
+    return { value, toSQL: () => ({ sql: '?', params: [value] }) }
+  }
 
   return sqlFn
 }