Skip to content

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

Merged
waleedlatif1 merged 3 commits into
stagingfrom
waleedlatif1/audit-500-error-fallthroughs
May 26, 2026
Merged

fix(api): classify access-denied and sandbox user-code errors with correct HTTP status#4740
waleedlatif1 merged 3 commits into
stagingfrom
waleedlatif1/audit-500-error-fallthroughs

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

@waleedlatif1 waleedlatif1 commented May 26, 2026

Summary

  • Add WorkspaceAccessDeniedError (statusCode 403) thrown by assertActiveWorkspaceAccess; route catch blocks map it to 403 instead of swallowing as 500. Updates: mothership/chats GET+POST, mothership/chats/[chatId]/fork, mothership/execute, copilot/chats POST, copilot/chat queries, tools/file/manage, copilot chat post (unified handler)
  • withRouteHandler now reads a numeric statusCode off unhandled Errors and maps to that status; falls back to 500 otherwise. Covers future typed errors automatically
  • /api/function/execute returns 422 (instead of 500) for E2B shell, JS, and Python user-code errors — mirrors the existing isolated-vm path
  • New createForbiddenResponse helper in lib/copilot/request/http.ts
  • Audited via Athena: ~95% of production 500s in last 24h fall into these categories (workspace 403s, user-code 422s, upstream 502s already correct)

Type of Change

  • Bug fix

Testing

  • bun run check:api-validation passes
  • tsc --noEmit for apps/sim clean
  • Vitest targeted suites: function/execute (35), mothership/chats (5), copilot/chats (8), copilot/chat/delete (8), copilot/chat/post (5), permissions utils (58) — 119/119 pass

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 26, 2026 5:50pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 26, 2026

PR Summary

Low Risk
Behavioral HTTP status changes only; no auth logic changes beyond surfacing existing denials as 403. Typed error messages are client-exposed by design in withRouteHandler—contract already documented for safe messages.

Overview
This PR fixes misclassified HTTP statuses so clients and monitoring see the right codes instead of blanket 500s.

Workspace access now throws WorkspaceAccessDeniedError (statusCode 403) from assertActiveWorkspaceAccess. Copilot/mothership chat routes, file manage, unified chat post, and mothership execute map that error to 403 (via createForbiddenResponse or equivalent JSON). A shared createForbiddenResponse helper was added for copilot HTTP utilities.

withRouteHandler reads a numeric statusCode on unhandled Error instances and responds with that status and the error message (4xx logged as warn); untyped errors still return generic 500.

/api/function/execute returns 422 (not 500) for E2B shell failures and E2B JavaScript/Python user-code errors, aligning with the existing isolated-vm user-error path.

Reviewed by Cursor Bugbot for commit 2e2d10b. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 26, 2026

Greptile Summary

This PR fixes HTTP status code classification for two classes of previously-misclassified 500 errors: workspace access-denied checks (now 403) and E2B sandbox user-code failures (now 422). It introduces a typed WorkspaceAccessDeniedError with statusCode = 403, a readTypedErrorStatus fallback in withRouteHandler, and explicit guards in the seven affected routes.

  • Typed error + centralized fallback: WorkspaceAccessDeniedError carries statusCode = 403; withRouteHandler now reads any statusCode in the 400–599 range off an unhandled Error and maps it to that status, covering future typed domain errors automatically without per-route boilerplate.
  • 422 for E2B user-code errors: Shell, JS, and Python sandbox execution failures in /api/function/execute are now returned as 422 (Unprocessable Entity), consistent with the existing isolated-VM path.
  • createForbiddenResponse helper: Adds a small HTTP helper in lib/copilot/request/http.ts matching the existing createBadRequestResponse / createUnauthorizedResponse pattern.

Confidence Score: 5/5

Safe to merge — changes are targeted status-code reclassifications with no business logic alterations.

All seven routes that call assertActiveWorkspaceAccess (directly or through resolveOrCreateChat / generateWorkspaceContext) were audited and updated. The withRouteHandler fallback provides a safety net for any future typed error. The 422 change for E2B errors mirrors the pre-existing isolated-VM behavior, making the execution paths consistent. No auth logic, data mutations, or core execution paths were modified.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/lib/core/utils/with-route-handler.ts Adds readTypedErrorStatus helper that reads a numeric statusCode (400–599) off typed domain errors; withRouteHandler now maps those to the correct HTTP status instead of always returning 500. 4xx typed errors log at warn, 5xx at error.
apps/sim/lib/workspaces/permissions/utils.ts Adds WorkspaceAccessDeniedError typed error class with statusCode=403 and isWorkspaceAccessDeniedError type guard; assertActiveWorkspaceAccess now throws the typed error instead of a plain Error.
apps/sim/app/api/function/execute/route.ts E2B shell, JS, and Python user-code error responses changed from 500 → 422 (Unprocessable Entity), matching the existing isolated-vm path behavior.
apps/sim/lib/copilot/request/http.ts Adds createForbiddenResponse helper that returns a 403 JSON response, consistent with existing createBadRequestResponse/createUnauthorizedResponse helpers.
apps/sim/app/api/mothership/execute/route.ts Adds isWorkspaceAccessDeniedError guard in the outer catch block after the abort (499) check, returning 403 for workspace access denied instead of falling through to the generic 500.
apps/sim/app/api/copilot/chat/queries.ts Adds explicit WorkspaceAccessDeniedError guard in the GET handler catch block, returning createForbiddenResponse before the generic 500 path.
apps/sim/app/api/mothership/chats/route.ts Adds isWorkspaceAccessDeniedError guard to both GET and POST catch blocks, returning createForbiddenResponse instead of the generic 500.
apps/sim/lib/copilot/chat/post.ts Adds isWorkspaceAccessDeniedError guard in handleUnifiedChatPost's outer catch, between ZodError handling and the generic 500 path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Route Handler] -->|try| B[assertActiveWorkspaceAccess]
    B -->|access OK| C[Continue handler logic]
    B -->|access denied| D[throw WorkspaceAccessDeniedError\nstatusCode = 403]
    D -->|explicit inner catch| E{isWorkspaceAccessDeniedError?}
    E -->|yes| F[createForbiddenResponse\n403 — 'Workspace access denied']
    E -->|no / not present| G[withRouteHandler catch]
    G -->|readTypedErrorStatus| H{statusCode 400–599?}
    H -->|yes| I[NextResponse 403\nerror: message includes workspaceId]
    H -->|no| J[NextResponse 500\nerror: 'Internal server error']

    K[E2B sandbox execute] -->|shellError / e2bError| L[functionJsonResponse\n422 — user code error]
    M[isolated-vm execute] -->|vm error| N[functionJsonResponse\n422 — already correct]
Loading

Reviews (3): Last reviewed commit: "refactor(api): match NestJS/Spring conve..." | Re-trigger Greptile

Comment thread apps/sim/lib/core/utils/with-route-handler.ts
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d5683f5. Configure here.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2e2d10b. Configure here.

@waleedlatif1 waleedlatif1 merged commit 3b18d3b into staging May 26, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/audit-500-error-fallthroughs branch May 26, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant