Skip to content

Commit 5e683d1

Browse files
fix(integ): bound credential scans and classify disabled TTL
1 parent d7ef91a commit 5e683d1

7 files changed

Lines changed: 184 additions & 67 deletions

File tree

apps/sim/lib/credentials/deletion.test.ts

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,16 @@
1515
* which cannot express a subquery), and `@sim/db` is a `drizzle-orm/pg-proxy`
1616
* client whose driver captures the compiled statement and replays the rows
1717
* Postgres would return for the scenario under test.
18+
*
19+
* Credential-reference scans also use the real query builder to verify that
20+
* workspace filtering stays inside a materialized boundary before JSON search.
1821
*/
1922
import { drizzle } from 'drizzle-orm/pg-proxy'
2023
import { beforeEach, describe, expect, it, vi } from 'vitest'
2124

2225
const { capturedQueries, driverRows, mockLogger } = vi.hoisted(() => ({
2326
capturedQueries: [] as { sql: string; params: unknown[] }[],
24-
driverRows: { value: [] as unknown[] },
27+
driverRows: { value: [] as unknown[], error: null as Error | null },
2528
mockLogger: {
2629
info: vi.fn(),
2730
warn: vi.fn(),
@@ -38,11 +41,12 @@ vi.mock('@sim/logger', () => ({ createLogger: () => mockLogger }))
3841
vi.mock('@sim/db', () => ({
3942
db: drizzle(async (sql: string, params: unknown[]) => {
4043
capturedQueries.push({ sql, params })
44+
if (driverRows.error) throw driverRows.error
4145
return { rows: driverRows.value }
4246
}),
4347
}))
4448

45-
import { deleteOrphanedOAuthAccount } from '@/lib/credentials/deletion'
49+
import { clearCredentialRefs, deleteOrphanedOAuthAccount } from '@/lib/credentials/deletion'
4650

4751
const ACCOUNT_ID = 'acct-bob-google'
4852

@@ -64,13 +68,14 @@ function guardSubquery(sql: string): string {
6468
return match[1]
6569
}
6670

67-
describe('deleteOrphanedOAuthAccount', () => {
68-
beforeEach(() => {
69-
capturedQueries.length = 0
70-
driverRows.value = []
71-
vi.clearAllMocks()
72-
})
71+
beforeEach(() => {
72+
capturedQueries.length = 0
73+
driverRows.value = []
74+
driverRows.error = null
75+
vi.clearAllMocks()
76+
})
7377

78+
describe('deleteOrphanedOAuthAccount', () => {
7479
it('guards the account delete with a reference check against the credential table', async () => {
7580
await deleteOrphanedOAuthAccount(ACCOUNT_ID)
7681

@@ -137,3 +142,65 @@ describe('deleteOrphanedOAuthAccount', () => {
137142
expect(sql).not.toContain('provider_id')
138143
})
139144
})
145+
146+
describe('clearCredentialRefs', () => {
147+
const sources = [
148+
['workflow_blocks', 'sub_blocks'],
149+
['workflow_deployment_version', 'state'],
150+
['paused_executions', 'execution_snapshot'],
151+
['workflow_checkpoints', 'workflow_state'],
152+
] as const
153+
154+
it('scopes every snapshot scan before converting JSON to text, including archived workflows', async () => {
155+
await clearCredentialRefs('credential-target', 'workspace-target')
156+
157+
const reads = capturedQueries.filter((query) => normalizeSql(query.sql).startsWith('WITH'))
158+
expect(reads).toHaveLength(sources.length)
159+
for (const [table, column] of sources) {
160+
const query = reads.find((query) => query.sql.includes(`FROM "${table}"`))
161+
expect(query).toBeDefined()
162+
const statement = normalizeSql(query!.sql)
163+
expect(statement).toContain(
164+
`WITH workspace_credential_refs AS MATERIALIZED ( SELECT "${table}"."id" AS id, "${table}"."${column}" AS value FROM "${table}" INNER JOIN "workflow" ON "workflow"."id" = "${table}"."workflow_id" WHERE "workflow"."workspace_id" = $1 ) SELECT id, value FROM workspace_credential_refs WHERE value::text LIKE $2`
165+
)
166+
expect(statement).not.toContain('deleted_at')
167+
expect(query!.params).toEqual(['workspace-target', '%credential-target%'])
168+
}
169+
})
170+
171+
it('clears matching references returned by every raw scan and preserves other values', async () => {
172+
driverRows.value = [
173+
{
174+
id: 'snapshot-target',
175+
value: {
176+
blocks: [{ id: 'credential', value: 'credential-target' }],
177+
params: { credential: 'credential-target', other: 'credential-other' },
178+
name: 'credential-target',
179+
},
180+
},
181+
{ id: 'substring-only', value: { name: 'credential-target' } },
182+
]
183+
184+
await clearCredentialRefs('credential-target', 'workspace-target')
185+
186+
for (const [table] of sources) {
187+
const updates = capturedQueries.filter((query) => query.sql.startsWith(`update "${table}"`))
188+
expect(updates).toHaveLength(1)
189+
expect(JSON.parse(updates[0].params[0] as string)).toEqual({
190+
blocks: [{ id: 'credential', value: '' }],
191+
params: { credential: '', other: 'credential-other' },
192+
name: 'credential-target',
193+
})
194+
expect(updates[0].params.at(-1)).toBe('snapshot-target')
195+
}
196+
})
197+
198+
it('propagates database failures', async () => {
199+
driverRows.error = new Error('database unavailable')
200+
await expect(
201+
clearCredentialRefs('credential-target', 'workspace-target')
202+
).rejects.toMatchObject({
203+
cause: driverRows.error,
204+
})
205+
})
206+
})

apps/sim/lib/credentials/deletion.ts

Lines changed: 52 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { db } from '@sim/db'
33
import * as schema from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
55
import { and, eq, notExists, or, sql } from 'drizzle-orm'
6+
import type { AnyPgColumn, PgTable } from 'drizzle-orm/pg-core'
67
import type { NextRequest } from 'next/server'
78
import {
89
type ResourceOwner,
@@ -216,23 +217,16 @@ async function clearInWorkflowBlocks(
216217
workspaceId: string,
217218
needle: string
218219
): Promise<void> {
219-
const rows = await db
220-
.select({
221-
id: schema.workflowBlocks.id,
222-
subBlocks: schema.workflowBlocks.subBlocks,
223-
})
224-
.from(schema.workflowBlocks)
225-
.innerJoin(schema.workflow, eq(schema.workflow.id, schema.workflowBlocks.workflowId))
226-
.where(
227-
and(
228-
eq(schema.workflow.workspaceId, workspaceId),
229-
sql`${schema.workflowBlocks.subBlocks}::text LIKE ${needle}`
230-
)
231-
)
220+
const rows = await readWorkspaceCredentialRefs(workspaceId, needle, {
221+
table: schema.workflowBlocks,
222+
id: schema.workflowBlocks.id,
223+
workflowId: schema.workflowBlocks.workflowId,
224+
value: schema.workflowBlocks.subBlocks,
225+
})
232226

233227
let updated = 0
234228
for (const row of rows) {
235-
const next = clearCredentialInValue(row.subBlocks, credentialId)
229+
const next = clearCredentialInValue(row.value, credentialId)
236230
if (next.changed) {
237231
await db
238232
.update(schema.workflowBlocks)
@@ -255,22 +249,15 @@ async function clearInDeploymentVersions(
255249
workspaceId: string,
256250
needle: string
257251
): Promise<void> {
258-
const rows = await db
259-
.select({
260-
id: schema.workflowDeploymentVersion.id,
261-
state: schema.workflowDeploymentVersion.state,
262-
})
263-
.from(schema.workflowDeploymentVersion)
264-
.innerJoin(schema.workflow, eq(schema.workflow.id, schema.workflowDeploymentVersion.workflowId))
265-
.where(
266-
and(
267-
eq(schema.workflow.workspaceId, workspaceId),
268-
sql`${schema.workflowDeploymentVersion.state}::text LIKE ${needle}`
269-
)
270-
)
252+
const rows = await readWorkspaceCredentialRefs(workspaceId, needle, {
253+
table: schema.workflowDeploymentVersion,
254+
id: schema.workflowDeploymentVersion.id,
255+
workflowId: schema.workflowDeploymentVersion.workflowId,
256+
value: schema.workflowDeploymentVersion.state,
257+
})
271258

272259
for (const row of rows) {
273-
const next = clearCredentialInValue(row.state, credentialId)
260+
const next = clearCredentialInValue(row.value, credentialId)
274261
if (next.changed) {
275262
await db
276263
.update(schema.workflowDeploymentVersion)
@@ -285,22 +272,15 @@ async function clearInPausedExecutions(
285272
workspaceId: string,
286273
needle: string
287274
): Promise<void> {
288-
const rows = await db
289-
.select({
290-
id: schema.pausedExecutions.id,
291-
executionSnapshot: schema.pausedExecutions.executionSnapshot,
292-
})
293-
.from(schema.pausedExecutions)
294-
.innerJoin(schema.workflow, eq(schema.workflow.id, schema.pausedExecutions.workflowId))
295-
.where(
296-
and(
297-
eq(schema.workflow.workspaceId, workspaceId),
298-
sql`${schema.pausedExecutions.executionSnapshot}::text LIKE ${needle}`
299-
)
300-
)
275+
const rows = await readWorkspaceCredentialRefs(workspaceId, needle, {
276+
table: schema.pausedExecutions,
277+
id: schema.pausedExecutions.id,
278+
workflowId: schema.pausedExecutions.workflowId,
279+
value: schema.pausedExecutions.executionSnapshot,
280+
})
301281

302282
for (const row of rows) {
303-
const next = clearCredentialInValue(row.executionSnapshot, credentialId)
283+
const next = clearCredentialInValue(row.value, credentialId)
304284
if (next.changed) {
305285
await db
306286
.update(schema.pausedExecutions)
@@ -315,22 +295,15 @@ async function clearInWorkflowCheckpoints(
315295
workspaceId: string,
316296
needle: string
317297
): Promise<void> {
318-
const rows = await db
319-
.select({
320-
id: schema.workflowCheckpoints.id,
321-
workflowState: schema.workflowCheckpoints.workflowState,
322-
})
323-
.from(schema.workflowCheckpoints)
324-
.innerJoin(schema.workflow, eq(schema.workflow.id, schema.workflowCheckpoints.workflowId))
325-
.where(
326-
and(
327-
eq(schema.workflow.workspaceId, workspaceId),
328-
sql`${schema.workflowCheckpoints.workflowState}::text LIKE ${needle}`
329-
)
330-
)
298+
const rows = await readWorkspaceCredentialRefs(workspaceId, needle, {
299+
table: schema.workflowCheckpoints,
300+
id: schema.workflowCheckpoints.id,
301+
workflowId: schema.workflowCheckpoints.workflowId,
302+
value: schema.workflowCheckpoints.workflowState,
303+
})
331304

332305
for (const row of rows) {
333-
const next = clearCredentialInValue(row.workflowState, credentialId)
306+
const next = clearCredentialInValue(row.value, credentialId)
334307
if (next.changed) {
335308
await db
336309
.update(schema.workflowCheckpoints)
@@ -340,6 +313,29 @@ async function clearInWorkflowCheckpoints(
340313
}
341314
}
342315

316+
/**
317+
* Restrict the rows before inspecting their JSON. With a plain join, Postgres can push
318+
* the text predicate below the workspace join and detoast every tenant's snapshots.
319+
* This query has reached 46s in production. Materializing the workspace selection
320+
* keeps the expensive scan local, including archived workflows
321+
* whose frozen snapshots still need their credential references removed.
322+
*/
323+
async function readWorkspaceCredentialRefs(
324+
workspaceId: string,
325+
needle: string,
326+
source: { table: PgTable; id: AnyPgColumn; workflowId: AnyPgColumn; value: AnyPgColumn }
327+
): Promise<Array<{ id: string; value: unknown }>> {
328+
return db.execute<{ id: string; value: unknown }>(sql`
329+
WITH workspace_credential_refs AS MATERIALIZED (
330+
SELECT ${source.id} AS id, ${source.value} AS value
331+
FROM ${source.table}
332+
INNER JOIN ${schema.workflow} ON ${schema.workflow.id} = ${source.workflowId}
333+
WHERE ${schema.workflow.workspaceId} = ${workspaceId}
334+
)
335+
SELECT id, value FROM workspace_credential_refs WHERE value::text LIKE ${needle}
336+
`)
337+
}
338+
343339
async function clearInKnowledgeConnectors(credentialId: string): Promise<void> {
344340
await db
345341
.update(schema.knowledgeConnector)

apps/sim/lib/table/api/route-policies.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { generateInternalDelegationToken, generateInternalToken } from '@/lib/au
3030
import { OrchestrationError } from '@/lib/core/orchestration/types'
3131
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
3232
import { v2TableErrorPolicies } from '@/lib/table/api/route-policies'
33+
import { TableRowTtlDisabledError } from '@/lib/table/errors'
3334

3435
afterAll(resetEnvMock)
3536

@@ -187,4 +188,24 @@ describe('internal Table route authentication', () => {
187188
error: { code: 'BAD_REQUEST', message: 'Invalid workflow ID' },
188189
})
189190
})
191+
192+
it.each([false, true])(
193+
'preserves the TTL-disabled reason code (wrapped: %s)',
194+
async (wrapped) => {
195+
const error = new TableRowTtlDisabledError()
196+
error.message = 'TTL support is turned off'
197+
const response = v2TableErrorPolicies.default.render(
198+
wrapped ? new Error('operation failed', { cause: error }) : error
199+
)
200+
201+
expect(response.status).toBe(400)
202+
await expect(response.json()).resolves.toEqual({
203+
error: {
204+
code: 'BAD_REQUEST',
205+
message: 'TTL support is turned off',
206+
details: { code: 'TABLE_ROW_TTL_DISABLED' },
207+
},
208+
})
209+
}
210+
)
190211
})

apps/sim/lib/table/api/route-policies.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@ import {
77
internalOrchestrationErrorPolicy,
88
type V2ErrorPolicy,
99
} from '@/lib/api/server/routes'
10+
import { asOrchestrationError } from '@/lib/core/orchestration/types'
1011
import { TABLE_DELEGATION_AUDIENCE } from '@/lib/table/application/authorization'
1112
import { TableOperationError } from '@/lib/table/application/errors'
13+
import { TableRowTtlDisabledError } from '@/lib/table/errors'
1214
import { TableLockedError } from '@/lib/table/mutation-locks'
1315
import {
1416
v2CaughtOrchestrationError,
@@ -25,6 +27,12 @@ export const internalTableSessionOrExecutorAuth = createInternalSessionOrExecuto
2527
})
2628

2729
function renderTableError(error: unknown) {
30+
const classified = asOrchestrationError(error)
31+
if (classified instanceof TableRowTtlDisabledError) {
32+
return v2Error('BAD_REQUEST', classified.message, {
33+
details: { code: classified.detailCode },
34+
})
35+
}
2836
if (error instanceof TableOperationError) {
2937
return v2ErrorForOrchestration(
3038
error.code,

apps/sim/lib/table/errors.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
import { OrchestrationError } from '@/lib/core/orchestration/types'
2+
3+
/** A disabled TTL feature, distinct from malformed column input. */
4+
export class TableRowTtlDisabledError extends OrchestrationError {
5+
readonly detailCode = 'TABLE_ROW_TTL_DISABLED'
6+
7+
constructor() {
8+
super('validation', 'Expiration columns are not enabled')
9+
this.name = 'TableRowTtlDisabledError'
10+
}
11+
}
12+
113
/**
214
* Stable, machine-readable codes for table query failures. SDKs and clients
315
* branch on these instead of string-matching human-facing messages.

apps/sim/lib/table/ttl-availability.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,20 @@ describe('table row TTL availability', () => {
2828

2929
await expect(assertTableRowTtlEnabled()).rejects.toMatchObject({
3030
code: 'validation',
31-
message: 'Expiration columns are not enabled',
31+
detailCode: 'TABLE_ROW_TTL_DISABLED',
3232
})
3333
})
34+
35+
it('allows TTL column creation while the flag is enabled', async () => {
36+
mockIsFeatureEnabled.mockResolvedValue(true)
37+
38+
await expect(assertTableRowTtlEnabled()).resolves.toBeUndefined()
39+
})
40+
41+
it('propagates flag lookup failures instead of reporting the feature as disabled', async () => {
42+
const error = new Error('flag service unavailable')
43+
mockIsFeatureEnabled.mockRejectedValue(error)
44+
45+
await expect(assertTableRowTtlEnabled()).rejects.toBe(error)
46+
})
3447
})
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { isFeatureEnabled } from '@/lib/core/config/feature-flags'
2-
import { OrchestrationError } from '@/lib/core/orchestration/types'
2+
import { TableRowTtlDisabledError } from '@/lib/table/errors'
33

44
/** Whether TTL columns and their cleanup behavior are enabled globally. */
55
export function isTableRowTtlEnabled(): Promise<boolean> {
@@ -9,5 +9,5 @@ export function isTableRowTtlEnabled(): Promise<boolean> {
99
/** Rejects attempts to introduce a TTL column while the feature is disabled. */
1010
export async function assertTableRowTtlEnabled(): Promise<void> {
1111
if (await isTableRowTtlEnabled()) return
12-
throw new OrchestrationError('validation', 'Expiration columns are not enabled')
12+
throw new TableRowTtlDisabledError()
1313
}

0 commit comments

Comments
 (0)