mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-30 19:32:46 +08:00
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.
This commit is contained in:
@@ -33,7 +33,11 @@ import {
|
||||
InvalidInternalDelegationBindingError,
|
||||
} from '@/lib/auth/internal-delegation'
|
||||
import type { ApplicationOperation, OperationUseCase } from '@/lib/core/application'
|
||||
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
|
||||
import {
|
||||
asOrchestrationError,
|
||||
messageForOrchestrationError,
|
||||
statusForOrchestrationError,
|
||||
} from '@/lib/core/orchestration/types'
|
||||
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
|
||||
|
||||
export class InternalUnauthenticatedError extends Error {
|
||||
@@ -142,7 +146,10 @@ export const internalOrchestrationErrorPolicy: InternalErrorPolicy = {
|
||||
const classified = asOrchestrationError(error)
|
||||
if (!classified) return null
|
||||
return internalErrorResponse(statusForOrchestrationError(classified.code), {
|
||||
error: classified.message,
|
||||
error: messageForOrchestrationError(
|
||||
{ error: classified.message, errorCode: classified.code },
|
||||
'Internal server error'
|
||||
),
|
||||
})
|
||||
},
|
||||
unhandled() {
|
||||
|
||||
@@ -2,7 +2,15 @@
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { statusForOrchestrationError } from '@/lib/core/orchestration/types'
|
||||
import {
|
||||
messageForOrchestrationError,
|
||||
OrchestrationError,
|
||||
statusForOrchestrationError,
|
||||
throwOrchestrationFailure,
|
||||
} from '@/lib/core/orchestration/types'
|
||||
|
||||
const RAW_DRIVER_MESSAGE =
|
||||
'insert into "workflow" ("id") values ($1) - duplicate key value violates unique constraint "workflow_pkey"'
|
||||
|
||||
describe('statusForOrchestrationError', () => {
|
||||
it.each([
|
||||
@@ -15,3 +23,62 @@ describe('statusForOrchestrationError', () => {
|
||||
expect(statusForOrchestrationError(code)).toBe(expected)
|
||||
})
|
||||
})
|
||||
|
||||
describe('messageForOrchestrationError', () => {
|
||||
it('withholds the message of an explicitly internal failure', () => {
|
||||
expect(
|
||||
messageForOrchestrationError(
|
||||
{ error: RAW_DRIVER_MESSAGE, errorCode: 'internal' },
|
||||
'Failed to create workflow'
|
||||
)
|
||||
).toBe('Failed to create workflow')
|
||||
})
|
||||
|
||||
it('withholds the message of a failure carrying no code', () => {
|
||||
expect(
|
||||
messageForOrchestrationError({ error: RAW_DRIVER_MESSAGE }, 'Failed to create workflow')
|
||||
).toBe('Failed to create workflow')
|
||||
})
|
||||
|
||||
it('returns a classified failure message to the caller', () => {
|
||||
expect(
|
||||
messageForOrchestrationError(
|
||||
{ error: 'Workflow name is already taken', errorCode: 'conflict' },
|
||||
'Failed to create workflow'
|
||||
)
|
||||
).toBe('Workflow name is already taken')
|
||||
})
|
||||
|
||||
it('falls back when a classified failure carries no message', () => {
|
||||
expect(
|
||||
messageForOrchestrationError({ errorCode: 'conflict' }, 'Failed to create workflow')
|
||||
).toBe('Failed to create workflow')
|
||||
})
|
||||
})
|
||||
|
||||
describe('throwOrchestrationFailure', () => {
|
||||
it('classifies an uncoded failure as internal without rendering its message', () => {
|
||||
try {
|
||||
throwOrchestrationFailure({ error: RAW_DRIVER_MESSAGE }, 'Failed to update workflow')
|
||||
expect.unreachable('expected throwOrchestrationFailure to throw')
|
||||
} catch (error) {
|
||||
expect(error).toBeInstanceOf(OrchestrationError)
|
||||
expect((error as OrchestrationError).code).toBe('internal')
|
||||
expect((error as OrchestrationError).message).toBe('Failed to update workflow')
|
||||
}
|
||||
})
|
||||
|
||||
it('preserves the code and message of a classified failure', () => {
|
||||
try {
|
||||
throwOrchestrationFailure(
|
||||
{ error: 'No such workflow', errorCode: 'not_found' },
|
||||
'Failed to delete workflow'
|
||||
)
|
||||
expect.unreachable('expected throwOrchestrationFailure to throw')
|
||||
} catch (error) {
|
||||
expect(error).toBeInstanceOf(OrchestrationError)
|
||||
expect((error as OrchestrationError).code).toBe('not_found')
|
||||
expect((error as OrchestrationError).message).toBe('No such workflow')
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -72,6 +72,25 @@ export class OrchestrationError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Rethrows a failed orchestration result as its classified {@link OrchestrationError}.
|
||||
*
|
||||
* Pairs the code with the message {@link messageForOrchestrationError} permits for
|
||||
* it, so the two can never disagree. Hand-rolling that pair is what let raw driver
|
||||
* text reach clients: a site that defaulted the code with `?? 'internal'` but then
|
||||
* compared the *raw* `errorCode` against `'internal'` classified an uncoded failure
|
||||
* as internal while still rendering its own message.
|
||||
*/
|
||||
export function throwOrchestrationFailure(
|
||||
result: { error?: string; errorCode?: OrchestrationErrorCode },
|
||||
fallback: string
|
||||
): never {
|
||||
throw new OrchestrationError(
|
||||
result.errorCode ?? 'internal',
|
||||
messageForOrchestrationError(result, fallback)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The {@link OrchestrationError} in `error`'s cause chain, or `null` when the
|
||||
* failure is not a classified one.
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { AuditAction, AuditResourceType } from '@sim/audit'
|
||||
import type { folder } from '@sim/db/schema'
|
||||
import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types'
|
||||
import {
|
||||
OrchestrationError,
|
||||
type OrchestrationErrorCode,
|
||||
throwOrchestrationFailure,
|
||||
} from '@/lib/core/orchestration/types'
|
||||
import {
|
||||
createFolderAtPath,
|
||||
deleteFolderByPath,
|
||||
@@ -49,10 +53,7 @@ export interface DeleteKnowledgeFolderInput {
|
||||
}
|
||||
|
||||
function throwFolderFailure(result: { error?: string; errorCode?: OrchestrationErrorCode }): never {
|
||||
throw new OrchestrationError(
|
||||
result.errorCode ?? 'internal',
|
||||
result.error ?? 'Folder operation failed'
|
||||
)
|
||||
throwOrchestrationFailure(result, 'Folder operation failed')
|
||||
}
|
||||
|
||||
export const listKnowledgeFolders = defineAuthorizedKnowledgeUseCase({
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { OrchestrationError } from '@/lib/core/orchestration/types'
|
||||
import { requireWorkflowTransition } from '@/lib/workflows/application/transition-result'
|
||||
|
||||
describe('requireWorkflowTransition', () => {
|
||||
it('returns without throwing for a successful transition', () => {
|
||||
expect(() => requireWorkflowTransition({ success: true }, 'Failed')).not.toThrow()
|
||||
})
|
||||
|
||||
it('withholds the raw message a failed lifecycle transition carries', () => {
|
||||
expect(() =>
|
||||
requireWorkflowTransition(
|
||||
{
|
||||
success: false,
|
||||
error: 'duplicate key value violates unique constraint "workflow_pkey"',
|
||||
errorCode: 'internal',
|
||||
},
|
||||
'Failed to create workflow'
|
||||
)
|
||||
).toThrow('Failed to create workflow')
|
||||
})
|
||||
|
||||
it('preserves a classified failure so the route maps the right status', () => {
|
||||
try {
|
||||
requireWorkflowTransition(
|
||||
{ success: false, error: 'No such workflow', errorCode: 'not_found' },
|
||||
'Failed to delete workflow'
|
||||
)
|
||||
expect.unreachable('expected requireWorkflowTransition to throw')
|
||||
} catch (error) {
|
||||
expect(error).toBeInstanceOf(OrchestrationError)
|
||||
expect((error as OrchestrationError).code).toBe('not_found')
|
||||
expect((error as OrchestrationError).message).toBe('No such workflow')
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -1,8 +1,11 @@
|
||||
import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types'
|
||||
import {
|
||||
type OrchestrationErrorCode,
|
||||
throwOrchestrationFailure,
|
||||
} from '@/lib/core/orchestration/types'
|
||||
|
||||
export function requireWorkflowTransition<
|
||||
T extends { success: boolean; error?: string; errorCode?: OrchestrationErrorCode },
|
||||
>(result: T, fallbackMessage: string): asserts result is T & { success: true } {
|
||||
if (result.success) return
|
||||
throw new OrchestrationError(result.errorCode ?? 'internal', result.error ?? fallbackMessage)
|
||||
throwOrchestrationFailure(result, fallbackMessage)
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@ import { AuditAction, AuditResourceType } from '@sim/audit'
|
||||
import { resolvePrincipalAttribution } from '@sim/auth/principal'
|
||||
import type { folder } from '@sim/db/schema'
|
||||
import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types'
|
||||
import { OrchestrationError } from '@/lib/core/orchestration/types'
|
||||
import { OrchestrationError, throwOrchestrationFailure } from '@/lib/core/orchestration/types'
|
||||
import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants'
|
||||
import { withFolderTreeLock } from '@/lib/folders/locks'
|
||||
import {
|
||||
@@ -73,11 +73,7 @@ function throwFolderMutationFailure(result: {
|
||||
error?: string
|
||||
errorCode?: OrchestrationErrorCode
|
||||
}): never {
|
||||
const code = result.errorCode ?? 'internal'
|
||||
throw new OrchestrationError(
|
||||
code,
|
||||
code === 'internal' ? 'Internal server error' : (result.error ?? 'Folder mutation failed')
|
||||
)
|
||||
throwOrchestrationFailure(result, 'Internal server error')
|
||||
}
|
||||
|
||||
export async function resolveWorkflowFolderPath(
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
asOrchestrationError,
|
||||
OrchestrationError,
|
||||
type OrchestrationErrorCode,
|
||||
throwOrchestrationFailure,
|
||||
} from '@/lib/core/orchestration/types'
|
||||
import { generateRequestId } from '@/lib/core/utils/request'
|
||||
import {
|
||||
@@ -246,12 +247,7 @@ function resolveWorkflowSources(
|
||||
}
|
||||
|
||||
function throwFolderFailure(result: { error?: string; errorCode?: OrchestrationErrorCode }): never {
|
||||
throw new OrchestrationError(
|
||||
result.errorCode ?? 'internal',
|
||||
result.errorCode === 'internal'
|
||||
? 'Workflow folder mutation failed'
|
||||
: (result.error ?? 'Folder mutation failed')
|
||||
)
|
||||
throwOrchestrationFailure(result, 'Workflow folder mutation failed')
|
||||
}
|
||||
|
||||
async function reloadFolderIndex(state: WorkflowVfsIndexState, workspaceId: string): Promise<void> {
|
||||
|
||||
Reference in New Issue
Block a user