From cf599d05072ac6ba24c6b3e78c3875eff6c8e5dc Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Tue, 28 Jul 2026 11:33:50 -0700 Subject: [PATCH] fix(cleanup): bind array params as arrays, not expanded value lists --- .../payloads/large-value-metadata.ts | 6 +- .../payloads/prune-metadata-sql.test.ts | 63 +++++++++++++++++++ apps/sim/lib/webhooks/polling/utils.test.ts | 23 ++++--- apps/sim/lib/webhooks/polling/utils.ts | 3 +- packages/testing/src/mocks/database.mock.ts | 7 +++ 5 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.ts b/apps/sim/lib/execution/payloads/large-value-metadata.ts index c98f4756c5e..6fedd8d7c63 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.ts @@ -396,7 +396,7 @@ async function pruneStaleReferences( WHERE ref.ctid IN ( SELECT ref.ctid FROM ${executionLargeValueReferences} AS ref - WHERE ref.workspace_id = ANY(${workspaceIds}::text[]) + WHERE ref.workspace_id = ANY(${sql.param(workspaceIds)}::text[]) AND ( ( ref.source = 'execution_log' @@ -437,7 +437,7 @@ async function pruneDeletedParentDependencies( WHERE dependency.ctid IN ( SELECT dependency.ctid FROM ${executionLargeValueDependencies} AS dependency - WHERE dependency.workspace_id = ANY(${workspaceIds}::text[]) + WHERE dependency.workspace_id = ANY(${sql.param(workspaceIds)}::text[]) AND ( EXISTS ( SELECT 1 @@ -472,7 +472,7 @@ async function pruneDeletedLargeValueTombstones( WHERE value.ctid IN ( SELECT value.ctid FROM ${executionLargeValues} AS value - WHERE value.workspace_id = ANY(${workspaceIds}::text[]) + WHERE value.workspace_id = ANY(${sql.param(workspaceIds)}::text[]) AND value.deleted_at IS NOT NULL AND value.deleted_at < ${deletedBefore} AND NOT EXISTS ( diff --git a/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts b/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts new file mode 100644 index 00000000000..0d5daa27e0a --- /dev/null +++ b/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts @@ -0,0 +1,63 @@ +/** + * @vitest-environment node + */ + +// Renders the real prune statements against the real drizzle dialect and +// schema. These are raw `sql` templates, so a rendering bug only surfaces at +// execution time — the global drizzle/schema mocks would hide it entirely. +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 { PgDialect } = await import('drizzle-orm/pg-core') +const { pruneLargeValueMetadata } = await import('@/lib/execution/payloads/large-value-metadata') + +/** Captures the raw statements `pruneLargeValueMetadata` issues, without a database. */ +async function renderPruneStatements(workspaceIds: string[]): Promise { + const dialect = new PgDialect() + const rendered: string[] = [] + const capturingClient = { + execute: async (query: Parameters[0]) => { + rendered.push(dialect.sqlToQuery(query).sql) + return [{ count: 0 }] + }, + } + + await pruneLargeValueMetadata({ + workspaceIds, + tombstonesDeletedBefore: new Date('2026-01-01T00:00:00Z'), + dbClient: capturingClient as unknown as Parameters< + typeof pruneLargeValueMetadata + >[0]['dbClient'], + }) + + return rendered +} + +describe('pruneLargeValueMetadata SQL', () => { + it('binds workspace ids as one array parameter, not a value list', async () => { + const statements = await renderPruneStatements(['ws-1', 'ws-2']) + + expect(statements.length).toBeGreaterThan(0) + for (const statement of statements) { + // `ANY(($1, $2)::text[])` is what interpolating the raw JS array yields — + // Postgres rejects it ("cannot cast type record to text[]"), and with a + // single id `ANY(($1)::text[])` fails as 22P02 instead. + expect(statement).not.toMatch(/ANY\(\(\$\d+/) + expect(statement).toMatch(/ANY\(\$\d+::text\[\]\)/) + } + }) + + it('renders identically for a single workspace id', async () => { + const statements = await renderPruneStatements(['ws-only']) + + expect(statements.length).toBeGreaterThan(0) + for (const statement of statements) { + expect(statement).toMatch(/ANY\(\$\d+::text\[\]\)/) + } + }) +}) diff --git a/apps/sim/lib/webhooks/polling/utils.test.ts b/apps/sim/lib/webhooks/polling/utils.test.ts index 48deafe868d..7e988dcca11 100644 --- a/apps/sim/lib/webhooks/polling/utils.test.ts +++ b/apps/sim/lib/webhooks/polling/utils.test.ts @@ -9,18 +9,23 @@ const { sqlCalls } = vi.hoisted(() => ({ sqlCalls: [] as Array<{ strings: readonly string[]; values: unknown[] }>, })) -vi.mock('drizzle-orm', () => ({ - sql: (strings: readonly string[], ...values: unknown[]) => { +vi.mock('drizzle-orm', () => { + const sql = (strings: readonly string[], ...values: unknown[]) => { const node = { strings, values } sqlCalls.push(node) return node - }, - and: vi.fn(), - eq: vi.fn((field: unknown, value: unknown) => ({ field, value })), - isNull: vi.fn(), - ne: vi.fn(), - or: vi.fn(), -})) + } + // Identity, so a `sql.param(arr)` value still shows up verbatim in `values`. + sql.param = (value: unknown) => value + return { + sql, + and: vi.fn(), + eq: vi.fn((field: unknown, value: unknown) => ({ field, value })), + isNull: vi.fn(), + ne: vi.fn(), + or: vi.fn(), + } +}) vi.mock('@/app/api/auth/oauth/utils', () => ({ getOAuthToken: vi.fn(), refreshAccessTokenIfNeeded: vi.fn(), diff --git a/apps/sim/lib/webhooks/polling/utils.ts b/apps/sim/lib/webhooks/polling/utils.ts index efab00fc046..32a9c82a111 100644 --- a/apps/sim/lib/webhooks/polling/utils.ts +++ b/apps/sim/lib/webhooks/polling/utils.ts @@ -164,7 +164,8 @@ export async function updateWebhookProviderConfig( } const merged = sql`COALESCE(${webhook.providerConfig}::jsonb, '{}'::jsonb) || ${JSON.stringify(defined)}::jsonb` - const nextConfig = removedKeys.length > 0 ? sql`(${merged}) - ${removedKeys}::text[]` : merged + const nextConfig = + removedKeys.length > 0 ? sql`(${merged}) - ${sql.param(removedKeys)}::text[]` : merged await db .update(webhook) diff --git a/packages/testing/src/mocks/database.mock.ts b/packages/testing/src/mocks/database.mock.ts index f9699300af2..d9e405b5cd0 100644 --- a/packages/testing/src/mocks/database.mock.ts +++ b/packages/testing/src/mocks/database.mock.ts @@ -25,6 +25,13 @@ 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] }), + }) + return sqlFn }