Skip to content

Commit 083319b

Browse files
waleedlatif1waleedlatif1
authored
fix(api): conceal cross-tenant resource denials on internal routes (#6586)
The v2 routes rewrite DelegatedWorkspaceAuthorizationError, NoWorkspaceAccessError, and WorkspaceApiKeyScopeAuthorizationError to a 404 so a caller with no reach into a workspace cannot confirm a resource exists. The internal routes reach the same application use cases and still answered 403, so the same probe worked from the other surface. Same-workspace role denials stay 403 on both.
1 parent 5e1862e commit 083319b

37 files changed

Lines changed: 778 additions & 278 deletions

File tree

‎apps/sim/app/api/table/[tableId]/exports/route.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import { createTableExportResourceContract } from '@/lib/api/contracts/table-transfers'
2-
import {
3-
defineInternalJsonRoute,
4-
internalOrchestrationErrorPolicy,
5-
internalRateLimits,
6-
} from '@/lib/api/server/routes'
7-
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
2+
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
3+
import { internalTableErrorPolicies, internalTableSessionOrExecutorAuth } from '@/lib/table/api'
84
import { createTableExportUseCase } from '@/lib/table/application/exports'
95
import { tableOperations } from '@/lib/table/application/operations'
106
import { toV2TableExport } from '@/lib/table/orchestration/export-resource'
@@ -16,7 +12,7 @@ export const POST = defineInternalJsonRoute({
1612
rateLimit: internalRateLimits.none({
1713
reason: 'Existing authenticated table export creation has no request-rate policy',
1814
}),
19-
errorPolicy: internalOrchestrationErrorPolicy,
15+
errorPolicy: internalTableErrorPolicies.concealTableAuthorization,
2016
mapInput: ({ params, body }) => ({
2117
tableId: params.tableId,
2218
workspaceId: body.workspaceId,

‎apps/sim/app/api/table/[tableId]/groups/route.test.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { describe, expect, it, vi } from 'vitest'
66
interface CapturedDefinition {
77
contract: { method: string; path: string }
88
auth: unknown
9+
errorPolicy: unknown
910
operation: { id: string }
1011
useCase: unknown
1112
mapInput(input: {
@@ -21,6 +22,7 @@ interface CapturedDefinition {
2122

2223
const mocks = vi.hoisted(() => ({
2324
auth: { kind: 'session-or-executor' },
25+
concealTableGroupAuthorization: { kind: 'conceal-table-group' },
2426
definitions: [] as CapturedDefinition[],
2527
useCases: {
2628
create: { operation: { id: 'tables.groups.create' } },
@@ -34,15 +36,17 @@ vi.mock('@/lib/api/server/routes', () => ({
3436
mocks.definitions.push(definition)
3537
return vi.fn()
3638
},
37-
extendInternalErrorPolicy: vi.fn(() => ({ kind: 'table' })),
38-
internalErrorResponse: vi.fn(),
39-
internalOrchestrationErrorPolicy: { kind: 'plain' },
4039
internalRateLimits: {
4140
none: ({ reason }: { reason: string }) => ({ kind: 'none', reason }),
4241
},
4342
}))
4443

45-
vi.mock('@/lib/table/api', () => ({ internalTableSessionOrExecutorAuth: mocks.auth }))
44+
vi.mock('@/lib/table/api', () => ({
45+
internalTableErrorPolicies: {
46+
concealTableGroupAuthorization: mocks.concealTableGroupAuthorization,
47+
},
48+
internalTableSessionOrExecutorAuth: mocks.auth,
49+
}))
4650

4751
vi.mock('@/lib/table/application/groups', () => ({
4852
createTableGroupUseCase: mocks.useCases.create,
@@ -77,6 +81,7 @@ describe('/api/table/[tableId]/groups', () => {
7781
expect(route.auth).toBe(mocks.auth)
7882
expect(route.useCase).toBe(useCase)
7983
expect(route.operation.id).toBe(useCase.operation.id)
84+
expect(route.errorPolicy).toBe(mocks.concealTableGroupAuthorization)
8085
}
8186
})
8287

‎apps/sim/app/api/table/[tableId]/groups/route.ts‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,17 @@ import {
33
deleteWorkflowGroupContract,
44
updateWorkflowGroupContract,
55
} from '@/lib/api/contracts/tables'
6-
import {
7-
defineInternalJsonRoute,
8-
extendInternalErrorPolicy,
9-
internalErrorResponse,
10-
internalOrchestrationErrorPolicy,
11-
internalRateLimits,
12-
} from '@/lib/api/server/routes'
13-
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
6+
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
7+
import { internalTableErrorPolicies, internalTableSessionOrExecutorAuth } from '@/lib/table/api'
148
import {
159
createTableGroupUseCase,
1610
deleteTableGroupUseCase,
1711
updateTableGroupUseCase,
1812
} from '@/lib/table/application/groups'
1913
import { tableOperations } from '@/lib/table/application/operations'
20-
import { TableLockedError } from '@/lib/table/mutation-locks'
2114
import type { TableDefinition } from '@/lib/table/types'
2215
import { normalizeColumn } from '@/app/api/table/utils'
2316

24-
const errorPolicy = extendInternalErrorPolicy(internalOrchestrationErrorPolicy, (error) =>
25-
error instanceof TableLockedError
26-
? internalErrorResponse(423, { error: error.message, lock: error.lock })
27-
: null
28-
)
29-
3017
const rateLimit = internalRateLimits.none({
3118
reason: 'Existing authenticated table group mutations have no request-rate policy',
3219
})
@@ -47,7 +34,7 @@ export const POST = defineInternalJsonRoute({
4734
useCase: createTableGroupUseCase,
4835
auth: internalTableSessionOrExecutorAuth,
4936
rateLimit,
50-
errorPolicy,
37+
errorPolicy: internalTableErrorPolicies.concealTableGroupAuthorization,
5138
mapInput: ({ params, body }) => ({
5239
tableId: params.tableId,
5340
...body,
@@ -62,7 +49,7 @@ export const PATCH = defineInternalJsonRoute({
6249
useCase: updateTableGroupUseCase,
6350
auth: internalTableSessionOrExecutorAuth,
6451
rateLimit,
65-
errorPolicy,
52+
errorPolicy: internalTableErrorPolicies.concealTableGroupAuthorization,
6653
mapInput: ({ params, body }) => ({ tableId: params.tableId, ...body }),
6754
present: ({ table }) => presentTable(table),
6855
})
@@ -73,7 +60,7 @@ export const DELETE = defineInternalJsonRoute({
7360
useCase: deleteTableGroupUseCase,
7461
auth: internalTableSessionOrExecutorAuth,
7562
rateLimit,
76-
errorPolicy,
63+
errorPolicy: internalTableErrorPolicies.concealTableGroupAuthorization,
7764
mapInput: ({ params, body }) => ({ tableId: params.tableId, ...body }),
7865
present: ({ table }) => presentTable(table),
7966
})

‎apps/sim/app/api/table/exports/[exportId]/download/route.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import { downloadTableExportResourceContract } from '@/lib/api/contracts/table-transfers'
2-
import {
3-
defineInternalJsonRoute,
4-
internalOrchestrationErrorPolicy,
5-
internalRateLimits,
6-
} from '@/lib/api/server/routes'
7-
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
2+
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
3+
import { internalTableErrorPolicies, internalTableSessionOrExecutorAuth } from '@/lib/table/api'
84
import { downloadTableExportUseCase } from '@/lib/table/application/exports'
95
import { tableOperations } from '@/lib/table/application/operations'
106

@@ -15,7 +11,7 @@ export const GET = defineInternalJsonRoute({
1511
rateLimit: internalRateLimits.none({
1612
reason: 'Existing authenticated table export download signing has no request-rate policy',
1713
}),
18-
errorPolicy: internalOrchestrationErrorPolicy,
14+
errorPolicy: internalTableErrorPolicies.concealExportAuthorization,
1915
mapInput: ({ params, query }) => ({
2016
exportId: params.exportId,
2117
workspaceId: query.workspaceId,

‎apps/sim/app/api/table/exports/[exportId]/route.ts‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,8 @@ import {
22
cancelTableExportResourceContract,
33
getTableExportResourceContract,
44
} from '@/lib/api/contracts/table-transfers'
5-
import {
6-
defineInternalJsonRoute,
7-
internalOrchestrationErrorPolicy,
8-
internalRateLimits,
9-
} from '@/lib/api/server/routes'
10-
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
5+
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
6+
import { internalTableErrorPolicies, internalTableSessionOrExecutorAuth } from '@/lib/table/api'
117
import { cancelTableExportUseCase, readTableExportUseCase } from '@/lib/table/application/exports'
128
import { tableOperations } from '@/lib/table/application/operations'
139
import { toV2TableExport } from '@/lib/table/orchestration/export-resource'
@@ -21,7 +17,7 @@ export const GET = defineInternalJsonRoute({
2117
auth: internalTableSessionOrExecutorAuth,
2218
operation: tableOperations.readExport,
2319
rateLimit,
24-
errorPolicy: internalOrchestrationErrorPolicy,
20+
errorPolicy: internalTableErrorPolicies.concealExportAuthorization,
2521
mapInput: ({ params, query }) => ({
2622
exportId: params.exportId,
2723
workspaceId: query.workspaceId,
@@ -35,7 +31,7 @@ export const DELETE = defineInternalJsonRoute({
3531
auth: internalTableSessionOrExecutorAuth,
3632
operation: tableOperations.cancelExport,
3733
rateLimit,
38-
errorPolicy: internalOrchestrationErrorPolicy,
34+
errorPolicy: internalTableErrorPolicies.concealExportAuthorization,
3935
mapInput: ({ params, query }) => ({
4036
exportId: params.exportId,
4137
workspaceId: query.workspaceId,

‎apps/sim/app/api/table/imports/[importId]/complete/route.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import { completeTableImportResourceContract } from '@/lib/api/contracts/table-transfers'
2-
import {
3-
defineInternalJsonRoute,
4-
internalOrchestrationErrorPolicy,
5-
internalRateLimits,
6-
} from '@/lib/api/server/routes'
7-
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
2+
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
3+
import { internalTableErrorPolicies, internalTableSessionOrExecutorAuth } from '@/lib/table/api'
84
import { completeTableImportUseCase } from '@/lib/table/application/imports'
95
import { tableOperations } from '@/lib/table/application/operations'
106
import { toV2TableImport } from '@/lib/table/orchestration/import-resource'
@@ -16,7 +12,7 @@ export const POST = defineInternalJsonRoute({
1612
rateLimit: internalRateLimits.none({
1713
reason: 'Existing authenticated table import completion has no request-rate policy',
1814
}),
19-
errorPolicy: internalOrchestrationErrorPolicy,
15+
errorPolicy: internalTableErrorPolicies.concealImportAuthorization,
2016
mapInput: ({ params, query, headers }) => ({
2117
importId: params.importId,
2218
workspaceId: query.workspaceId,

‎apps/sim/app/api/table/imports/[importId]/parts/route.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import { createTableImportPartUrlsContract } from '@/lib/api/contracts/table-transfers'
2-
import {
3-
defineInternalJsonRoute,
4-
internalOrchestrationErrorPolicy,
5-
internalRateLimits,
6-
} from '@/lib/api/server/routes'
7-
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
2+
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
3+
import { internalTableErrorPolicies, internalTableSessionOrExecutorAuth } from '@/lib/table/api'
84
import { createTableImportPartsUseCase } from '@/lib/table/application/imports'
95
import { tableOperations } from '@/lib/table/application/operations'
106

@@ -15,7 +11,7 @@ export const POST = defineInternalJsonRoute({
1511
rateLimit: internalRateLimits.none({
1612
reason: 'Existing authenticated table import part signing has no request-rate policy',
1713
}),
18-
errorPolicy: internalOrchestrationErrorPolicy,
14+
errorPolicy: internalTableErrorPolicies.concealImportAuthorization,
1915
mapInput: ({ params, query, headers, body }) => ({
2016
importId: params.importId,
2117
workspaceId: query.workspaceId,

‎apps/sim/app/api/table/imports/[importId]/route.ts‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,8 @@ import {
22
cancelTableImportResourceContract,
33
getTableImportResourceContract,
44
} from '@/lib/api/contracts/table-transfers'
5-
import {
6-
defineInternalJsonRoute,
7-
internalOrchestrationErrorPolicy,
8-
internalRateLimits,
9-
} from '@/lib/api/server/routes'
10-
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
5+
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
6+
import { internalTableErrorPolicies, internalTableSessionOrExecutorAuth } from '@/lib/table/api'
117
import { cancelTableImportUseCase, readTableImportUseCase } from '@/lib/table/application/imports'
128
import { tableOperations } from '@/lib/table/application/operations'
139
import { toV2TableImport } from '@/lib/table/orchestration/import-resource'
@@ -21,7 +17,7 @@ export const GET = defineInternalJsonRoute({
2117
auth: internalTableSessionOrExecutorAuth,
2218
operation: tableOperations.readImport,
2319
rateLimit,
24-
errorPolicy: internalOrchestrationErrorPolicy,
20+
errorPolicy: internalTableErrorPolicies.concealImportAuthorization,
2521
mapInput: ({ params, query }) => ({
2622
importId: params.importId,
2723
workspaceId: query.workspaceId,
@@ -35,7 +31,7 @@ export const DELETE = defineInternalJsonRoute({
3531
auth: internalTableSessionOrExecutorAuth,
3632
operation: tableOperations.cancelImport,
3733
rateLimit,
38-
errorPolicy: internalOrchestrationErrorPolicy,
34+
errorPolicy: internalTableErrorPolicies.concealImportAuthorization,
3935
mapInput: ({ params, query, headers }) => ({
4036
importId: params.importId,
4137
workspaceId: query.workspaceId,

‎apps/sim/app/api/table/imports/route.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import { createTableImportResourceContract } from '@/lib/api/contracts/table-transfers'
2-
import {
3-
defineInternalJsonRoute,
4-
internalOrchestrationErrorPolicy,
5-
internalRateLimits,
6-
} from '@/lib/api/server/routes'
7-
import { internalTableSessionOrExecutorAuth } from '@/lib/table/api'
2+
import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes'
3+
import { internalTableErrorPolicies, internalTableSessionOrExecutorAuth } from '@/lib/table/api'
84
import { createTableImportUseCase } from '@/lib/table/application/imports'
95
import { tableOperations } from '@/lib/table/application/operations'
106
import { toV2CreateTableImport } from '@/lib/table/orchestration/import-resource'
@@ -16,7 +12,7 @@ export const POST = defineInternalJsonRoute({
1612
rateLimit: internalRateLimits.none({
1713
reason: 'Existing authenticated table import creation has no request-rate policy',
1814
}),
19-
errorPolicy: internalOrchestrationErrorPolicy,
15+
errorPolicy: internalTableErrorPolicies.concealTableAuthorization,
2016
mapInput: ({ body }) => ({ body }),
2117
useCase: createTableImportUseCase,
2218
present: ({ import: created }) => ({ data: toV2CreateTableImport(created) }),

‎apps/sim/app/api/table/table-transfer-routes.test.ts‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,19 @@ interface CapturedDefinition {
1010
response: { status?: number }
1111
}
1212
auth: unknown
13+
errorPolicy: unknown
1314
operation: { id: string }
1415
useCase: unknown
1516
}
1617

1718
const mocks = vi.hoisted(() => ({
1819
auth: { kind: 'session-or-executor' },
1920
definitions: [] as CapturedDefinition[],
21+
errorPolicies: {
22+
concealTableAuthorization: { kind: 'conceal-table' },
23+
concealImportAuthorization: { kind: 'conceal-import' },
24+
concealExportAuthorization: { kind: 'conceal-export' },
25+
},
2026
useCases: {
2127
cancelExport: { operation: { id: 'tables.exports.cancel' } },
2228
cancelImport: { operation: { id: 'tables.imports.cancel' } },
@@ -41,7 +47,10 @@ vi.mock('@/lib/api/server/routes', () => ({
4147
},
4248
}))
4349

44-
vi.mock('@/lib/table/api', () => ({ internalTableSessionOrExecutorAuth: mocks.auth }))
50+
vi.mock('@/lib/table/api', () => ({
51+
internalTableErrorPolicies: mocks.errorPolicies,
52+
internalTableSessionOrExecutorAuth: mocks.auth,
53+
}))
4554

4655
vi.mock('@/lib/table/application/imports', () => ({
4756
cancelTableImportUseCase: mocks.useCases.cancelImport,
@@ -106,6 +115,36 @@ describe('internal table transfer routes', () => {
106115
}
107116
})
108117

118+
it('conceals cross-tenant authorization on every table transfer control leg', () => {
119+
const expected = [
120+
['POST', '/api/table/imports', mocks.errorPolicies.concealTableAuthorization],
121+
['GET', '/api/table/imports/[importId]', mocks.errorPolicies.concealImportAuthorization],
122+
['DELETE', '/api/table/imports/[importId]', mocks.errorPolicies.concealImportAuthorization],
123+
[
124+
'POST',
125+
'/api/table/imports/[importId]/parts',
126+
mocks.errorPolicies.concealImportAuthorization,
127+
],
128+
[
129+
'POST',
130+
'/api/table/imports/[importId]/complete',
131+
mocks.errorPolicies.concealImportAuthorization,
132+
],
133+
['POST', '/api/table/[tableId]/exports', mocks.errorPolicies.concealTableAuthorization],
134+
['GET', '/api/table/exports/[exportId]', mocks.errorPolicies.concealExportAuthorization],
135+
['DELETE', '/api/table/exports/[exportId]', mocks.errorPolicies.concealExportAuthorization],
136+
[
137+
'GET',
138+
'/api/table/exports/[exportId]/download',
139+
mocks.errorPolicies.concealExportAuthorization,
140+
],
141+
] as const
142+
143+
for (const [method, path, errorPolicy] of expected) {
144+
expect(definition(method, path).errorPolicy).toBe(errorPolicy)
145+
}
146+
})
147+
109148
it('preserves the create response statuses', () => {
110149
expect(definition('POST', '/api/table/imports').contract.response.status).toBe(201)
111150
expect(definition('POST', '/api/table/[tableId]/exports').contract.response.status).toBe(201)

0 commit comments

Comments
 (0)