Skip to content

Commit fc7aa66

Browse files
authored
fix(api): withhold internal failure messages from internal route responses (#7015)
An orchestration result carrying `errorCode: 'internal'` holds whatever text the fault happened to have — `workflow-lifecycle.ts` catch-alls return `toError(error).message`, which is the driver's failed SQL. Three application helpers projected that straight into an `OrchestrationError`, and the internal route policy rendered its message into a 500 body, so raw SQL reached clients. The v2 envelope already scrubbed the same failures; internal routes did not. `messageForOrchestrationError` already encoded the rule and two sites honored it. The three that hand-rolled it disagreed, and `workflow-vfs` disagreed with itself: it defaulted the code with `?? 'internal'` but compared the raw `errorCode` against `'internal'`, so an uncoded failure was classified internal and still rendered its own message. Pair the two in `throwOrchestrationFailure` so a code and its message cannot disagree, and scrub at the internal route boundary as well, matching v2 — no call site authors a curated `internal` message, so nothing legitimate is masked, and site N+1 cannot reopen this by forgetting the rule.
1 parent a20a546 commit fc7aa66

8 files changed

Lines changed: 150 additions & 22 deletions

File tree

apps/sim/lib/api/server/routes/internal-json-route.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ import {
3333
InvalidInternalDelegationBindingError,
3434
} from '@/lib/auth/internal-delegation'
3535
import type { ApplicationOperation, OperationUseCase } from '@/lib/core/application'
36-
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
36+
import {
37+
asOrchestrationError,
38+
messageForOrchestrationError,
39+
statusForOrchestrationError,
40+
} from '@/lib/core/orchestration/types'
3741
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
3842

3943
export class InternalUnauthenticatedError extends Error {
@@ -142,7 +146,10 @@ export const internalOrchestrationErrorPolicy: InternalErrorPolicy = {
142146
const classified = asOrchestrationError(error)
143147
if (!classified) return null
144148
return internalErrorResponse(statusForOrchestrationError(classified.code), {
145-
error: classified.message,
149+
error: messageForOrchestrationError(
150+
{ error: classified.message, errorCode: classified.code },
151+
'Internal server error'
152+
),
146153
})
147154
},
148155
unhandled() {

apps/sim/lib/core/orchestration/types.test.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,15 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5-
import { statusForOrchestrationError } from '@/lib/core/orchestration/types'
5+
import {
6+
messageForOrchestrationError,
7+
OrchestrationError,
8+
statusForOrchestrationError,
9+
throwOrchestrationFailure,
10+
} from '@/lib/core/orchestration/types'
11+
12+
const RAW_DRIVER_MESSAGE =
13+
'insert into "workflow" ("id") values ($1) - duplicate key value violates unique constraint "workflow_pkey"'
614

715
describe('statusForOrchestrationError', () => {
816
it.each([
@@ -15,3 +23,62 @@ describe('statusForOrchestrationError', () => {
1523
expect(statusForOrchestrationError(code)).toBe(expected)
1624
})
1725
})
26+
27+
describe('messageForOrchestrationError', () => {
28+
it('withholds the message of an explicitly internal failure', () => {
29+
expect(
30+
messageForOrchestrationError(
31+
{ error: RAW_DRIVER_MESSAGE, errorCode: 'internal' },
32+
'Failed to create workflow'
33+
)
34+
).toBe('Failed to create workflow')
35+
})
36+
37+
it('withholds the message of a failure carrying no code', () => {
38+
expect(
39+
messageForOrchestrationError({ error: RAW_DRIVER_MESSAGE }, 'Failed to create workflow')
40+
).toBe('Failed to create workflow')
41+
})
42+
43+
it('returns a classified failure message to the caller', () => {
44+
expect(
45+
messageForOrchestrationError(
46+
{ error: 'Workflow name is already taken', errorCode: 'conflict' },
47+
'Failed to create workflow'
48+
)
49+
).toBe('Workflow name is already taken')
50+
})
51+
52+
it('falls back when a classified failure carries no message', () => {
53+
expect(
54+
messageForOrchestrationError({ errorCode: 'conflict' }, 'Failed to create workflow')
55+
).toBe('Failed to create workflow')
56+
})
57+
})
58+
59+
describe('throwOrchestrationFailure', () => {
60+
it('classifies an uncoded failure as internal without rendering its message', () => {
61+
try {
62+
throwOrchestrationFailure({ error: RAW_DRIVER_MESSAGE }, 'Failed to update workflow')
63+
expect.unreachable('expected throwOrchestrationFailure to throw')
64+
} catch (error) {
65+
expect(error).toBeInstanceOf(OrchestrationError)
66+
expect((error as OrchestrationError).code).toBe('internal')
67+
expect((error as OrchestrationError).message).toBe('Failed to update workflow')
68+
}
69+
})
70+
71+
it('preserves the code and message of a classified failure', () => {
72+
try {
73+
throwOrchestrationFailure(
74+
{ error: 'No such workflow', errorCode: 'not_found' },
75+
'Failed to delete workflow'
76+
)
77+
expect.unreachable('expected throwOrchestrationFailure to throw')
78+
} catch (error) {
79+
expect(error).toBeInstanceOf(OrchestrationError)
80+
expect((error as OrchestrationError).code).toBe('not_found')
81+
expect((error as OrchestrationError).message).toBe('No such workflow')
82+
}
83+
})
84+
})

apps/sim/lib/core/orchestration/types.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,25 @@ export class OrchestrationError extends Error {
7272
}
7373
}
7474

75+
/**
76+
* Rethrows a failed orchestration result as its classified {@link OrchestrationError}.
77+
*
78+
* Pairs the code with the message {@link messageForOrchestrationError} permits for
79+
* it, so the two can never disagree. Hand-rolling that pair is what let raw driver
80+
* text reach clients: a site that defaulted the code with `?? 'internal'` but then
81+
* compared the *raw* `errorCode` against `'internal'` classified an uncoded failure
82+
* as internal while still rendering its own message.
83+
*/
84+
export function throwOrchestrationFailure(
85+
result: { error?: string; errorCode?: OrchestrationErrorCode },
86+
fallback: string
87+
): never {
88+
throw new OrchestrationError(
89+
result.errorCode ?? 'internal',
90+
messageForOrchestrationError(result, fallback)
91+
)
92+
}
93+
7594
/**
7695
* The {@link OrchestrationError} in `error`'s cause chain, or `null` when the
7796
* failure is not a classified one.

apps/sim/lib/knowledge/application/folders.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { AuditAction, AuditResourceType } from '@sim/audit'
22
import type { folder } from '@sim/db/schema'
3-
import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types'
3+
import {
4+
OrchestrationError,
5+
type OrchestrationErrorCode,
6+
throwOrchestrationFailure,
7+
} from '@/lib/core/orchestration/types'
48
import {
59
createFolderAtPath,
610
deleteFolderByPath,
@@ -49,10 +53,7 @@ export interface DeleteKnowledgeFolderInput {
4953
}
5054

5155
function throwFolderFailure(result: { error?: string; errorCode?: OrchestrationErrorCode }): never {
52-
throw new OrchestrationError(
53-
result.errorCode ?? 'internal',
54-
result.error ?? 'Folder operation failed'
55-
)
56+
throwOrchestrationFailure(result, 'Folder operation failed')
5657
}
5758

5859
export const listKnowledgeFolders = defineAuthorizedKnowledgeUseCase({
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { OrchestrationError } from '@/lib/core/orchestration/types'
6+
import { requireWorkflowTransition } from '@/lib/workflows/application/transition-result'
7+
8+
describe('requireWorkflowTransition', () => {
9+
it('returns without throwing for a successful transition', () => {
10+
expect(() => requireWorkflowTransition({ success: true }, 'Failed')).not.toThrow()
11+
})
12+
13+
it('withholds the raw message a failed lifecycle transition carries', () => {
14+
expect(() =>
15+
requireWorkflowTransition(
16+
{
17+
success: false,
18+
error: 'duplicate key value violates unique constraint "workflow_pkey"',
19+
errorCode: 'internal',
20+
},
21+
'Failed to create workflow'
22+
)
23+
).toThrow('Failed to create workflow')
24+
})
25+
26+
it('preserves a classified failure so the route maps the right status', () => {
27+
try {
28+
requireWorkflowTransition(
29+
{ success: false, error: 'No such workflow', errorCode: 'not_found' },
30+
'Failed to delete workflow'
31+
)
32+
expect.unreachable('expected requireWorkflowTransition to throw')
33+
} catch (error) {
34+
expect(error).toBeInstanceOf(OrchestrationError)
35+
expect((error as OrchestrationError).code).toBe('not_found')
36+
expect((error as OrchestrationError).message).toBe('No such workflow')
37+
}
38+
})
39+
})
Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1-
import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types'
1+
import {
2+
type OrchestrationErrorCode,
3+
throwOrchestrationFailure,
4+
} from '@/lib/core/orchestration/types'
25

36
export function requireWorkflowTransition<
47
T extends { success: boolean; error?: string; errorCode?: OrchestrationErrorCode },
58
>(result: T, fallbackMessage: string): asserts result is T & { success: true } {
69
if (result.success) return
7-
throw new OrchestrationError(result.errorCode ?? 'internal', result.error ?? fallbackMessage)
10+
throwOrchestrationFailure(result, fallbackMessage)
811
}

apps/sim/lib/workflows/application/workflow-folders.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { AuditAction, AuditResourceType } from '@sim/audit'
22
import { resolvePrincipalAttribution } from '@sim/auth/principal'
33
import type { folder } from '@sim/db/schema'
44
import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types'
5-
import { OrchestrationError } from '@/lib/core/orchestration/types'
5+
import { OrchestrationError, throwOrchestrationFailure } from '@/lib/core/orchestration/types'
66
import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants'
77
import { withFolderTreeLock } from '@/lib/folders/locks'
88
import {
@@ -73,11 +73,7 @@ function throwFolderMutationFailure(result: {
7373
error?: string
7474
errorCode?: OrchestrationErrorCode
7575
}): never {
76-
const code = result.errorCode ?? 'internal'
77-
throw new OrchestrationError(
78-
code,
79-
code === 'internal' ? 'Internal server error' : (result.error ?? 'Folder mutation failed')
80-
)
76+
throwOrchestrationFailure(result, 'Internal server error')
8177
}
8278

8379
export async function resolveWorkflowFolderPath(

apps/sim/lib/workflows/application/workflow-vfs.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
asOrchestrationError,
1414
OrchestrationError,
1515
type OrchestrationErrorCode,
16+
throwOrchestrationFailure,
1617
} from '@/lib/core/orchestration/types'
1718
import { generateRequestId } from '@/lib/core/utils/request'
1819
import {
@@ -246,12 +247,7 @@ function resolveWorkflowSources(
246247
}
247248

248249
function throwFolderFailure(result: { error?: string; errorCode?: OrchestrationErrorCode }): never {
249-
throw new OrchestrationError(
250-
result.errorCode ?? 'internal',
251-
result.errorCode === 'internal'
252-
? 'Workflow folder mutation failed'
253-
: (result.error ?? 'Folder mutation failed')
254-
)
250+
throwOrchestrationFailure(result, 'Workflow folder mutation failed')
255251
}
256252

257253
async function reloadFolderIndex(state: WorkflowVfsIndexState, workspaceId: string): Promise<void> {

0 commit comments

Comments
 (0)