fix(api): distinguish visible resource authorization failures (#6537)

Author: TheodoreSpeaks

apps/sim/app/api/v2/files/[fileId]/content/route.test.ts

@@ -47,6 +47,7 @@ vi.mock('@/lib/users/queries', () => ({
   requireResolvedUserEmail: (emails: Map<string, string>, userId: string) => emails.get(userId)!,
 }))
 
+import { NoWorkspaceAccessError } from '@/lib/core/application'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { PUT } from '@/app/api/v2/files/[fileId]/content/route'
 
@@ -110,9 +111,7 @@ describe('PUT /api/v2/files/[fileId]/content', () => {
   })
 
   it('performs authenticated admission before parsing a large or malformed body', async () => {
-    mocks.admit.mockRejectedValue(
-      new OrchestrationError('forbidden', 'Insufficient workspace permissions')
-    )
+    mocks.admit.mockRejectedValue(new NoWorkspaceAccessError())
 
     const response = await callPut('{not-json')
 

apps/sim/app/api/v2/files/[fileId]/metadata/route.test.ts

@@ -41,7 +41,7 @@ vi.mock('@/lib/users/queries', () => ({
   requireResolvedUserEmail: (emails: Map<string, string>, userId: string) => emails.get(userId)!,
 }))
 
-import { OrchestrationError } from '@/lib/core/orchestration/types'
+import { NoWorkspaceAccessError } from '@/lib/core/application'
 import { GET } from '@/app/api/v2/files/[fileId]/metadata/route'
 
 const WORKSPACE_ID = 'workspace-1'
@@ -114,9 +114,7 @@ describe('GET /api/v2/files/[fileId]/metadata', () => {
   })
 
   it('conceals an authorization failure as not found', async () => {
-    mocks.readMetadata.mockRejectedValue(
-      new OrchestrationError('forbidden', 'Insufficient workspace permissions')
-    )
+    mocks.readMetadata.mockRejectedValue(new NoWorkspaceAccessError())
 
     const response = await callGet(`workspaceId=${WORKSPACE_ID}`)
 

apps/sim/app/api/v2/files/[fileId]/route.test.ts

@@ -55,6 +55,10 @@ vi.mock('@/lib/users/queries', () => ({
   requireResolvedUserEmail: (emails: Map<string, string>, userId: string) => emails.get(userId)!,
 }))
 
+import {
+  InsufficientWorkspacePermissionsError,
+  NoWorkspaceAccessError,
+} from '@/lib/core/application'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { DELETE, GET, PATCH } from '@/app/api/v2/files/[fileId]/route'
 
@@ -136,9 +140,7 @@ describe('v2 single-file routes', () => {
   })
 
   it('conceals download authorization failures', async () => {
-    mocks.download.mockRejectedValue(
-      new OrchestrationError('forbidden', 'Insufficient workspace permissions')
-    )
+    mocks.download.mockRejectedValue(new NoWorkspaceAccessError())
 
     const response = await GET(
       new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}?workspaceId=${WORKSPACE_ID}`),
@@ -166,7 +168,7 @@ describe('v2 single-file routes', () => {
     })
   })
 
-  it('maps rename conflicts and conceals authorization failures', async () => {
+  it('maps rename conflicts and conceals absent workspace access', async () => {
     mocks.rename.mockRejectedValueOnce(new OrchestrationError('conflict', 'Name exists'))
     const conflict = await PATCH(
       new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}`, {
@@ -178,9 +180,7 @@ describe('v2 single-file routes', () => {
     )
     expect(conflict.status).toBe(409)
 
-    mocks.rename.mockRejectedValueOnce(
-      new OrchestrationError('forbidden', 'Insufficient workspace permissions')
-    )
+    mocks.rename.mockRejectedValueOnce(new NoWorkspaceAccessError())
     const concealed = await PATCH(
       new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}`, {
         method: 'PATCH',
@@ -192,6 +192,23 @@ describe('v2 single-file routes', () => {
     expect(concealed.status).toBe(404)
   })
 
+  it('returns forbidden when the current workspace role cannot rename the file', async () => {
+    mocks.rename.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())
+    const response = await PATCH(
+      new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}`, {
+        method: 'PATCH',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({ workspaceId: WORKSPACE_ID, name: 'renamed.csv' }),
+      }),
+      context
+    )
+
+    expect(response.status).toBe(403)
+    expect(await response.json()).toEqual({
+      error: { code: 'FORBIDDEN', message: 'Insufficient workspace permissions' },
+    })
+  })
+
   it('archives through the same principal and operation pipeline', async () => {
     const request = new NextRequest(
       `http://localhost:3000/api/v2/files/${FILE_ID}?workspaceId=${WORKSPACE_ID}`,

apps/sim/app/api/v2/workflows/[id]/route.test.ts

@@ -36,10 +36,7 @@ vi.mock('@/lib/core/rate-limiter', () => ({
 }))
 vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate }))
 
-import {
-  InsufficientWorkspacePermissionsError,
-  PersonalApiKeysDisabledError,
-} from '@/lib/core/application'
+import { NoWorkspaceAccessError, PersonalApiKeysDisabledError } from '@/lib/core/application'
 import { DELETE, GET, PATCH } from '@/app/api/v2/workflows/[id]/route'
 
 const WORKSPACE_ID = 'workspace-1'
@@ -124,8 +121,8 @@ describe('/api/v2/workflows/[id]', () => {
     })
   })
 
-  it('conceals typed insufficient authorization as workflow absence', async () => {
-    mocks.readWorkflow.mockRejectedValue(new InsufficientWorkspacePermissionsError())
+  it('conceals absent workspace access as workflow absence', async () => {
+    mocks.readWorkflow.mockRejectedValue(new NoWorkspaceAccessError())
     const response = await GET(
       new NextRequest(`http://localhost/api/v2/workflows/${WORKFLOW_ID}`),
       routeContext

apps/sim/app/api/v2/workflows/[id]/runs/[runId]/route.test.ts

@@ -52,7 +52,10 @@ vi.mock('@/lib/workflows/application/cancel-run', () => ({
   },
 }))
 
-import { InsufficientWorkspacePermissionsError } from '@/lib/core/application'
+import {
+  InsufficientWorkspacePermissionsError,
+  NoWorkspaceAccessError,
+} from '@/lib/core/application'
 import { POST as cancelPost } from '@/app/api/v2/workflows/[id]/runs/[runId]/cancel/route'
 import { GET } from '@/app/api/v2/workflows/[id]/runs/[runId]/route'
 
@@ -194,7 +197,7 @@ describe('v2 run detail and cancel adapters', () => {
   })
 
   it('conceals canonical run authorization failures as absence', async () => {
-    mocks.readRun.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())
+    mocks.readRun.mockRejectedValueOnce(new NoWorkspaceAccessError())
 
     const response = await callStatus()
 
@@ -263,17 +266,17 @@ describe('v2 run detail and cancel adapters', () => {
     expect(mocks.cancel).not.toHaveBeenCalled()
   })
 
-  it('conceals cancellation authorization failures using canonical run policy', async () => {
+  it('returns forbidden when the current workspace role cannot cancel the run', async () => {
     mocks.cancel.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())
 
     const response = await cancelPost(createMockRequest('POST', undefined, {}), {
       params: Promise.resolve({ id: 'workflow-1', runId: 'run-1' }),
     })
 
-    expect(response.status).toBe(404)
+    expect(response.status).toBe(403)
     expect((await response.json()).error).toMatchObject({
-      code: 'NOT_FOUND',
-      message: 'Run not found',
+      code: 'FORBIDDEN',
+      message: 'Insufficient workspace permissions',
     })
     expect(mocks.capture).not.toHaveBeenCalled()
   })

apps/sim/app/api/v2/workflows/[id]/runs/route.test.ts

@@ -35,10 +35,7 @@ vi.mock('@/lib/workflows/application/list-workflow-runs', () => ({
   },
 }))
 
-import {
-  InsufficientWorkspacePermissionsError,
-  PersonalApiKeysDisabledError,
-} from '@/lib/core/application'
+import { NoWorkspaceAccessError, PersonalApiKeysDisabledError } from '@/lib/core/application'
 import { GET } from '@/app/api/v2/workflows/[id]/runs/route'
 
 const principal = {
@@ -176,7 +173,7 @@ describe('GET /api/v2/workflows/[id]/runs', () => {
   })
 
   it('conceals workflow authorization failures as absence', async () => {
-    mocks.listRuns.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())
+    mocks.listRuns.mockRejectedValueOnce(new NoWorkspaceAccessError())
 
     const response = await callGet()
 

apps/sim/lib/api/server/routes/v2-resource-concealment.test.ts

@@ -6,6 +6,7 @@ import type { V2ErrorPolicy } from '@/lib/api/server/routes'
 import {
   DelegatedWorkspaceAuthorizationError,
   InsufficientWorkspacePermissionsError,
+  NoWorkspaceAccessError,
   PersonalApiKeysDisabledError,
   PrincipalKindAuthorizationError,
   WorkspaceApiKeyAuthorizationError,
@@ -59,7 +60,7 @@ const policies: Array<{
 ]
 
 const resourceAuthorizationErrors = [
-  new InsufficientWorkspacePermissionsError(),
+  new NoWorkspaceAccessError(),
   new WorkspaceApiKeyAuthorizationError(),
   new DelegatedWorkspaceAuthorizationError(),
   new PrincipalKindAuthorizationError('workspace_api_key', 'resources.read'),
@@ -88,11 +89,21 @@ describe.each(policies)('$domain resource concealment', ({ policy, notFoundMessa
     })
   })
 
-  it('preserves unrelated forbidden business failures', async () => {
-    const response = policy.render(new OrchestrationError('forbidden', 'Business rule denied'))
+  it('preserves insufficient workspace role as forbidden', async () => {
+    const response = policy.render(new InsufficientWorkspacePermissionsError())
     expect(response?.status).toBe(403)
     await expect(response?.json()).resolves.toEqual({
-      error: { code: 'FORBIDDEN', message: 'Business rule denied' },
+      error: { code: 'FORBIDDEN', message: 'Insufficient workspace permissions' },
+    })
+  })
+
+  it('does not classify generic forbidden errors by message', async () => {
+    const response = policy.render(
+      new OrchestrationError('forbidden', 'Insufficient workspace permissions')
+    )
+    expect(response?.status).toBe(403)
+    await expect(response?.json()).resolves.toEqual({
+      error: { code: 'FORBIDDEN', message: 'Insufficient workspace permissions' },
     })
   })
 })

apps/sim/lib/api/server/routes/v2-resource-concealment.ts

@@ -1,7 +1,7 @@
 import type { V2ErrorPolicy } from '@/lib/api/server/routes/v2-json-route'
 import {
   DelegatedWorkspaceAuthorizationError,
-  InsufficientWorkspacePermissionsError,
+  NoWorkspaceAccessError,
   PrincipalKindAuthorizationError,
   WorkspaceApiKeyAuthorizationError,
 } from '@/lib/core/application'
@@ -12,7 +12,7 @@ type V2ErrorRenderer = V2ErrorPolicy['render']
 function isResourceAuthorizationError(error: unknown): boolean {
   return (
     error instanceof DelegatedWorkspaceAuthorizationError ||
-    error instanceof InsufficientWorkspacePermissionsError ||
+    error instanceof NoWorkspaceAccessError ||
     error instanceof PrincipalKindAuthorizationError ||
     error instanceof WorkspaceApiKeyAuthorizationError
   )

apps/sim/lib/core/application/index.ts

@@ -19,6 +19,7 @@ export {
   DelegatedServiceAuthorizationError,
   DelegatedWorkspaceAuthorizationError,
   InsufficientWorkspacePermissionsError,
+  NoWorkspaceAccessError,
   PersonalApiKeysDisabledError,
   PrincipalKindAuthorizationError,
   requireAllowedWorkspacePrincipal,

apps/sim/lib/core/application/workspace-authorization.test.ts

@@ -0,0 +1,73 @@
+/**
+ * @vitest-environment node
+ */
+import type { SessionPrincipal } from '@sim/auth/principal'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const mocks = vi.hoisted(() => ({
+  resolvePermission: vi.fn(),
+}))
+
+vi.mock('@sim/platform-authz/workspace', () => ({
+  permissionSatisfies: (actual: string, required: string) => {
+    const rank = { read: 1, write: 2, admin: 3 } as const
+    return rank[actual as keyof typeof rank] >= rank[required as keyof typeof rank]
+  },
+  resolveEffectiveWorkspacePermission: mocks.resolvePermission,
+}))
+
+import {
+  authorizeWorkspaceOperation,
+  defineWorkspaceOperation,
+  InsufficientWorkspacePermissionsError,
+  NoWorkspaceAccessError,
+} from '@/lib/core/application'
+
+const writeOperation = defineWorkspaceOperation({
+  id: 'test.write',
+  minimumRole: 'write',
+  workspaceApiKey: 'deny',
+  principalKinds: ['session'],
+})
+
+const principal: SessionPrincipal = {
+  kind: 'session',
+  userId: 'user-1',
+  sessionId: 'session-1',
+}
+
+const context = {
+  workspaceId: 'workspace-1',
+  workspaceOrganizationId: 'organization-1',
+  allowPersonalApiKeys: true,
+}
+
+describe('authorizeWorkspaceOperation', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('rejects a null effective permission as no workspace access', async () => {
+    mocks.resolvePermission.mockResolvedValue(null)
+
+    await expect(
+      authorizeWorkspaceOperation(principal, writeOperation, context)
+    ).rejects.toBeInstanceOf(NoWorkspaceAccessError)
+  })
+
+  it('rejects a readable workspace as an insufficient role for a write operation', async () => {
+    mocks.resolvePermission.mockResolvedValue('read')
+
+    await expect(
+      authorizeWorkspaceOperation(principal, writeOperation, context)
+    ).rejects.toBeInstanceOf(InsufficientWorkspacePermissionsError)
+  })
+
+  it('authorizes the write operation when the current role satisfies it', async () => {
+    mocks.resolvePermission.mockResolvedValue('write')
+
+    await expect(
+      authorizeWorkspaceOperation(principal, writeOperation, context)
+    ).resolves.toBeUndefined()
+  })
+})