fix(api): classify access-denied and sandbox user-code errors with correct HTTP status (#4740)

* fix(api): classify access-denied and sandbox user-code errors with correct HTTP status

* fix(api): gate typed-error message exposure behind publicMessage opt-in

* refactor(api): match NestJS/Spring convention for typed-error message exposure
This commit is contained in:
Waleed
2026-05-26 11:00:29 -07:00
committed by GitHub
parent 81bf93b184
commit 3b18d3ba0e
11 changed files with 122 additions and 12 deletions
+8 -1
View File
@@ -12,13 +12,17 @@ import { normalizeMessage } from '@/lib/copilot/chat/persisted-message'
import {
authenticateCopilotRequestSessionOnly,
createBadRequestResponse,
createForbiddenResponse,
createInternalServerErrorResponse,
createUnauthorizedResponse,
} from '@/lib/copilot/request/http'
import { readFilePreviewSessions } from '@/lib/copilot/request/session'
import { readEvents } from '@/lib/copilot/request/session/buffer'
import { toStreamBatchEvent } from '@/lib/copilot/request/session/types'
import { assertActiveWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
import {
assertActiveWorkspaceAccess,
isWorkspaceAccessDeniedError,
} from '@/lib/workspaces/permissions/utils'
const logger = createLogger('CopilotChatAPI')
@@ -196,6 +200,9 @@ export async function GET(req: NextRequest) {
chats: chats.map(transformChatListItem),
})
} catch (error) {
if (isWorkspaceAccessDeniedError(error)) {
return createForbiddenResponse('Workspace access denied')
}
logger.error('Error fetching copilot chats:', error)
return createInternalServerErrorResponse('Failed to fetch chats')
}
+8 -1
View File
@@ -10,12 +10,16 @@ import { resolveOrCreateChat } from '@/lib/copilot/chat/lifecycle'
import {
authenticateCopilotRequestSessionOnly,
createBadRequestResponse,
createForbiddenResponse,
createInternalServerErrorResponse,
createUnauthorizedResponse,
} from '@/lib/copilot/request/http'
import { taskPubSub } from '@/lib/copilot/tasks'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { assertActiveWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
import {
assertActiveWorkspaceAccess,
isWorkspaceAccessDeniedError,
} from '@/lib/workspaces/permissions/utils'
const logger = createLogger('CopilotChatsListAPI')
@@ -138,6 +142,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
return NextResponse.json({ success: true, id: result.chatId })
} catch (error) {
if (isWorkspaceAccessDeniedError(error)) {
return createForbiddenResponse('Workspace access denied')
}
logger.error('Error creating workflow copilot chat:', error)
return createInternalServerErrorResponse('Failed to create chat')
}
+3 -3
View File
@@ -1132,7 +1132,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
output: { result: null, stdout: cleanStdout(shellStdout), executionTime },
},
routeContext,
{ status: 500 }
{ status: 422 }
)
}
@@ -1269,7 +1269,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
output: { result: null, stdout: cleanedOutput, executionTime },
},
routeContext,
{ status: 500 }
{ status: 422 }
)
}
@@ -1356,7 +1356,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
output: { result: null, stdout: cleanedOutput, executionTime },
},
routeContext,
{ status: 500 }
{ status: 422 }
)
}
@@ -11,6 +11,7 @@ import { fetchGo } from '@/lib/copilot/request/go/fetch'
import {
authenticateCopilotRequestSessionOnly,
createBadRequestResponse,
createForbiddenResponse,
createInternalServerErrorResponse,
createNotFoundResponse,
createUnauthorizedResponse,
@@ -21,7 +22,10 @@ import { taskPubSub } from '@/lib/copilot/tasks'
import { env } from '@/lib/core/config/env'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { captureServerEvent } from '@/lib/posthog/server'
import { assertActiveWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
import {
assertActiveWorkspaceAccess,
isWorkspaceAccessDeniedError,
} from '@/lib/workspaces/permissions/utils'
const logger = createLogger('ForkChatAPI')
@@ -150,6 +154,9 @@ export const POST = withRouteHandler(
return NextResponse.json({ success: true, id: newId })
} catch (error) {
if (isWorkspaceAccessDeniedError(error)) {
return createForbiddenResponse('Workspace access denied')
}
logger.error('Error forking chat:', error)
return createInternalServerErrorResponse('Failed to fork chat')
}
+11 -1
View File
@@ -11,13 +11,17 @@ import { parseRequest } from '@/lib/api/server'
import { reconcileChatStreamMarkers } from '@/lib/copilot/chat/stream-liveness'
import {
authenticateCopilotRequestSessionOnly,
createForbiddenResponse,
createInternalServerErrorResponse,
createUnauthorizedResponse,
} from '@/lib/copilot/request/http'
import { taskPubSub } from '@/lib/copilot/tasks'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { captureServerEvent } from '@/lib/posthog/server'
import { assertActiveWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
import {
assertActiveWorkspaceAccess,
isWorkspaceAccessDeniedError,
} from '@/lib/workspaces/permissions/utils'
const logger = createLogger('MothershipChatsAPI')
@@ -68,6 +72,9 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
return NextResponse.json({ success: true, data: reconciled })
} catch (error) {
if (isWorkspaceAccessDeniedError(error)) {
return createForbiddenResponse('Workspace access denied')
}
logger.error('Error fetching mothership chats:', error)
return createInternalServerErrorResponse('Failed to fetch chats')
}
@@ -118,6 +125,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
return NextResponse.json({ success: true, id: chat.id })
} catch (error) {
if (isWorkspaceAccessDeniedError(error)) {
return createForbiddenResponse('Workspace access denied')
}
logger.error('Error creating mothership chat:', error)
return createInternalServerErrorResponse('Failed to create chat')
}
@@ -19,6 +19,7 @@ import { buildMothershipToolsForRequest } from '@/lib/mothership/settings/runtim
import {
assertActiveWorkspaceAccess,
getUserEntityPermissions,
isWorkspaceAccessDeniedError,
} from '@/lib/workspaces/permissions/utils'
export const maxDuration = 3600
@@ -378,6 +379,10 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
return NextResponse.json({ error: 'Mothership execution aborted' }, { status: 499 })
}
if (isWorkspaceAccessDeniedError(error)) {
return NextResponse.json({ error: 'Workspace access denied' }, { status: 403 })
}
logger.error(
messageId ? `Mothership execute error [messageId:${messageId}]` : 'Mothership execute error',
{
+10 -1
View File
@@ -19,7 +19,10 @@ import {
} from '@/lib/uploads/contexts/workspace/workspace-file-manager'
import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils'
import { performMoveWorkspaceFileItems } from '@/lib/workspace-files/orchestration'
import { assertActiveWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
import {
assertActiveWorkspaceAccess,
isWorkspaceAccessDeniedError,
} from '@/lib/workspaces/permissions/utils'
export const dynamic = 'force-dynamic'
@@ -352,6 +355,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
}
}
} catch (error) {
if (isWorkspaceAccessDeniedError(error)) {
return NextResponse.json(
{ success: false, error: 'Workspace access denied' },
{ status: 403 }
)
}
const message = getErrorMessage(error, 'Unknown error')
logger.error('File operation failed', { operation: body.operation, error: message })
return NextResponse.json({ success: false, error: message }, { status: 500 })
+8 -1
View File
@@ -44,7 +44,10 @@ import { taskPubSub } from '@/lib/copilot/tasks'
import { prepareExecutionContext } from '@/lib/copilot/tools/handlers/context'
import { getEffectiveDecryptedEnv } from '@/lib/environment/utils'
import { getWorkflowById, resolveWorkflowIdForUser } from '@/lib/workflows/utils'
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
import {
getUserEntityPermissions,
isWorkspaceAccessDeniedError,
} from '@/lib/workspaces/permissions/utils'
import type { ChatContext } from '@/stores/panel'
export const maxDuration = 3600
@@ -1039,6 +1042,10 @@ export async function handleUnifiedChatPost(req: NextRequest) {
return validationErrorResponse(error, 'Invalid request data')
}
if (isWorkspaceAccessDeniedError(error)) {
return NextResponse.json({ error: 'Workspace access denied' }, { status: 403 })
}
logger.error(`[${requestId}] Error handling unified chat request`, {
error: getErrorMessage(error, 'Unknown error'),
stack: error instanceof Error ? error.stack : undefined,
+4
View File
@@ -35,6 +35,10 @@ export function createBadRequestResponse(message: string): NextResponse {
return NextResponse.json({ error: message }, { status: 400 })
}
export function createForbiddenResponse(message: string): NextResponse {
return NextResponse.json({ error: message }, { status: 403 })
}
export function createNotFoundResponse(message: string): NextResponse {
return NextResponse.json({ error: message }, { status: 404 })
}
+35 -2
View File
@@ -11,6 +11,26 @@ type RouteHandler<T = unknown> = (
context: T
) => Promise<NextResponse | Response> | NextResponse | Response
/**
* Reads a numeric `statusCode` (4xx or 5xx) off an Error so typed domain errors
* (e.g. `WorkspaceAccessDeniedError`, `InvalidFieldError`) map to the correct
* HTTP status when they bubble up unhandled instead of defaulting to 500.
*
* When a typed status is returned, the error's `message` is sent to the client
* verbatim — matching the NestJS `HttpException` / Spring `ResponseStatusException`
* convention. The safety contract is convention-based: only attach `statusCode`
* to errors whose `message` is safe to expose to clients (no stack traces,
* secrets, file paths, ORM internals). Untyped errors fall back to a generic
* 500 response with no message exposure.
*/
function readTypedErrorStatus(error: unknown): number | undefined {
if (!(error instanceof Error)) return undefined
const status = (error as { statusCode?: unknown }).statusCode
if (typeof status !== 'number') return undefined
if (status < 400 || status >= 600) return undefined
return status
}
/**
* Wraps a Next.js API route handler with centralized error reporting.
*
@@ -35,8 +55,21 @@ export function withRouteHandler<T>(handler: RouteHandler<T>): RouteHandler<T> {
} catch (error) {
const duration = Date.now() - startTime
const message = getErrorMessage(error, 'Unknown error')
logger.error('Unhandled route error', { duration, error: message })
response = NextResponse.json({ error: 'Internal server error', requestId }, { status: 500 })
const typedStatus = readTypedErrorStatus(error)
if (typedStatus !== undefined) {
if (typedStatus >= 500) {
logger.error('Unhandled route error', { duration, status: typedStatus, error: message })
} else {
logger.warn('Typed route error', { duration, status: typedStatus, error: message })
}
response = NextResponse.json({ error: message, requestId }, { status: typedStatus })
} else {
logger.error('Unhandled route error', { duration, error: message })
response = NextResponse.json(
{ error: 'Internal server error', requestId },
{ status: 500 }
)
}
response?.headers?.set('x-request-id', requestId)
return response
}
+22 -1
View File
@@ -147,13 +147,34 @@ export async function checkWorkspaceAccess(
return { exists: true, hasAccess: true, canWrite, workspace: ws }
}
/**
* Thrown when a user attempts to access a workspace they don't have access to,
* or that doesn't exist / has been archived. Carries `statusCode = 403` so the
* centralized route wrapper maps it to HTTP 403 instead of defaulting to 500.
* The `message` is intentionally client-safe and is exposed to API responses.
*/
export class WorkspaceAccessDeniedError extends Error {
readonly statusCode = 403
readonly workspaceId: string
constructor(workspaceId: string) {
super(`Workspace access denied: ${workspaceId}`)
this.name = 'WorkspaceAccessDeniedError'
this.workspaceId = workspaceId
}
}
export function isWorkspaceAccessDeniedError(error: unknown): error is WorkspaceAccessDeniedError {
return error instanceof WorkspaceAccessDeniedError
}
export async function assertActiveWorkspaceAccess(
workspaceId: string,
userId: string
): Promise<WorkspaceAccess> {
const access = await checkWorkspaceAccess(workspaceId, userId)
if (!access.exists || !access.hasAccess) {
throw new Error(`Active workspace access denied: ${workspaceId}`)
throw new WorkspaceAccessDeniedError(workspaceId)
}
return access
}