Skip to content

Commit 3664953

Browse files
fix(api): refine resource authorization boundaries
1 parent 57ebc89 commit 3664953

69 files changed

Lines changed: 565 additions & 146 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/sim/app/api/v2/custom-tools/[id]/route.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
*/
44
import { NextRequest } from 'next/server'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
import {
7+
InsufficientWorkspacePermissionsError,
8+
NoWorkspaceAccessError,
9+
} from '@/lib/core/application'
610

711
const { mocks, MockV2ApiKeyUnauthenticatedError } = vi.hoisted(() => {
812
class MockV2ApiKeyUnauthenticatedError extends Error {}
@@ -160,4 +164,15 @@ describe('/api/v2/custom-tools/[id]', () => {
160164
expect(response.status).toBe(401)
161165
expect(mocks.update).not.toHaveBeenCalled()
162166
})
167+
168+
it('conceals cross-tenant access while preserving same-workspace role denials', async () => {
169+
mocks.get.mockRejectedValueOnce(new NoWorkspaceAccessError())
170+
expect((await GET(request('GET'), context)).status).toBe(404)
171+
172+
mocks.update.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())
173+
expect(
174+
(await PATCH(request('PATCH', { workspaceId: WORKSPACE_ID, code: 'return 2' }), context))
175+
.status
176+
).toBe(403)
177+
})
163178
})

apps/sim/app/api/v2/custom-tools/[id]/route.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ import {
44
v2UpdateCustomToolContract,
55
} from '@/lib/api/contracts/v2/custom-tools'
66
import {
7+
createV2ResourceConcealmentPolicy,
78
defineV2JsonRoute,
89
v2ApiKeyAuth,
9-
v2OrchestrationErrorPolicy,
1010
v2RateLimits,
1111
} from '@/lib/api/server/routes'
1212
import { customToolOperations } from '@/lib/custom-tools/application/operations'
@@ -20,13 +20,17 @@ import { toV2CustomTool } from '@/app/api/v2/custom-tools/utils'
2020
export const dynamic = 'force-dynamic'
2121
export const revalidate = 0
2222

