From 826af3bbf14205582ef5940b17a004c5320bdac9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 2 Nov 2026 18:52:00 -0800 Subject: [PATCH 1/2] improvement(workspaces): move workspace updates behind an application operation and let Chat rename workspaces PATCH/PUT /api/workspaces/[id] now run through defineInternalJsonRoute and the session-only workspaces.update use case, keeping the legacy 403/400/500 bodies, audit changes and response shape. Chat gets a workspace/general settings section backed by workspaces.identity.read and workspaces.identity.update, which admit only name and logo; billing payer and the personal-key policy stay session-only. --- apps/sim/app/api/workspaces/[id]/route.ts | 238 +++++------------- apps/sim/lib/api/contracts/workspaces.ts | 20 +- .../lib/mothership/tools/server/settings.ts | 23 ++ apps/sim/lib/settings/sections.ts | 1 + .../update-workspace.integration.ts | 236 +++++++++++++++++ .../application/update-workspace.ts | 139 ++++++++++ apps/sim/lib/workspaces/update.ts | 122 +++++++++ scripts/check-unused-exports.baseline.json | 1 - 8 files changed, 595 insertions(+), 185 deletions(-) create mode 100644 apps/sim/lib/workspaces/application/update-workspace.integration.ts create mode 100644 apps/sim/lib/workspaces/application/update-workspace.ts create mode 100644 apps/sim/lib/workspaces/update.ts diff --git a/apps/sim/app/api/workspaces/[id]/route.ts b/apps/sim/app/api/workspaces/[id]/route.ts index 0c3eeb6a67b..db049b18fe5 100644 --- a/apps/sim/app/api/workspaces/[id]/route.ts +++ b/apps/sim/app/api/workspaces/[id]/route.ts @@ -3,12 +3,31 @@ import { workflow } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq, isNull } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' -import { deleteWorkspaceBodySchema, updateWorkspaceContract } from '@/lib/api/contracts' -import { parseRequest, validationErrorResponse } from '@/lib/api/server' +import { + deleteWorkspaceBodySchema, + updateWorkspaceContract, + updateWorkspacePutContract, +} from '@/lib/api/contracts' +import { validationErrorResponse } from '@/lib/api/server' import { orchestrationFailureResponse } from '@/lib/api/server/orchestration-response' +import { + defineInternalJsonRoute, + type InternalErrorPolicy, + internalErrorResponse, + internalOrchestrationErrorPolicy, + internalRateLimits, + internalSessionAuth, +} from '@/lib/api/server/routes' import { getSession } from '@/lib/auth' -import { changeWorkspaceStoragePayerInTx } from '@/lib/billing/storage/payer-transfer' +import { + InsufficientWorkspacePermissionsError, + NoWorkspaceAccessError, +} from '@/lib/core/application' import { captureServerEvent } from '@/lib/posthog/server' +import { + updateWorkspace, + workspaceUpdateOperations, +} from '@/lib/workspaces/application/update-workspace' import { archiveWorkspace } from '@/lib/workspaces/lifecycle' import { presentWorkspace } from '@/lib/workspaces/response' @@ -18,10 +37,8 @@ import { db } from '@sim/db' import { permissions, workspace } from '@sim/db/schema' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery' -import { - getEffectiveWorkspacePermission, - getUserEntityPermissions, -} from '@/lib/workspaces/permissions/utils' +import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' +import type { WorkspaceUpdatePatch, WorkspaceUpdateResult } from '@/lib/workspaces/update' export const GET = withRouteHandler( async (_request: NextRequest, { params }: { params: Promise<{ id: string }> }) => { @@ -60,174 +77,35 @@ export const GET = withRouteHandler( } ) -export const PATCH = withRouteHandler( - async (request: NextRequest, context: { params: Promise<{ id: string }> }) => { - const session = await getSession() - - if (!session?.user?.id) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) - } - - const parsed = await parseRequest(updateWorkspaceContract, request, context) - if (!parsed.success) return parsed.response - - const workspaceId = parsed.data.params.id - - // Check if user has admin permissions to update workspace - const userPermission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId) - if (userPermission !== 'admin') { - return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 }) - } - - try { - const body = parsed.data.body - const { name, logoUrl, billedAccountUserId, allowPersonalApiKeys } = body - - if ( - name === undefined && - logoUrl === undefined && - billedAccountUserId === undefined && - allowPersonalApiKeys === undefined - ) { - return NextResponse.json({ error: 'No updates provided' }, { status: 400 }) - } - - const existingWorkspace = await db - .select() - .from(workspace) - .where(and(eq(workspace.id, workspaceId), isNull(workspace.archivedAt))) - .then((rows) => rows[0]) - - if (!existingWorkspace) { - return NextResponse.json({ error: 'Workspace not found' }, { status: 404 }) - } - - const updateData: Record = {} - - if (name !== undefined) { - updateData.name = name - } - - if (logoUrl !== undefined) { - updateData.logoUrl = logoUrl - } - - if (allowPersonalApiKeys !== undefined) { - updateData.allowPersonalApiKeys = Boolean(allowPersonalApiKeys) - } - - if (billedAccountUserId !== undefined) { - if ( - existingWorkspace.organizationId && - existingWorkspace.workspaceMode === 'organization' - ) { - return NextResponse.json( - { - error: - 'Organization workspaces use organization billing and cannot change billed account.', - }, - { status: 400 } - ) - } - - if (existingWorkspace.workspaceMode === 'personal') { - return NextResponse.json( - { - error: - 'Personal workspaces are always billed to their owner and cannot change billed account.', - }, - { status: 400 } - ) - } - - const candidateId = billedAccountUserId - - const candidatePermission = await getEffectiveWorkspacePermission( - candidateId, - existingWorkspace - ) - if (candidatePermission !== 'admin') { - return NextResponse.json( - { error: 'Billed account must be a workspace admin' }, - { status: 400 } - ) - } - } - - if (Object.keys(updateData).length === 0 && billedAccountUserId === undefined) { - return NextResponse.json({ error: 'No valid updates provided' }, { status: 400 }) - } - - updateData.updatedAt = new Date() - - if (billedAccountUserId !== undefined) { - await db.transaction(async (tx) => { - await changeWorkspaceStoragePayerInTx(tx, { - workspaceId, - organizationId: existingWorkspace.organizationId, - billedAccountUserId, - expectedCurrentPayer: { - organizationId: existingWorkspace.organizationId, - billedAccountUserId: existingWorkspace.billedAccountUserId, - }, - }) - await tx.update(workspace).set(updateData).where(eq(workspace.id, workspaceId)) - }) - } else { - await db.update(workspace).set(updateData).where(eq(workspace.id, workspaceId)) - } - - const updatedWorkspace = await db - .select() - .from(workspace) - .where(eq(workspace.id, workspaceId)) - .then((rows) => rows[0]) - - recordAudit({ - workspaceId, - actorId: session.user.id, - actorName: session.user.name, - actorEmail: session.user.email, - action: AuditAction.WORKSPACE_UPDATED, - resourceType: AuditResourceType.WORKSPACE, - resourceId: workspaceId, - resourceName: updatedWorkspace?.name ?? existingWorkspace.name, - description: `Updated workspace "${updatedWorkspace?.name ?? existingWorkspace.name}"`, - metadata: { - changes: { - ...(name !== undefined && { name: { from: existingWorkspace.name, to: name } }), - ...(logoUrl !== undefined && { - logoUrl: { from: existingWorkspace.logoUrl, to: logoUrl }, - }), - ...(allowPersonalApiKeys !== undefined && { - allowPersonalApiKeys: { - from: existingWorkspace.allowPersonalApiKeys, - to: allowPersonalApiKeys, - }, - }), - ...(billedAccountUserId !== undefined && { - billedAccountUserId: { - from: existingWorkspace.billedAccountUserId, - to: billedAccountUserId, - }, - }), - }, - }, - request, - }) - - return NextResponse.json({ - workspace: { - ...(updatedWorkspace ? presentWorkspace(updatedWorkspace) : {}), - permissions: userPermission, - }, - }) - } catch (error) { - logger.error('Error updating workspace:', error) - return NextResponse.json({ error: 'Failed to update workspace' }, { status: 500 }) - } - } -) +const updateWorkspaceErrorPolicy: InternalErrorPolicy = { + project: (error) => + error instanceof NoWorkspaceAccessError || + error instanceof InsufficientWorkspacePermissionsError + ? internalErrorResponse(403, { error: 'Insufficient permissions' }) + : internalOrchestrationErrorPolicy.project(error), + unhandled: () => internalErrorResponse(500, { error: 'Failed to update workspace' }), +} + +const updateWorkspaceRoute = { + auth: internalSessionAuth, + operation: workspaceUpdateOperations.update, + rateLimit: internalRateLimits.none({ reason: 'Preserve existing workspace update admission' }), + errorPolicy: updateWorkspaceErrorPolicy, + mapInput: ({ params, body }: { params: { id: string }; body: WorkspaceUpdatePatch }) => ({ + workspaceId: params.id, + patch: body, + }), + useCase: updateWorkspace, + /** Only a workspace admin reaches a successful update, so the caller's role is known. */ + present: ({ updated }: WorkspaceUpdateResult) => ({ + workspace: { ...presentWorkspace(updated), permissions: 'admin' as const }, + }), +} + +export const PATCH = defineInternalJsonRoute({ + contract: updateWorkspaceContract, + ...updateWorkspaceRoute, +}) export const DELETE = withRouteHandler( async (request: NextRequest, { params }: { params: Promise<{ id: string }> }) => { @@ -344,9 +222,7 @@ export const DELETE = withRouteHandler( } ) -export const PUT = withRouteHandler( - async (request: NextRequest, { params }: { params: Promise<{ id: string }> }) => { - // Reuse the PATCH handler implementation for PUT requests - return PATCH(request, { params }) - } -) +export const PUT = defineInternalJsonRoute({ + contract: updateWorkspacePutContract, + ...updateWorkspaceRoute, +}) diff --git a/apps/sim/lib/api/contracts/workspaces.ts b/apps/sim/lib/api/contracts/workspaces.ts index 71bb7ae8efb..f25b92b4e3e 100644 --- a/apps/sim/lib/api/contracts/workspaces.ts +++ b/apps/sim/lib/api/contracts/workspaces.ts @@ -346,6 +346,10 @@ export const getWorkspaceUsageGateContract = defineRouteContract({ }, }) +const updateWorkspaceResponseSchema = z.object({ + workspace: workspaceSchema, +}) + export const updateWorkspaceContract = defineRouteContract({ method: 'PATCH', path: '/api/workspaces/[id]', @@ -353,9 +357,19 @@ export const updateWorkspaceContract = defineRouteContract({ body: updateWorkspaceBodySchema, response: { mode: 'json', - schema: z.object({ - workspace: workspaceSchema, - }), + schema: updateWorkspaceResponseSchema, + }, +}) + +/** `PUT` is a legacy alias of {@link updateWorkspaceContract} with the same partial-update semantics. */ +export const updateWorkspacePutContract = defineRouteContract({ + method: 'PUT', + path: '/api/workspaces/[id]', + params: workspaceParamsSchema, + body: updateWorkspaceBodySchema, + response: { + mode: 'json', + schema: updateWorkspaceResponseSchema, }, }) diff --git a/apps/sim/lib/mothership/tools/server/settings.ts b/apps/sim/lib/mothership/tools/server/settings.ts index ccd509b7189..59c5cf076c8 100644 --- a/apps/sim/lib/mothership/tools/server/settings.ts +++ b/apps/sim/lib/mothership/tools/server/settings.ts @@ -74,6 +74,11 @@ import { import { listCustomBlockSettings } from '@/lib/workflows/custom-blocks/application/settings' import { readInboxSettings, updateInboxSettings } from '@/lib/workspaces/application/inbox-settings' import { readWorkspacePermissions } from '@/lib/workspaces/application/manage-permissions' +import { + readWorkspaceIdentity, + updateWorkspaceIdentity, + workspaceIdentityPatchSchema, +} from '@/lib/workspaces/application/update-workspace' interface SettingAdapter { get(context: SettingsContext): Promise @@ -88,6 +93,24 @@ function organizationInput(context: SettingsContext) { /** Closed adapters call the same application operations as Settings; no route proxy or metadata patch. */ const adapters: Record = { + 'workspace/general': { + schema: workspaceIdentityPatchSchema, + get: (context) => + readWorkspaceIdentity.execute({ + principal: context.principal, + input: { workspaceId: settingsWorkspaceId(context) }, + }), + update: async (context, changes) => + ( + await updateWorkspaceIdentity.execute({ + principal: context.principal, + input: { + workspaceId: settingsWorkspaceId(context), + patch: workspaceIdentityPatchSchema.parse(changes), + }, + }) + ).identity, + }, 'workspace/custom-blocks': { get: (context) => listCustomBlockSettings.execute({ diff --git a/apps/sim/lib/settings/sections.ts b/apps/sim/lib/settings/sections.ts index 3cade5c6717..2fcd1772318 100644 --- a/apps/sim/lib/settings/sections.ts +++ b/apps/sim/lib/settings/sections.ts @@ -100,6 +100,7 @@ export const settingsSections: Record authMock) + +import { createTrustedCopilotPrincipal } from '@/lib/mothership/auth/application-delegation' +import { settingsServerTool } from '@/lib/mothership/tools/server/settings' +import { updateWorkspace } from '@/lib/workspaces/application/update-workspace' +import { PATCH } from '@/app/api/workspaces/[id]/route' + +const adminId = generateId() +const secondAdminId = generateId() +const writerId = generateId() +const outsiderId = generateId() +const workspaceId = generateId() +const archivedWorkspaceId = generateId() +const ORIGINAL_NAME = 'Workspace update fixture' + +function signIn(userId: string) { + authMockFns.mockGetSession.mockResolvedValue({ + user: { id: userId, name: 'Fixture', email: `${userId}@workspace-update.test` }, + session: { id: generateId() }, + }) +} + +function patchWorkspace(id: string, body: Record) { + return PATCH( + new NextRequest(`http://localhost:3000/api/workspaces/${id}`, { + method: 'PATCH', + body: JSON.stringify(body), + headers: { 'content-type': 'application/json' }, + }), + { params: Promise.resolve({ id }) } + ) +} + +function chatSettings(userId: string, input: Record) { + return settingsServerTool.execute( + { scope: 'workspace', ...input } as Parameters[0], + { copilotToolExecution: true, userId, workspaceId, toolCallId: generateId() } + ) +} + +async function readWorkspace() { + const [row] = await db.select().from(workspace).where(eq(workspace.id, workspaceId)) + return row +} + +describe('workspace update operations', () => { + beforeAll(async () => { + const now = new Date() + await db.insert(user).values( + [adminId, secondAdminId, writerId, outsiderId].map((id) => ({ + id, + name: 'Workspace update fixture', + email: `${id}@workspace-update.test`, + emailVerified: true, + createdAt: now, + updatedAt: now, + })) + ) + await insertWorkspaceFixture(db, [ + { + id: workspaceId, + name: ORIGINAL_NAME, + ownerId: adminId, + billedAccountUserId: adminId, + allowPersonalApiKeys: true, + }, + { + id: archivedWorkspaceId, + name: 'Archived fixture', + ownerId: adminId, + billedAccountUserId: adminId, + archivedAt: now, + }, + ]) + await db.insert(permissions).values([ + ...[workspaceId, archivedWorkspaceId].map((entityId) => ({ + id: generateId(), + entityType: 'workspace', + entityId, + userId: adminId, + permissionType: 'admin' as const, + })), + { + id: generateId(), + entityType: 'workspace', + entityId: workspaceId, + userId: secondAdminId, + permissionType: 'admin' as const, + }, + { + id: generateId(), + entityType: 'workspace', + entityId: workspaceId, + userId: writerId, + permissionType: 'write' as const, + }, + ]) + }, 30_000) + + afterAll(async () => { + await db.delete(auditLog).where(eq(auditLog.workspaceId, workspaceId)) + await db + .delete(permissions) + .where(inArray(permissions.entityId, [workspaceId, archivedWorkspaceId])) + await deleteWorkspaceFixture(db, inArray(workspace.id, [workspaceId, archivedWorkspaceId])) + await db.delete(user).where(inArray(user.id, [adminId, secondAdminId, writerId, outsiderId])) + }) + + describe('session route', () => { + it.each([ + ['a write member', writerId, workspaceId], + ['a non-member', outsiderId, workspaceId], + ['an admin of an archived workspace', adminId, archivedWorkspaceId], + ])('refuses %s with the legacy 403 body', async (_label, userId, id) => { + signIn(userId) + const response = await patchWorkspace(id, { name: 'Renamed by someone else' }) + expect(response.status).toBe(403) + expect(await response.json()).toMatchObject({ error: 'Insufficient permissions' }) + expect((await readWorkspace()).name).toBe(ORIGINAL_NAME) + }) + + it('rejects an empty patch from an admin', async () => { + signIn(adminId) + const response = await patchWorkspace(workspaceId, {}) + expect(response.status).toBe(400) + expect(await response.json()).toMatchObject({ error: 'No updates provided' }) + }) + + it('refuses a billed-account change to a non-admin with the legacy message', async () => { + signIn(adminId) + const response = await patchWorkspace(workspaceId, { billedAccountUserId: writerId }) + expect(response.status).toBe(400) + expect(await response.json()).toMatchObject({ + error: 'Billed account must be a workspace admin', + }) + expect((await readWorkspace()).billedAccountUserId).toBe(adminId) + }) + + it('updates as an admin and answers with the workspace and the admin role', async () => { + signIn(adminId) + const response = await patchWorkspace(workspaceId, { allowPersonalApiKeys: false }) + expect(response.status).toBe(200) + const body = await response.json() + expect(body.workspace).toMatchObject({ + id: workspaceId, + allowPersonalApiKeys: false, + permissions: 'admin', + }) + expect((await readWorkspace()).allowPersonalApiKeys).toBe(false) + await db + .update(workspace) + .set({ allowPersonalApiKeys: true }) + .where(eq(workspace.id, workspaceId)) + }) + }) + + describe('Chat settings tool', () => { + it('refuses a write member renaming the workspace', async () => { + await expect( + chatSettings(writerId, { + action: 'update', + section: 'general', + changes: { name: 'Renamed by a writer' }, + }) + ).rejects.toThrow('Insufficient workspace permissions') + expect((await readWorkspace()).name).toBe(ORIGINAL_NAME) + }) + + it.each([ + ['billedAccountUserId', { billedAccountUserId: secondAdminId }], + ['allowPersonalApiKeys', { allowPersonalApiKeys: false }], + ])('cannot change %s', async (_field, changes) => { + await expect( + chatSettings(adminId, { action: 'update', section: 'general', changes }) + ).rejects.toThrow(/Unrecognized key/) + const row = await readWorkspace() + expect(row.billedAccountUserId).toBe(adminId) + expect(row.allowPersonalApiKeys).toBe(true) + }) + + it('cannot reach the full workspace update as a delegated principal', async () => { + const principal = createTrustedCopilotPrincipal( + { userId: adminId, workspaceId, delegationId: generateId() }, + { audience: 'sim:settings', ttlMs: 60_000 } + ) + await expect( + updateWorkspace.execute({ + principal, + input: { workspaceId, patch: { billedAccountUserId: adminId } }, + }) + ).rejects.toThrow('cannot perform operation workspaces.update') + }) + + it('renames the workspace as an admin and audits the change as Chat acting for them', async () => { + const result = await chatSettings(adminId, { + action: 'update', + section: 'general', + changes: { name: ' Renamed by Chat ' }, + }) + expect(result).toMatchObject({ + status: 'updated', + value: { id: workspaceId, name: 'Renamed by Chat' }, + }) + expect((await readWorkspace()).name).toBe('Renamed by Chat') + await vi.waitFor(async () => { + const entries = await db + .select() + .from(auditLog) + .where(and(eq(auditLog.workspaceId, workspaceId), eq(auditLog.actorId, adminId))) + const entry = entries.find( + (row) => toRecord(row.metadata).operation === 'workspaces.identity.update' + ) + expect(entry).toMatchObject({ action: 'workspace.updated', resourceId: workspaceId }) + expect(entry?.metadata).toMatchObject({ + operation: 'workspaces.identity.update', + changes: { name: { from: ORIGINAL_NAME, to: 'Renamed by Chat' } }, + }) + }) + }) + }) +}) diff --git a/apps/sim/lib/workspaces/application/update-workspace.ts b/apps/sim/lib/workspaces/application/update-workspace.ts new file mode 100644 index 00000000000..501efe92b3e --- /dev/null +++ b/apps/sim/lib/workspaces/application/update-workspace.ts @@ -0,0 +1,139 @@ +import { AuditAction, AuditResourceType } from '@sim/audit' +import type { z } from 'zod' +import { updateWorkspaceBodySchema } from '@/lib/api/contracts/workspaces' +import { + defineAuthorizedWorkspaceUseCase, + NoWorkspaceAccessError, + type WorkspaceUseCaseAuditEntry, +} from '@/lib/core/application' +import { defineWorkspaceOperation } from '@/lib/core/application/workspace-operation' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { + loadActiveWorkspaceApplicationContext, + resolveActiveWorkspaceApplicationContext, +} from '@/lib/workspaces/application/context' +import { + applyWorkspaceUpdate, + readWorkspaceIdentityRecord, + type WorkspaceIdentity, + type WorkspaceUpdatePatch, + type WorkspaceUpdateResult, +} from '@/lib/workspaces/update' + +export const workspaceUpdateOperations = { + // permission-group-exempt: workspace administration has no module capability; the admin role is the gate, as it was before this operation existed + update: defineWorkspaceOperation({ + id: 'workspaces.update', + minimumRole: 'admin', + capability: 'none', + workspaceApiKey: 'deny', + principalKinds: ['session'], + }), + // permission-group-exempt: reading the workspace's own name and logo, which every member already sees in the workspace switcher + readIdentity: defineWorkspaceOperation({ + id: 'workspaces.identity.read', + minimumRole: 'read', + capability: 'none', + contentDelivery: 'none', + workspaceApiKey: 'deny', + principalKinds: ['delegated'], + delegatedServices: ['copilot'], + }), + /** + * Name and logo only. Billing payer and the personal-key policy stay on + * {@link workspaceUpdateOperations.update}, which Copilot cannot reach. + */ + // permission-group-exempt: same admin-role gate as the full workspace update, narrowed to presentation fields + updateIdentity: defineWorkspaceOperation({ + id: 'workspaces.identity.update', + minimumRole: 'admin', + capability: 'none', + contentDelivery: 'none', + workspaceApiKey: 'deny', + principalKinds: ['delegated'], + delegatedServices: ['copilot'], + }), +} as const + +export const workspaceIdentityPatchSchema = updateWorkspaceBodySchema + .pick({ name: true, logoUrl: true }) + .strict() +type WorkspaceIdentityPatch = z.output + +function projectUpdateAudit( + patch: WorkspaceUpdatePatch, + { previous, updated }: WorkspaceUpdateResult +): WorkspaceUseCaseAuditEntry { + const { name, logoUrl, allowPersonalApiKeys, billedAccountUserId } = patch + return { + action: AuditAction.WORKSPACE_UPDATED, + resourceType: AuditResourceType.WORKSPACE, + resourceId: updated.id, + resourceName: updated.name, + description: `Updated workspace "${updated.name}"`, + metadata: { + changes: { + ...(name !== undefined && { name: { from: previous.name, to: name } }), + ...(logoUrl !== undefined && { logoUrl: { from: previous.logoUrl, to: logoUrl } }), + ...(allowPersonalApiKeys !== undefined && { + allowPersonalApiKeys: { from: previous.allowPersonalApiKeys, to: allowPersonalApiKeys }, + }), + ...(billedAccountUserId !== undefined && { + billedAccountUserId: { from: previous.billedAccountUserId, to: billedAccountUserId }, + }), + }, + }, + } +} + +/** + * The full workspace settings update: name, logo, personal-key policy and + * billed account. An inactive or unreachable workspace is refused the same way + * as a non-admin, so the caller cannot tell the two apart. + */ +export const updateWorkspace = defineAuthorizedWorkspaceUseCase({ + operation: workspaceUpdateOperations.update, + resolveContext: async ({ + input, + }: { + input: { workspaceId: string; patch: WorkspaceUpdatePatch } + }) => { + const context = await loadActiveWorkspaceApplicationContext(input.workspaceId) + if (!context) throw new NoWorkspaceAccessError() + return context + }, + authorizationOptions: {}, + execute: ({ context, input }) => applyWorkspaceUpdate(context.workspaceId, input.patch), + projectAudit: ({ input, result }) => projectUpdateAudit(input.patch, result), +}) + +const identityDelegation = { + delegation: { audience: 'sim:settings', isWithinScope: () => true }, +} + +function presentIdentity(row: WorkspaceIdentity): WorkspaceIdentity { + return { id: row.id, name: row.name, logoUrl: row.logoUrl } +} + +export const readWorkspaceIdentity = defineAuthorizedWorkspaceUseCase({ + operation: workspaceUpdateOperations.readIdentity, + resolveContext: ({ input }: { input: { workspaceId: string } }) => + resolveActiveWorkspaceApplicationContext(input.workspaceId), + authorizationOptions: identityDelegation, + execute: ({ context }) => readWorkspaceIdentityRecord(context.workspaceId), +}) + +export const updateWorkspaceIdentity = defineAuthorizedWorkspaceUseCase({ + operation: workspaceUpdateOperations.updateIdentity, + resolveContext: ({ input }: { input: { workspaceId: string; patch: WorkspaceIdentityPatch } }) => + resolveActiveWorkspaceApplicationContext(input.workspaceId), + authorizationOptions: identityDelegation, + execute: async ({ context, input }) => { + const parsed = workspaceIdentityPatchSchema.safeParse(input.patch) + if (!parsed.success) throw new OrchestrationError('validation', parsed.error.issues[0].message) + const patch = { name: parsed.data.name, logoUrl: parsed.data.logoUrl } + const result = await applyWorkspaceUpdate(context.workspaceId, patch) + return { ...result, patch, identity: presentIdentity(result.updated) } + }, + projectAudit: ({ result }) => projectUpdateAudit(result.patch, result), +}) diff --git a/apps/sim/lib/workspaces/update.ts b/apps/sim/lib/workspaces/update.ts new file mode 100644 index 00000000000..bff278810b4 --- /dev/null +++ b/apps/sim/lib/workspaces/update.ts @@ -0,0 +1,122 @@ +import { db } from '@sim/db' +import { workspace } from '@sim/db/schema' +import { and, eq, isNull } from 'drizzle-orm' +import { changeWorkspaceStoragePayerInTx } from '@/lib/billing/storage/payer-transfer' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import type { DbOrTx } from '@/lib/db/types' +import { getEffectiveWorkspacePermission } from '@/lib/workspaces/permissions/utils' + +type WorkspaceRow = typeof workspace.$inferSelect + +export interface WorkspaceUpdatePatch { + name?: string + logoUrl?: string | null + allowPersonalApiKeys?: boolean + billedAccountUserId?: string +} + +export interface WorkspaceIdentity { + id: string + name: string + logoUrl: string | null +} + +export interface WorkspaceUpdateResult { + previous: WorkspaceRow + updated: WorkspaceRow +} + +/** The name and logo of an active workspace. */ +export async function readWorkspaceIdentityRecord(workspaceId: string): Promise { + const [row] = await db + .select({ id: workspace.id, name: workspace.name, logoUrl: workspace.logoUrl }) + .from(workspace) + .where(and(eq(workspace.id, workspaceId), isNull(workspace.archivedAt))) + .limit(1) + if (!row) throw new OrchestrationError('not_found', 'Workspace not found') + return row +} + +/** + * Applies a workspace metadata patch to an active workspace. A billed-account + * change moves the workspace's storage to the new payer in the same transaction. + */ +export async function applyWorkspaceUpdate( + workspaceId: string, + patch: WorkspaceUpdatePatch +): Promise { + const { name, logoUrl, allowPersonalApiKeys, billedAccountUserId } = patch + if ( + name === undefined && + logoUrl === undefined && + billedAccountUserId === undefined && + allowPersonalApiKeys === undefined + ) { + throw new OrchestrationError('validation', 'No updates provided') + } + + const [previous] = await db + .select() + .from(workspace) + .where(and(eq(workspace.id, workspaceId), isNull(workspace.archivedAt))) + .limit(1) + if (!previous) throw new OrchestrationError('not_found', 'Workspace not found') + + const updateData: Partial = {} + if (name !== undefined) updateData.name = name + if (logoUrl !== undefined) updateData.logoUrl = logoUrl + if (allowPersonalApiKeys !== undefined) updateData.allowPersonalApiKeys = allowPersonalApiKeys + + if (billedAccountUserId !== undefined) { + if (previous.organizationId && previous.workspaceMode === 'organization') { + throw new OrchestrationError( + 'validation', + 'Organization workspaces use organization billing and cannot change billed account.' + ) + } + if (previous.workspaceMode === 'personal') { + throw new OrchestrationError( + 'validation', + 'Personal workspaces are always billed to their owner and cannot change billed account.' + ) + } + const candidatePermission = await getEffectiveWorkspacePermission(billedAccountUserId, previous) + if (candidatePermission !== 'admin') { + throw new OrchestrationError('validation', 'Billed account must be a workspace admin') + } + } + + updateData.updatedAt = new Date() + + const updated = + billedAccountUserId === undefined + ? await updateWorkspaceRow(db, workspaceId, updateData) + : await db.transaction(async (tx) => { + await changeWorkspaceStoragePayerInTx(tx, { + workspaceId, + organizationId: previous.organizationId, + billedAccountUserId, + expectedCurrentPayer: { + organizationId: previous.organizationId, + billedAccountUserId: previous.billedAccountUserId, + }, + }) + return updateWorkspaceRow(tx, workspaceId, updateData) + }) + + return { previous, updated } +} + +async function updateWorkspaceRow( + executor: DbOrTx, + workspaceId: string, + updateData: Partial +): Promise { + const [row] = await executor + .update(workspace) + .set(updateData) + .where(eq(workspace.id, workspaceId)) + .returning() + if (!row) throw new OrchestrationError('not_found', 'Workspace not found') + return row +} diff --git a/scripts/check-unused-exports.baseline.json b/scripts/check-unused-exports.baseline.json index e4448b682b2..3adf26239c0 100644 --- a/scripts/check-unused-exports.baseline.json +++ b/scripts/check-unused-exports.baseline.json @@ -3964,7 +3964,6 @@ "apps/sim/lib/api/contracts/workspaces.ts#createWorkspaceBodySchema", "apps/sim/lib/api/contracts/workspaces.ts#deploymentFeaturesSchema", "apps/sim/lib/api/contracts/workspaces.ts#deploymentShapeSchema", - "apps/sim/lib/api/contracts/workspaces.ts#updateWorkspaceBodySchema", "apps/sim/lib/api/contracts/workspaces.ts#workspaceCreationPolicySchema", "apps/sim/lib/api/contracts/workspaces.ts#workspaceCreditAvailabilitySchema", "apps/sim/lib/api/contracts/workspaces.ts#workspaceHostContextSchema", From f3c7ca6747cca336480a205927716e48750519f8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 2 Nov 2026 19:38:09 -0800 Subject: [PATCH 2/2] fix(workspaces): recheck active state atomically on update The workspace UPDATE now carries the not-archived predicate, so an archival that commits after the use case's reads leaves nothing to write and answers the same 404 as a missing workspace, on both the session route and Chat's identity update. The integration suite holds the row lock, archives the row while the update waits, and asserts the 404 and the unchanged name; the archived-fixture refusal now checks that fixture's own row. --- .../update-workspace.integration.ts | 130 +++++++++++++++--- apps/sim/lib/workspaces/update.ts | 8 +- 2 files changed, 115 insertions(+), 23 deletions(-) diff --git a/apps/sim/lib/workspaces/application/update-workspace.integration.ts b/apps/sim/lib/workspaces/application/update-workspace.integration.ts index 2b022a3ab82..5f03bea5c63 100644 --- a/apps/sim/lib/workspaces/application/update-workspace.integration.ts +++ b/apps/sim/lib/workspaces/application/update-workspace.integration.ts @@ -6,10 +6,11 @@ import { db } from '@sim/db' import { auditLog, permissions, user, workspace } from '@sim/db/schema' import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures' +import { createDeferred } from '@sim/testing/helpers/deferred' import { authMock, authMockFns } from '@sim/testing/mocks/auth.mock' import { generateId } from '@sim/utils/id' import { toRecord } from '@sim/utils/object' -import { and, eq, inArray } from 'drizzle-orm' +import { and, eq, inArray, sql } from 'drizzle-orm' import { NextRequest } from 'next/server' import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' @@ -26,7 +27,16 @@ const writerId = generateId() const outsiderId = generateId() const workspaceId = generateId() const archivedWorkspaceId = generateId() +const routeRaceWorkspaceId = generateId() +const chatRaceWorkspaceId = generateId() +const fixtureWorkspaceIds = [ + workspaceId, + archivedWorkspaceId, + routeRaceWorkspaceId, + chatRaceWorkspaceId, +] const ORIGINAL_NAME = 'Workspace update fixture' +const ARCHIVED_NAME = 'Archived fixture' function signIn(userId: string) { authMockFns.mockGetSession.mockResolvedValue({ @@ -46,18 +56,59 @@ function patchWorkspace(id: string, body: Record) { ) } -function chatSettings(userId: string, input: Record) { +function chatSettings(userId: string, input: Record, targetId = workspaceId) { return settingsServerTool.execute( { scope: 'workspace', ...input } as Parameters[0], - { copilotToolExecution: true, userId, workspaceId, toolCallId: generateId() } + { copilotToolExecution: true, userId, workspaceId: targetId, toolCallId: generateId() } ) } -async function readWorkspace() { - const [row] = await db.select().from(workspace).where(eq(workspace.id, workspaceId)) +async function readWorkspace(id: string) { + const [row] = await db.select().from(workspace).where(eq(workspace.id, id)) return row } +/** + * Holds the workspace row lock until `update` is queued behind it, then archives + * the row and commits, so the update's write runs after its own active-state reads + * passed but against an archived row. + */ +async function archiveWhileUpdateWaits(id: string, update: () => Promise) { + const held = createDeferred() + const release = createDeferred() + const archival = db.transaction(async (tx) => { + await tx.select({ id: workspace.id }).from(workspace).where(eq(workspace.id, id)).for('update') + const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`) + held.resolve(connection.pid) + await release.promise + await tx.update(workspace).set({ archivedAt: new Date() }).where(eq(workspace.id, id)) + }) + void archival.catch((error: unknown) => held.reject(error)) + try { + const blockerPid = await held.promise + const pending = update() + void pending.catch(() => undefined) + await vi.waitFor( + async () => { + const [waiting] = await db.execute<{ pid: number }>(sql` + SELECT pid FROM pg_stat_activity + WHERE datname = current_database() + AND ${blockerPid}::int = ANY(pg_blocking_pids(pid)) + `) + expect(waiting).toBeDefined() + }, + { timeout: 10_000, interval: 25 } + ) + release.resolve() + await archival + const [settled] = await Promise.allSettled([pending]) + return settled + } finally { + release.resolve() + await Promise.allSettled([archival]) + } +} + describe('workspace update operations', () => { beforeAll(async () => { const now = new Date() @@ -81,14 +132,20 @@ describe('workspace update operations', () => { }, { id: archivedWorkspaceId, - name: 'Archived fixture', + name: ARCHIVED_NAME, ownerId: adminId, billedAccountUserId: adminId, archivedAt: now, }, + ...[routeRaceWorkspaceId, chatRaceWorkspaceId].map((id) => ({ + id, + name: ORIGINAL_NAME, + ownerId: adminId, + billedAccountUserId: adminId, + })), ]) await db.insert(permissions).values([ - ...[workspaceId, archivedWorkspaceId].map((entityId) => ({ + ...fixtureWorkspaceIds.map((entityId) => ({ id: generateId(), entityType: 'workspace', entityId, @@ -113,25 +170,37 @@ describe('workspace update operations', () => { }, 30_000) afterAll(async () => { - await db.delete(auditLog).where(eq(auditLog.workspaceId, workspaceId)) - await db - .delete(permissions) - .where(inArray(permissions.entityId, [workspaceId, archivedWorkspaceId])) - await deleteWorkspaceFixture(db, inArray(workspace.id, [workspaceId, archivedWorkspaceId])) + await db.delete(auditLog).where(inArray(auditLog.workspaceId, fixtureWorkspaceIds)) + await db.delete(permissions).where(inArray(permissions.entityId, fixtureWorkspaceIds)) + await deleteWorkspaceFixture(db, inArray(workspace.id, fixtureWorkspaceIds)) await db.delete(user).where(inArray(user.id, [adminId, secondAdminId, writerId, outsiderId])) }) describe('session route', () => { it.each([ - ['a write member', writerId, workspaceId], - ['a non-member', outsiderId, workspaceId], - ['an admin of an archived workspace', adminId, archivedWorkspaceId], - ])('refuses %s with the legacy 403 body', async (_label, userId, id) => { + ['a write member', writerId, workspaceId, ORIGINAL_NAME], + ['a non-member', outsiderId, workspaceId, ORIGINAL_NAME], + ['an admin of an archived workspace', adminId, archivedWorkspaceId, ARCHIVED_NAME], + ])('refuses %s with the legacy 403 body', async (_label, userId, id, unchangedName) => { signIn(userId) const response = await patchWorkspace(id, { name: 'Renamed by someone else' }) expect(response.status).toBe(403) expect(await response.json()).toMatchObject({ error: 'Insufficient permissions' }) - expect((await readWorkspace()).name).toBe(ORIGINAL_NAME) + expect((await readWorkspace(id)).name).toBe(unchangedName) + }) + + it('answers 404 and leaves the row alone when the workspace is archived mid-update', async () => { + signIn(adminId) + const settled = await archiveWhileUpdateWaits(routeRaceWorkspaceId, () => + patchWorkspace(routeRaceWorkspaceId, { name: 'Renamed after archival' }) + ) + expect(settled.status).toBe('fulfilled') + const response = settled.status === 'fulfilled' ? settled.value : null + expect(response?.status).toBe(404) + expect(await response?.json()).toMatchObject({ error: 'Workspace not found' }) + const row = await readWorkspace(routeRaceWorkspaceId) + expect(row.archivedAt).not.toBeNull() + expect(row.name).toBe(ORIGINAL_NAME) }) it('rejects an empty patch from an admin', async () => { @@ -148,7 +217,7 @@ describe('workspace update operations', () => { expect(await response.json()).toMatchObject({ error: 'Billed account must be a workspace admin', }) - expect((await readWorkspace()).billedAccountUserId).toBe(adminId) + expect((await readWorkspace(workspaceId)).billedAccountUserId).toBe(adminId) }) it('updates as an admin and answers with the workspace and the admin role', async () => { @@ -161,7 +230,7 @@ describe('workspace update operations', () => { allowPersonalApiKeys: false, permissions: 'admin', }) - expect((await readWorkspace()).allowPersonalApiKeys).toBe(false) + expect((await readWorkspace(workspaceId)).allowPersonalApiKeys).toBe(false) await db .update(workspace) .set({ allowPersonalApiKeys: true }) @@ -178,7 +247,7 @@ describe('workspace update operations', () => { changes: { name: 'Renamed by a writer' }, }) ).rejects.toThrow('Insufficient workspace permissions') - expect((await readWorkspace()).name).toBe(ORIGINAL_NAME) + expect((await readWorkspace(workspaceId)).name).toBe(ORIGINAL_NAME) }) it.each([ @@ -188,11 +257,28 @@ describe('workspace update operations', () => { await expect( chatSettings(adminId, { action: 'update', section: 'general', changes }) ).rejects.toThrow(/Unrecognized key/) - const row = await readWorkspace() + const row = await readWorkspace(workspaceId) expect(row.billedAccountUserId).toBe(adminId) expect(row.allowPersonalApiKeys).toBe(true) }) + it('refuses a rename when the workspace is archived mid-update', async () => { + const settled = await archiveWhileUpdateWaits(chatRaceWorkspaceId, () => + chatSettings( + adminId, + { action: 'update', section: 'general', changes: { name: 'Renamed after archival' } }, + chatRaceWorkspaceId + ) + ) + expect(settled.status).toBe('rejected') + expect(settled.status === 'rejected' ? settled.reason : null).toMatchObject({ + message: 'Workspace not found', + }) + const row = await readWorkspace(chatRaceWorkspaceId) + expect(row.archivedAt).not.toBeNull() + expect(row.name).toBe(ORIGINAL_NAME) + }) + it('cannot reach the full workspace update as a delegated principal', async () => { const principal = createTrustedCopilotPrincipal( { userId: adminId, workspaceId, delegationId: generateId() }, @@ -216,7 +302,7 @@ describe('workspace update operations', () => { status: 'updated', value: { id: workspaceId, name: 'Renamed by Chat' }, }) - expect((await readWorkspace()).name).toBe('Renamed by Chat') + expect((await readWorkspace(workspaceId)).name).toBe('Renamed by Chat') await vi.waitFor(async () => { const entries = await db .select() diff --git a/apps/sim/lib/workspaces/update.ts b/apps/sim/lib/workspaces/update.ts index bff278810b4..b45b4f80ac2 100644 --- a/apps/sim/lib/workspaces/update.ts +++ b/apps/sim/lib/workspaces/update.ts @@ -107,6 +107,12 @@ export async function applyWorkspaceUpdate( return { previous, updated } } +/** + * Writes the patch only while the workspace is still active: an archival that + * commits after the earlier reads leaves no row to update, which answers the + * same not-found as a workspace that was already gone (and rolls back a + * billed-account transfer). + */ async function updateWorkspaceRow( executor: DbOrTx, workspaceId: string, @@ -115,7 +121,7 @@ async function updateWorkspaceRow( const [row] = await executor .update(workspace) .set(updateData) - .where(eq(workspace.id, workspaceId)) + .where(and(eq(workspace.id, workspaceId), isNull(workspace.archivedAt))) .returning() if (!row) throw new OrchestrationError('not_found', 'Workspace not found') return row