Skip to content

Commit ebccc99

Browse files
committed
fix(v2): conceal workspace absence, and stop archived tables faulting their page
Two reads answered a caller more than they were entitled to know. A workspace a caller cannot reach at all returned FORBIDDEN while one that does not exist returned NOT_FOUND, so a workspace-key holder could enumerate which workspace ids exist by diffing the two. Both now answer the same absence, using the concealment policy the billing routes already use. A refusal from inside the workspace - a member whose role is too low - still answers FORBIDDEN, because that caller already knows the workspace exists. Separately, archiving a folder cascades onto its tables but leaves each table pointing at the archived folder row. The archived listing resolved those paths strictly, so one such row faulted the whole page and no cursor could step past it - which also made the ids undiscoverable and left restore unreachable for exactly the tables that need it. The archived scope now resolves leniently to the root, where a restore would place them, matching the shipped workflows behavior. Active listings still fault loudly on a dangling folder.
1 parent 5e20bf4 commit ebccc99

9 files changed

Lines changed: 300 additions & 17 deletions

File tree

apps/sim/app/api/v2/workspaces/[workspaceId]/members/route.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,9 @@ import {
33
v2WorkspaceMemberCursorSchema,
44
} from '@/lib/api/contracts/v2/workspaces'
55
import { cursorRoute, cursorScopeKey, UNREADABLE_CURSOR_MESSAGE } from '@/lib/api/cursor-binding'
6-
import {
7-
defineV2JsonRoute,
8-
v2ApiKeyAuth,
9-
v2OrchestrationErrorPolicy,
10-
v2RateLimits,
11-
} from '@/lib/api/server/routes'
6+
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
127
import { OrchestrationError } from '@/lib/core/orchestration/types'
8+
import { v2WorkspaceErrorPolicies } from '@/lib/workspaces/api/route-policies'
139
import { listPublicWorkspaceMembers } from '@/lib/workspaces/application/list-public-workspace-members'
1410
import { workspaceOperations } from '@/lib/workspaces/application/operations'
1511
import {
@@ -39,7 +35,7 @@ export const GET = defineV2JsonRoute({
3935
auth: v2ApiKeyAuth,
4036
operation: workspaceOperations.listPublicMembers,
4137
rateLimit: v2RateLimits.publicApi,
42-
errorPolicy: v2OrchestrationErrorPolicy,
38+
errorPolicy: v2WorkspaceErrorPolicies.concealWorkspaceAuthorization,
4339
mapInput: ({ params, query }) => {
4440
const inner = readScopedCursor(query.cursor, memberCursorScope(params.workspaceId))
4541
const decoded = inner ? v2WorkspaceMemberCursorSchema.safeParse(decodeCursor(inner)) : undefined
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import {
5+
V2_OPERATION_RATE_LIMIT_ALLOWED,
6+
V2_PREAUTH_RATE_LIMIT_ALLOWED,
7+
v2ApiKeyAuthModuleMock,
8+
v2RateLimiterModuleMock,
9+
v2RouteMocks,
10+
} from '@sim/testing'
11+
import { NextRequest } from 'next/server'
12+
import { beforeEach, describe, expect, it, vi } from 'vitest'
13+
14+
const mocks = vi.hoisted(() => ({
15+
getWorkspace: vi.fn(),
16+
listMembers: vi.fn(),
17+
}))
18+
19+
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => v2ApiKeyAuthModuleMock)
20+
vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock)
21+
22+
vi.mock('@/lib/workspaces/application/get-public-workspace', () => ({
23+
getPublicWorkspace: {
24+
operation: { id: 'workspaces.read_public_detail' },
25+
execute: mocks.getWorkspace,
26+
},
27+
}))
28+
29+
vi.mock('@/lib/workspaces/application/list-public-workspace-members', () => ({
30+
listPublicWorkspaceMembers: {
31+
operation: { id: 'workspaces.members.list_public' },
32+
execute: mocks.listMembers,
33+
},
34+
}))
35+
36+
import {
37+
InsufficientWorkspacePermissionsError,
38+
NoWorkspaceAccessError,
39+
WorkspaceApiKeyScopeAuthorizationError,
40+
} from '@/lib/core/application'
41+
import { OrchestrationError } from '@/lib/core/orchestration/types'
42+
import { GET as listMembers } from '@/app/api/v2/workspaces/[workspaceId]/members/route'
43+
import { GET as getWorkspace } from '@/app/api/v2/workspaces/[workspaceId]/route'
44+
45+
const WORKSPACE_ID = '6fc7631d-88cd-46f8-9f0a-d4764daef7f8'
46+
const auth = {
47+
principal: {
48+
kind: 'workspace_api_key' as const,
49+
workspaceId: WORKSPACE_ID,
50+
keyId: 'key-1',
51+
},
52+
rateLimitSubjectIds: ['api-key:key-1', `workspace:${WORKSPACE_ID}`] as const,
53+
rateLimitSubscription: null,
54+
keyType: 'workspace' as const,
55+
}
56+
57+
/**
58+
* The two reads a workspace id is addressable through. Both must conceal the
59+
* same way, or the pair that still answers `403` is the oracle.
60+
*/
61+
const routes = [
62+
{
63+
name: 'workspace detail',
64+
spy: mocks.getWorkspace,
65+
call: () =>
66+
getWorkspace(new NextRequest(`http://localhost:3000/api/v2/workspaces/${WORKSPACE_ID}`), {
67+
params: Promise.resolve({ workspaceId: WORKSPACE_ID }),
68+
}),
69+
},
70+
{
71+
name: 'member roster',
72+
spy: mocks.listMembers,
73+
call: () =>
74+
listMembers(
75+
new NextRequest(`http://localhost:3000/api/v2/workspaces/${WORKSPACE_ID}/members`),
76+
{ params: Promise.resolve({ workspaceId: WORKSPACE_ID }) }
77+
),
78+
},
79+
] as const
80+
81+
describe.each(routes)('v2 $name workspace concealment', ({ spy, call }) => {
82+
beforeEach(() => {
83+
vi.clearAllMocks()
84+
v2RouteMocks.authenticate.mockResolvedValue(auth)
85+
v2RouteMocks.preauthRate.mockResolvedValue(V2_PREAUTH_RATE_LIMIT_ALLOWED)
86+
v2RouteMocks.operationRate.mockResolvedValue(V2_OPERATION_RATE_LIMIT_ALLOWED)
87+
})
88+
89+
/**
90+
* Asserted as equality between the two responses rather than against a
91+
* literal: the leak is the DIFFERENCE, so a future rewording of either leg
92+
* must not be able to reintroduce it while the test still passes.
93+
*/
94+
it.each([
95+
['a workspace key scoped elsewhere', () => new WorkspaceApiKeyScopeAuthorizationError()],
96+
['a non-member personal key', () => new NoWorkspaceAccessError()],
97+
])('answers an unreachable workspace exactly as an absent one for %s', async (_label, raise) => {
98+
spy.mockRejectedValueOnce(new OrchestrationError('not_found', 'Workspace not found'))
99+
const absent = await call()
100+
const absentBody = await absent.json()
101+
102+
spy.mockRejectedValueOnce(raise())
103+
const unreachable = await call()
104+
105+
expect(unreachable.status).toBe(absent.status)
106+
expect(await unreachable.json()).toEqual(absentBody)
107+
expect(absent.status).toBe(404)
108+
expect(absentBody).toEqual({
109+
error: { code: 'NOT_FOUND', message: 'Workspace not found' },
110+
})
111+
})
112+
113+
/**
114+
* The negative leg. A caller already inside the workspace knows it exists, so
115+
* a role refusal stays an actionable `403` — concealing it too would widen the
116+
* policy past what it is for.
117+
*/
118+
it('still refuses an in-workspace role denial with 403', async () => {
119+
spy.mockRejectedValueOnce(new InsufficientWorkspacePermissionsError())
120+
121+
const response = await call()
122+
123+
expect(response.status).toBe(403)
124+
expect(await response.json()).toMatchObject({ error: { code: 'FORBIDDEN' } })
125+
})
126+
})

apps/sim/app/api/v2/workspaces/[workspaceId]/route.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import { v2GetWorkspaceContract } from '@/lib/api/contracts/v2/workspaces'
2-
import {
3-
defineV2JsonRoute,
4-
v2ApiKeyAuth,
5-
v2OrchestrationErrorPolicy,
6-
v2RateLimits,
7-
} from '@/lib/api/server/routes'
2+
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
3+
import { v2WorkspaceErrorPolicies } from '@/lib/workspaces/api/route-policies'
84
import { getPublicWorkspace } from '@/lib/workspaces/application/get-public-workspace'
95
import { workspaceOperations } from '@/lib/workspaces/application/operations'
106

@@ -14,7 +10,7 @@ export const GET = defineV2JsonRoute({
1410
auth: v2ApiKeyAuth,
1511
operation: workspaceOperations.readPublicDetail,
1612
rateLimit: v2RateLimits.publicApi,
17-
errorPolicy: v2OrchestrationErrorPolicy,
13+
errorPolicy: v2WorkspaceErrorPolicies.concealWorkspaceAuthorization,
1814
mapInput: ({ params }) => ({ workspaceId: params.workspaceId }),
1915
useCase: getPublicWorkspace,
2016
present: ({ workspace }) => ({

apps/sim/lib/copilot/tools/server/table/user-table.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ vi.mock('@/lib/table/application/folder-paths', () => ({
169169
index: { idByPath: new Map(), pathById: new Map() },
170170
}),
171171
tableFolderPathForId: () => '/',
172+
archivableTableFolderPath: () => '/',
172173
}))
173174

174175
vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () => ({
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import type { folder as folderTable } from '@sim/db/schema'
5+
import { describe, expect, it } from 'vitest'
6+
import { buildFolderPathIndex } from '@/lib/folders/paths'
7+
import {
8+
archivableTableFolderPath,
9+
tableFolderPathForId,
10+
} from '@/lib/table/application/folder-paths'
11+
12+
const activeFolder = {
13+
id: 'folder-active',
14+
resourceType: 'table' as const,
15+
name: 'Reports',
16+
userId: 'owner-1',
17+
workspaceId: 'workspace-1',
18+
parentId: null,
19+
sortOrder: 0,
20+
locked: false,
21+
createdAt: new Date('2026-01-01T00:00:00Z'),
22+
updatedAt: new Date('2026-01-01T00:00:00Z'),
23+
deletedAt: null,
24+
} as typeof folderTable.$inferSelect
25+
26+
/**
27+
* The index the listing actually projects against: `loadActiveFolderPathIndex`
28+
* filters on `isNull(deletedAt)`, so an archived folder is absent from it by
29+
* construction and its id dangles.
30+
*/
31+
const index = buildFolderPathIndex([activeFolder])
32+
const ARCHIVED_FOLDER_ID = 'folder-archived'
33+
34+
describe('table folder path projection', () => {
35+
/**
36+
* The scoping proof. A live table pointing at an unresolvable folder is a
37+
* genuine inconsistency, so the strict projector every active-only call site
38+
* uses must keep throwing on the very input the lenient one tolerates.
39+
*/
40+
it('throws on a dangling folder when the table is expected to be active', () => {
41+
expect(() => tableFolderPathForId(index, ARCHIVED_FOLDER_ID)).toThrow(
42+
'Table references an inactive or missing folder'
43+
)
44+
})
45+
46+
it('answers the root path instead, which is where restore would place it', () => {
47+
expect(archivableTableFolderPath(index, ARCHIVED_FOLDER_ID)).toBe('/')
48+
})
49+
50+
it('still resolves a folder that is active', () => {
51+
expect(archivableTableFolderPath(index, activeFolder.id)).toBe('/Reports')
52+
})
53+
54+
it('treats no folder as the root', () => {
55+
expect(archivableTableFolderPath(index, null)).toBe('/')
56+
expect(archivableTableFolderPath(index, undefined)).toBe('/')
57+
})
58+
})

apps/sim/lib/table/application/folder-paths.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,25 @@ export function tableFolderPathForId(
3434
if (!path) throw new Error('Table references an inactive or missing folder')
3535
return path
3636
}
37+
38+
/**
39+
* The same projection for a table that may itself be archived.
40+
*
41+
* Archiving a folder cascades onto the tables inside it but leaves their
42+
* `folderId` pointing at the now-inactive row — which is exactly why restore has
43+
* to re-root a dangling `folderId`. So on any read that can surface an archived
44+
* table, an unresolvable folder is the expected state rather than the
45+
* inconsistency {@link tableFolderPathForId} treats it as, and one such row
46+
* would otherwise throw a bare `Error` and 500 the whole page with no cursor
47+
* position able to skip past it.
48+
*
49+
* The root is the honest answer: it is where restore would put the table if the
50+
* caller restored it now.
51+
*/
52+
export function archivableTableFolderPath(
53+
index: FolderPathIndex<FolderRow>,
54+
folderId: string | null | undefined
55+
): string {
56+
if (!folderId) return ROOT_FOLDER_PATH
57+
return index.pathById.get(folderId) ?? ROOT_FOLDER_PATH
58+
}

apps/sim/lib/table/application/tables.test.ts

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,19 @@ vi.mock('@/lib/table/application/context', () => ({
5959
resolveTableWorkspaceContext: mocks.resolveWorkspaceContext,
6060
}))
6161

62+
/**
63+
* The two projectors are deliberately distinguishable here: the strict one
64+
* reproduces the bare `Error` a dangling `folderId` raises in production, so a
65+
* listing that reaches for the wrong one fails the test the same way it 500s
66+
* the page.
67+
*/
6268
vi.mock('@/lib/table/application/folder-paths', () => ({
6369
resolveTableFolderPath: vi.fn(),
64-
tableFolderPathForId: () => '/',
70+
tableFolderPathForId: (_index: unknown, folderId: string | null | undefined) => {
71+
if (folderId) throw new Error('Table references an inactive or missing folder')
72+
return '/'
73+
},
74+
archivableTableFolderPath: () => '/',
6575
}))
6676

6777
vi.mock('@/lib/table/events', () => ({ signalTableSchemaChanged: mocks.signal }))
@@ -102,6 +112,59 @@ describe('table list scope', () => {
102112
mocks.queryTables.mockResolvedValue({ tables: [], nextKeys: null })
103113
})
104114

115+
/**
116+
* Archiving a folder cascades onto its tables but leaves each `folderId`
117+
* pointing at the soft-deleted row, so the archived scope is exactly the
118+
* population whose folder cannot resolve. Projected strictly, one such row
119+
* threw and 500'd the whole page — and no cursor position could step past it,
120+
* which made every archived table id undiscoverable and `restore` unreachable.
121+
*/
122+
it('renders an archived table whose folder was archived too at the root', async () => {
123+
mocks.queryTables.mockResolvedValue({
124+
tables: [{ ...ARCHIVED, folderId: 'folder-archived' }],
125+
nextKeys: null,
126+
})
127+
128+
const result = await listTablesUseCase.execute({
129+
principal: PRINCIPAL,
130+
input: {
131+
workspaceId: 'workspace-1',
132+
scope: 'archived',
133+
sortBy: 'createdAt',
134+
sortOrder: 'asc',
135+
limit: 10,
136+
},
137+
})
138+
139+
expect(result.tables).toEqual([
140+
{ table: { ...ARCHIVED, folderId: 'folder-archived' }, folderPath: '/' },
141+
])
142+
})
143+
144+
/**
145+
* The negative leg. A LIVE table pointing at a folder that does not resolve is
146+
* a genuine inconsistency, so the active listing must stay loud rather than
147+
* quietly re-rooting it.
148+
*/
149+
it('still fails loudly on a dangling folder in the active listing', async () => {
150+
mocks.queryTables.mockResolvedValue({
151+
tables: [{ ...ARCHIVED, archivedAt: null, folderId: 'folder-archived' }],
152+
nextKeys: null,
153+
})
154+
155+
await expect(
156+
listTablesUseCase.execute({
157+
principal: PRINCIPAL,
158+
input: {
159+
workspaceId: 'workspace-1',
160+
sortBy: 'createdAt',
161+
sortOrder: 'asc',
162+
limit: 10,
163+
},
164+
})
165+
).rejects.toThrow('Table references an inactive or missing folder')
166+
})
167+
105168
it('lets the caller scope the listing without changing the default', async () => {
106169
await listTablesUseCase.execute({
107170
principal: PRINCIPAL,

apps/sim/lib/table/application/tables.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ import {
2626
resolveArchivedTableContext,
2727
resolveTableWorkspaceContext,
2828
} from '@/lib/table/application/context'
29-
import { resolveTableFolderPath, tableFolderPathForId } from '@/lib/table/application/folder-paths'
29+
import {
30+
archivableTableFolderPath,
31+
resolveTableFolderPath,
32+
tableFolderPathForId,
33+
} from '@/lib/table/application/folder-paths'
3034
import { tableOperations } from '@/lib/table/application/operations'
3135
import { signalTableSchemaChanged } from '@/lib/table/events'
3236

@@ -68,7 +72,10 @@ export const listTablesUseCase = defineAuthorizedTableUseCase({
6872
return {
6973
tables: tables.map((table) => ({
7074
table,
71-
folderPath: tableFolderPathForId(folderIndex, table.folderId),
75+
folderPath:
76+
input.scope === 'archived'
77+
? archivableTableFolderPath(folderIndex, table.folderId)
78+
: tableFolderPathForId(folderIndex, table.folderId),
7279
})),
7380
nextKeys,
7481
sortBy: input.sortBy,
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { createV2ResourceConcealmentPolicy } from '@/lib/api/server/routes'
2+
3+
/**
4+
* A caller naming a workspace it cannot reach must not be able to tell that
5+
* refusal apart from a workspace that does not exist. Both answer
6+
* `404 "Workspace not found"` — the message the unknown-workspace path in
7+
* `get-public-workspace` and `list-public-workspace-members` already uses, so
8+
* the two responses are byte-identical.
9+
*/
10+
export const v2WorkspaceErrorPolicies = {
11+
concealWorkspaceAuthorization: createV2ResourceConcealmentPolicy({
12+
notFoundMessage: 'Workspace not found',
13+
}),
14+
} as const

0 commit comments

Comments
 (0)