23+
const customToolResourceErrorPolicy = createV2ResourceConcealmentPolicy({
24+
notFoundMessage: 'Custom tool not found',
25+
})
26+
2327
/** GET /api/v2/custom-tools/[id] — Fetch a single custom tool. */
2428
export const GET = defineV2JsonRoute({
2529
contract: v2GetCustomToolContract,
2630
operation: customToolOperations.read,
2731
auth: v2ApiKeyAuth,
2832
rateLimit: v2RateLimits.publicApi,
29-
errorPolicy: v2OrchestrationErrorPolicy,
33+
errorPolicy: customToolResourceErrorPolicy,
3034
mapInput: ({ params, query }) => ({ workspaceId: query.workspaceId, toolId: params.id }),
3135
useCase: getWorkspaceCustomToolUseCase,
3236
present: ({ tool }) => ({ data: { customTool: toV2CustomTool(tool) } }),
@@ -38,7 +42,7 @@ export const PATCH = defineV2JsonRoute({
3842
operation: customToolOperations.update,
3943
auth: v2ApiKeyAuth,
4044
rateLimit: v2RateLimits.publicApi,
41-
errorPolicy: v2OrchestrationErrorPolicy,
45+
errorPolicy: customToolResourceErrorPolicy,
4246
mapInput: ({ params, body }) => ({
4347
...body,
4448
toolId: params.id,
@@ -54,7 +58,7 @@ export const DELETE = defineV2JsonRoute({
5458
operation: customToolOperations.delete,
5559
auth: v2ApiKeyAuth,
5660
rateLimit: v2RateLimits.publicApi,
57-
errorPolicy: v2OrchestrationErrorPolicy,
61+
errorPolicy: customToolResourceErrorPolicy,
5862
mapInput: ({ params, query }) => ({
5963
workspaceId: query.workspaceId,
6064
toolId: params.id,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ describe('PUT /api/v2/files/[fileId]/content', () => {
115115

116116
const response = await callPut('{not-json')
117117

118-
expect(response.status).toBe(403)
118+
expect(response.status).toBe(404)
119119
expect(mocks.admit).toHaveBeenCalledWith(auth.principal, FILE_ID)
120120
expect(mocks.updateContent).not.toHaveBeenCalled()
121121
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export const PUT = defineV2JsonRoute({
1919
auth: v2ApiKeyAuth,
2020
operation: fileOperations.updateContent,
2121
rateLimit: v2RateLimits.publicApi,
22-
errorPolicy: v2FileErrorPolicies.default,
22+
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
2323
parseOptions: {
2424
invalidJsonResponse: () => v2Error('BAD_REQUEST', 'Request body must be valid JSON'),
2525
maxBodyBytes: MAX_WORKSPACE_FILE_INLINE_BODY_BYTES,

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,13 @@ describe('GET /api/v2/files/[fileId]/metadata', () => {
113113
expect(mocks.readMetadata).not.toHaveBeenCalled()
114114
})
115115

116-
it('returns forbidden for an authorization failure', async () => {
116+
it('conceals cross-workspace authorization as not found', async () => {
117117
mocks.readMetadata.mockRejectedValue(new NoWorkspaceAccessError())
118118

119119
const response = await callGet(`workspaceId=${WORKSPACE_ID}`)
120120

121-
expect(response.status).toBe(403)
122-
expect((await response.json()).error.code).toBe('FORBIDDEN')
121+
expect(response.status).toBe(404)
122+
expect((await response.json()).error.code).toBe('NOT_FOUND')
123123
})
124124

125125
it('returns the v2 metadata projection through the shared use case', async () => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export const GET = defineV2JsonRoute({
1414
auth: v2ApiKeyAuth,
1515
operation: fileOperations.readMetadata,
1616
rateLimit: v2RateLimits.publicApi,
17-
errorPolicy: v2FileErrorPolicies.default,
17+
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
1818
mapInput: ({ params, query }) => ({
1919
fileId: params.fileId,
2020
assertedWorkspaceId: query.workspaceId,

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,16 +141,16 @@ describe('v2 single-file routes', () => {
141141
})
142142
})
143143

144-
it('returns forbidden for download authorization failures', async () => {
144+
it('conceals cross-workspace download authorization', async () => {
145145
mocks.download.mockRejectedValue(new NoWorkspaceAccessError())
146146

147147
const response = await GET(
148148
new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}?workspaceId=${WORKSPACE_ID}`),
149149
context
150150
)
151151

152-
expect(response.status).toBe(403)
153-
expect((await response.json()).error.code).toBe('FORBIDDEN')
152+
expect(response.status).toBe(404)
153+
expect((await response.json()).error.code).toBe('NOT_FOUND')
154154
})
155155

156156
it('renames through the shared use case and v2 presenter', async () => {
@@ -170,7 +170,7 @@ describe('v2 single-file routes', () => {
170170
})
171171
})
172172

173-
it('maps rename conflicts and returns forbidden for absent workspace access', async () => {
173+
it('maps rename conflicts and conceals absent workspace access', async () => {
174174
mocks.rename.mockRejectedValueOnce(new OrchestrationError('conflict', 'Name exists'))
175175
const conflict = await PATCH(
176176
new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}`, {
@@ -183,15 +183,15 @@ describe('v2 single-file routes', () => {
183183
expect(conflict.status).toBe(409)
184184

185185
mocks.rename.mockRejectedValueOnce(new NoWorkspaceAccessError())
186-
const forbidden = await PATCH(
186+
const concealed = await PATCH(
187187
new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}`, {
188188
method: 'PATCH',
189189
headers: { 'Content-Type': 'application/json' },
190190
body: JSON.stringify({ workspaceId: WORKSPACE_ID, name: 'renamed.csv' }),
191191
}),
192192
context
193193
)
194-
expect(forbidden.status).toBe(403)
194+
expect(concealed.status).toBe(404)
195195
})
196196

197197
it('returns forbidden when the current workspace role cannot rename the file', async () => {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export const GET = defineV2BinaryRoute({
3333
auth: v2ApiKeyAuth,
3434
operation: fileOperations.download,
3535
rateLimit: v2RateLimits.publicApi,
36-
errorPolicy: v2FileErrorPolicies.default,
36+
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
3737
mapInput: ({ params, query }) => ({
3838
fileId: params.fileId,
3939
assertedWorkspaceId: query.workspaceId,
@@ -59,7 +59,7 @@ export const PATCH = defineV2JsonRoute({
5959
auth: v2ApiKeyAuth,
6060
operation: fileOperations.rename,
6161
rateLimit: v2RateLimits.publicApi,
62-
errorPolicy: v2FileErrorPolicies.default,
62+
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
6363
mapInput: ({ params, body }) => ({
6464
fileId: params.fileId,
6565
assertedWorkspaceId: body.workspaceId,
@@ -81,7 +81,7 @@ export const DELETE = defineV2JsonRoute({
8181
auth: v2ApiKeyAuth,
8282
operation: fileOperations.delete,
8383
rateLimit: v2RateLimits.publicApi,
84-
errorPolicy: v2FileErrorPolicies.default,
84+
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
8585
mapInput: ({ params, query }) => ({
8686
fileId: params.fileId,
8787
assertedWorkspaceId: query.workspaceId,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export const GET = defineV2JsonRoute({
1515
auth: v2ApiKeyAuth,
1616
operation: fileOperations.readShare,
1717
rateLimit: v2RateLimits.publicApi,
18-
errorPolicy: v2FileErrorPolicies.default,
18+
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
1919
mapInput: ({ params, query }) => ({
2020
fileId: params.fileId,
2121
assertedWorkspaceId: query.workspaceId,
@@ -29,7 +29,7 @@ export const PUT = defineV2JsonRoute({
2929
auth: v2ApiKeyAuth,
3030
operation: fileOperations.updateShare,
3131
rateLimit: v2RateLimits.publicApi,
32-
errorPolicy: v2FileErrorPolicies.default,
32+
errorPolicy: v2FileErrorPolicies.concealResourceAuthorization,
3333
mapInput: ({ params, body }) => ({
3434
fileId: params.fileId,
3535
assertedWorkspaceId: body.workspaceId,

apps/sim/app/api/v2/files/uploads/utils.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,20 @@ function uploadStatus(status: string): V2UploadStatus {
3838

3939
import type { Principal } from '@sim/auth/principal'
4040
import type { NextRequest, NextResponse } from 'next/server'
41+
import { createV2ResourceConcealmentPolicy } from '@/lib/api/server/routes'
4142
import { authenticateV2ApiKey } from '@/lib/api/server/routes/v2-api-key-auth'
42-
import { v2CaughtOrchestrationError } from '@/app/api/v2/lib/response'
43+
44+
const uploadControlErrorPolicy = createV2ResourceConcealmentPolicy({
45+
notFoundMessage: 'Upload session not found',
46+
})
4347

4448
/** Re-authenticates the API key for each upload control leg. */
4549
export async function authenticateUploadPrincipal(request: NextRequest): Promise<Principal> {
4650
const auth = await authenticateV2ApiKey(request.headers.get('x-api-key'))
4751
return auth.principal
4852
}
4953

50-
/** Renders upload-control application failures without rewriting authorization status. */
54+
/** Conceals cross-tenant upload-session authorization while preserving same-workspace denials. */
5155
export function v2UploadControlError(error: unknown): NextResponse | null {
52-
return v2CaughtOrchestrationError(error)
56+
return uploadControlErrorPolicy.render(error)
5357
}

0 commit comments

Comments
 (0)