diff --git a/apps/docs/components/icons.tsx b/apps/docs/components/icons.tsx index 41732af3b29..bfd2ade63a7 100644 --- a/apps/docs/components/icons.tsx +++ b/apps/docs/components/icons.tsx @@ -2492,7 +2492,7 @@ export function LinkupIcon(props: SVGProps) { ) @@ -4967,7 +4967,7 @@ export function InfisicalIcon(props: SVGProps) { ) diff --git a/apps/docs/content/docs/en/tools/servicenow.mdx b/apps/docs/content/docs/en/tools/servicenow.mdx index 398e3fd6db2..7337cbf636b 100644 --- a/apps/docs/content/docs/en/tools/servicenow.mdx +++ b/apps/docs/content/docs/en/tools/servicenow.mdx @@ -215,7 +215,6 @@ Attach a file to a ServiceNow record | `recordSysId` | string | Yes | sys_id of the record to attach the file to | | `fileName` | string | Yes | Name to give the uploaded file \(e.g., logs.txt\) | | `file` | file | No | File to upload \(UserFile object\) | -| `fileContent` | string | No | Base64-encoded file content \(legacy\) | #### Output diff --git a/apps/sim/app/api/chat/[identifier]/route.ts b/apps/sim/app/api/chat/[identifier]/route.ts index 7a4a12d5754..330ece68852 100644 --- a/apps/sim/app/api/chat/[identifier]/route.ts +++ b/apps/sim/app/api/chat/[identifier]/route.ts @@ -6,6 +6,9 @@ import { and, eq, isNull } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { deployedChatPostContract } from '@/lib/api/contracts/chats' import { parseRequest } from '@/lib/api/server' +import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation' +import { admissionRejectedResponse, tryAdmit } from '@/lib/core/admission/gate' +import { env } from '@/lib/core/config/env' import { validateAuthToken } from '@/lib/core/security/deployment' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -40,13 +43,21 @@ function toChatConfigResponse(deployment: ChatConfigSource) { export const dynamic = 'force-dynamic' export const runtime = 'nodejs' +const CHAT_MAX_REQUEST_BYTES = Number.parseInt(env.CHAT_MAX_REQUEST_BYTES, 10) || 220 * 1024 * 1024 + export const POST = withRouteHandler( async (request: NextRequest, context: { params: Promise<{ identifier: string }> }) => { const { identifier } = await context.params const requestId = generateRequestId() + const ticket = tryAdmit() + if (!ticket) { + return admissionRejectedResponse() + } + try { const parsed = await parseRequest(deployedChatPostContract, request, context, { + maxBodyBytes: CHAT_MAX_REQUEST_BYTES, validationErrorResponse: (err) => { const message = err.issues.map((e) => `${e.path.join('.')}: ${e.message}`).join(', ') return createErrorResponse(`Invalid request body: ${message}`, 400, 'VALIDATION_ERROR') @@ -125,7 +136,14 @@ export const POST = withRouteHandler( const authResult = await validateChatAuth(requestId, deployment, request, parsedBody) if (!authResult.authorized) { - return createErrorResponse(authResult.error || 'Authentication required', 401) + const response = createErrorResponse( + authResult.error || 'Authentication required', + authResult.status || 401 + ) + if (authResult.status === 429 && authResult.retryAfterMs !== undefined) { + response.headers.set('Retry-After', String(Math.ceil(authResult.retryAfterMs / 1000))) + } + return response } const { input, password, email, conversationId, files } = parsedBody @@ -177,6 +195,9 @@ export const POST = withRouteHandler( const workspaceId = workflowRecord?.workspaceId if (!workspaceId) { logger.error(`[${requestId}] Workflow ${deployment.workflowId} has no workspaceId`) + // preprocessExecution reserved a billing concurrency slot; release it on + // this early exit since no LoggingSession will finalize to free it. + await releaseExecutionSlot(executionId) return createErrorResponse('Workflow has no associated workspace', 500) } @@ -283,11 +304,16 @@ export const POST = withRouteHandler( return streamResponse } catch (error: any) { logger.error(`[${requestId}] Error processing chat request:`, error) + // Setup failed before the workflow stream took over slot release; + // free the reserved billing slot (idempotent if already released). + await releaseExecutionSlot(executionId) return createErrorResponse(error.message || 'Failed to process request', 500) } } catch (error: any) { logger.error(`[${requestId}] Error processing chat request:`, error) return createErrorResponse(error.message || 'Failed to process request', 500) + } finally { + ticket.release() } } ) diff --git a/apps/sim/app/api/chat/utils.test.ts b/apps/sim/app/api/chat/utils.test.ts index 86e46340a92..337310d6303 100644 --- a/apps/sim/app/api/chat/utils.test.ts +++ b/apps/sim/app/api/chat/utils.test.ts @@ -19,6 +19,7 @@ const { mockSetDeploymentAuthCookie, mockIsEmailAllowed, mockGetSession, + mockCheckRateLimitDirect, } = vi.hoisted(() => ({ mockMergeSubblockStateWithValues: vi.fn().mockReturnValue({}), mockMergeSubBlockValues: vi.fn().mockReturnValue({}), @@ -26,6 +27,13 @@ const { mockSetDeploymentAuthCookie: vi.fn(), mockIsEmailAllowed: vi.fn(), mockGetSession: vi.fn(), + mockCheckRateLimitDirect: vi.fn().mockResolvedValue({ allowed: true }), +})) + +vi.mock('@/lib/core/rate-limiter', () => ({ + RateLimiter: class { + checkRateLimitDirect = mockCheckRateLimitDirect + }, })) vi.mock('@/lib/auth', () => ({ @@ -149,6 +157,7 @@ describe('Chat API Utils', () => { describe('Chat auth validation', () => { beforeEach(() => { mockDecryptSecret.mockResolvedValue({ decrypted: 'correct-password' }) + mockCheckRateLimitDirect.mockResolvedValue({ allowed: true }) }) it('should allow access to public chats', async () => { @@ -235,6 +244,32 @@ describe('Chat API Utils', () => { expect(result.error).toBe('Invalid password') }) + it('should return 429 when the password attempt rate limit is exceeded', async () => { + mockCheckRateLimitDirect.mockResolvedValueOnce({ allowed: false, retryAfterMs: 60_000 }) + + const deployment = { + id: 'chat-id', + authType: 'password', + password: 'encrypted-password', + } + + const mockRequest = { + method: 'POST', + cookies: { + get: vi.fn().mockReturnValue(null), + }, + } as any + + const result = await validateChatAuth('request-id', deployment, mockRequest, { + password: 'any-guess', + }) + + expect(result.authorized).toBe(false) + expect(result.status).toBe(429) + expect(result.retryAfterMs).toBe(60_000) + expect(decryptSecret).not.toHaveBeenCalled() + }) + it('should request email auth for email-protected chats', async () => { const deployment = { id: 'chat-id', diff --git a/apps/sim/app/api/chat/utils.ts b/apps/sim/app/api/chat/utils.ts index 5a3d0750e8d..70e4a657ac1 100644 --- a/apps/sim/app/api/chat/utils.ts +++ b/apps/sim/app/api/chat/utils.ts @@ -1,18 +1,34 @@ import { db } from '@sim/db' import { chat, workflow } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { safeCompare } from '@sim/security/compare' import { authorizeWorkflowByWorkspacePermission } from '@sim/workflow-authz' import { and, eq, isNull } from 'drizzle-orm' import type { NextRequest, NextResponse } from 'next/server' +import type { TokenBucketConfig } from '@/lib/core/rate-limiter' +import { RateLimiter } from '@/lib/core/rate-limiter' import { isEmailAllowed, setDeploymentAuthCookie, validateAuthToken, } from '@/lib/core/security/deployment' import { decryptSecret } from '@/lib/core/security/encryption' +import { getClientIp } from '@/lib/core/utils/request' const logger = createLogger('ChatAuthUtils') +const rateLimiter = new RateLimiter() + +/** + * Throttles unauthenticated password guesses per client IP against a single + * deployment, mirroring the OTP/SSO IP limits. + */ +const PASSWORD_IP_RATE_LIMIT: TokenBucketConfig = { + maxTokens: 10, + refillRate: 10, + refillIntervalMs: 15 * 60_000, +} + export function setChatAuthCookie( response: NextResponse, chatId: string, @@ -88,7 +104,7 @@ export async function validateChatAuth( deployment: any, request: NextRequest, parsedBody?: any -): Promise<{ authorized: boolean; error?: string }> { +): Promise<{ authorized: boolean; error?: string; status?: number; retryAfterMs?: number }> { const authType = deployment.authType || 'public' if (authType === 'public') { @@ -129,8 +145,25 @@ export async function validateChatAuth( return { authorized: false, error: 'Authentication configuration error' } } + const ip = getClientIp(request) + const ipRateLimit = await rateLimiter.checkRateLimitDirect( + `chat-password:ip:${deployment.id}:${ip}`, + PASSWORD_IP_RATE_LIMIT + ) + if (!ipRateLimit.allowed) { + logger.warn( + `[${requestId}] Password attempt IP rate limit exceeded for chat ${deployment.id} from ${ip}` + ) + return { + authorized: false, + error: 'Too many attempts. Please try again later.', + status: 429, + retryAfterMs: ipRateLimit.retryAfterMs ?? PASSWORD_IP_RATE_LIMIT.refillIntervalMs, + } + } + const { decrypted } = await decryptSecret(deployment.password) - if (password !== decrypted) { + if (!safeCompare(password, decrypted)) { return { authorized: false, error: 'Invalid password' } } diff --git a/apps/sim/app/api/files/authorization.test.ts b/apps/sim/app/api/files/authorization.test.ts index 463a1bdf0c0..4409108eacd 100644 --- a/apps/sim/app/api/files/authorization.test.ts +++ b/apps/sim/app/api/files/authorization.test.ts @@ -12,15 +12,18 @@ import { dbChainMock, dbChainMockFns } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockGetFileMetadataByKey, mockGetUserEntityPermissions } = vi.hoisted(() => ({ - mockGetFileMetadataByKey: vi.fn(), - mockGetUserEntityPermissions: vi.fn(), -})) +const { mockGetFileMetadataByKey, mockGetUserEntityPermissions, mockGetFileMetadata } = vi.hoisted( + () => ({ + mockGetFileMetadataByKey: vi.fn(), + mockGetUserEntityPermissions: vi.fn(), + mockGetFileMetadata: vi.fn(), + }) +) vi.mock('@sim/db', () => dbChainMock) vi.mock('@/lib/uploads', () => ({ - getFileMetadata: vi.fn(), + getFileMetadata: mockGetFileMetadata, })) vi.mock('@/lib/uploads/config', () => ({ @@ -151,3 +154,59 @@ describe('verifyKBFileWriteAccess (binding-only delete authorization)', () => { await expect(verifyKBFileWriteAccess(CLOUD_KEY, USER_ID)).resolves.toBe(false) }) }) + +describe('public-context access (profile-pictures / og-images / workspace-logos)', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + function read(cloudKey: string, context: 'profile-pictures' | 'og-images' | 'workspace-logos') { + return verifyFileAccess(cloudKey, USER_ID, undefined, context, false) + } + + function write(cloudKey: string, context: 'profile-pictures' | 'og-images' | 'workspace-logos') { + return verifyFileAccess(cloudKey, USER_ID, undefined, context, false, { requireWrite: true }) + } + + it('grants public reads without any ownership check', async () => { + await expect(read('og-images/banner.png', 'og-images')).resolves.toBe(true) + await expect(read('profile-pictures/123-avatar.png', 'profile-pictures')).resolves.toBe(true) + await expect(read('workspace-logos/123-logo.png', 'workspace-logos')).resolves.toBe(true) + expect(mockGetUserEntityPermissions).not.toHaveBeenCalled() + expect(mockGetFileMetadata).not.toHaveBeenCalled() + }) + + it('denies a cross-tenant delete that names a workspace key under a public context', async () => { + await expect(write('workspace/victim-ws/123-report.pdf', 'og-images')).resolves.toBe(false) + expect(mockGetUserEntityPermissions).not.toHaveBeenCalled() + }) + + it('grants a profile-picture delete only to the owning user', async () => { + mockGetFileMetadata.mockResolvedValue({ userId: USER_ID }) + await expect(write('profile-pictures/123-avatar.png', 'profile-pictures')).resolves.toBe(true) + }) + + it('denies a profile-picture delete for a non-owner', async () => { + mockGetFileMetadata.mockResolvedValue({ userId: 'other-user' }) + await expect(write('profile-pictures/123-avatar.png', 'profile-pictures')).resolves.toBe(false) + }) + + it('grants a workspace-logo delete to write/admin on the owning workspace', async () => { + mockGetFileMetadataByKey.mockResolvedValue({ workspaceId: 'ws-1' }) + mockGetUserEntityPermissions.mockResolvedValue('admin') + await expect(write('workspace-logos/123-logo.png', 'workspace-logos')).resolves.toBe(true) + expect(mockGetUserEntityPermissions).toHaveBeenCalledWith(USER_ID, 'workspace', 'ws-1') + }) + + it('denies a workspace-logo delete for a non-member of the owning workspace', async () => { + mockGetFileMetadataByKey.mockResolvedValue({ workspaceId: 'victim-ws' }) + mockGetUserEntityPermissions.mockResolvedValue(null) + await expect(write('workspace-logos/123-logo.png', 'workspace-logos')).resolves.toBe(false) + }) + + it('denies a workspace-logo delete when no ownership binding exists', async () => { + mockGetFileMetadataByKey.mockResolvedValue(null) + await expect(write('workspace-logos/123-logo.png', 'workspace-logos')).resolves.toBe(false) + expect(mockGetUserEntityPermissions).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/files/authorization.ts b/apps/sim/app/api/files/authorization.ts index b2cc73c429f..7b96031baf8 100644 --- a/apps/sim/app/api/files/authorization.ts +++ b/apps/sim/app/api/files/authorization.ts @@ -144,12 +144,15 @@ export async function verifyFileAccess( // Infer context from key if not explicitly provided const inferredContext = context || inferContextFromKey(cloudKey) - // 0. Public contexts: profile pictures, OG images, and workspace logos are publicly accessible + // 0. Public contexts: profile pictures, OG images, and workspace logos are world-readable, so reads short-circuit; writes require proof of ownership if ( inferredContext === 'profile-pictures' || inferredContext === 'og-images' || inferredContext === 'workspace-logos' ) { + if (requireWrite) { + return await verifyPublicAssetWriteAccess(cloudKey, userId, inferredContext, customConfig) + } logger.info('Public file access allowed', { cloudKey, context: inferredContext }) return true } @@ -267,6 +270,80 @@ async function verifyWorkspaceFileAccess( } } +/** + * Authorize a destructive operation (delete) on a "public" asset context: + * `profile-pictures`, `workspace-logos`, or `og-images`. These contexts are + * world-readable, so {@link verifyFileAccess} short-circuits reads — but a write + * must prove ownership of the user/workspace the object belongs to and never + * short-circuit to `true`. + * + * - `workspace-logos` carry a trusted `workspace_files` binding written at upload + * time; require write/admin on the owning workspace. + * - `profile-pictures` are owned by a single user, recorded in the storage + * object's `userId` metadata at upload time; require an exact owner match. + * - `og-images` are platform/blog assets with no per-user or per-workspace owner + * and no user-facing delete path; always deny. + */ +async function verifyPublicAssetWriteAccess( + cloudKey: string, + userId: string, + context: 'profile-pictures' | 'og-images' | 'workspace-logos', + customConfig?: StorageConfig +): Promise { + try { + if (context === 'workspace-logos') { + const binding = await getFileMetadataByKey(cloudKey, 'workspace-logos') + if (!binding?.workspaceId) { + logger.warn('workspace-logos delete denied: no ownership binding', { userId, cloudKey }) + return false + } + const permission = await getUserEntityPermissions(userId, 'workspace', binding.workspaceId) + if (!workspacePermissionSatisfies(permission, true)) { + logger.warn('workspace-logos delete denied: write/admin required on owner workspace', { + userId, + workspaceId: binding.workspaceId, + cloudKey, + }) + return false + } + return true + } + + if (context === 'profile-pictures') { + const config: StorageConfig = customConfig || {} + const metadata = await getFileMetadata(cloudKey, config) + if (metadata.userId && metadata.userId === userId) { + return true + } + // Fail closed when the owner cannot be established. Distinguish a missing + // owner record (no `userId` metadata — e.g. an object predating owner + // tagging) from a genuine ownership mismatch so the denial is diagnosable. + if (!metadata.userId) { + logger.warn( + 'profile-pictures delete denied: file has no owner metadata to verify against', + { + userId, + cloudKey, + } + ) + } else { + logger.warn('profile-pictures delete denied: caller does not own the file', { + userId, + fileUserId: metadata.userId, + cloudKey, + }) + } + return false + } + + logger.warn('og-images delete denied: no user-facing delete path', { userId, cloudKey }) + return false + } catch (error) { + logger.error('Error verifying public asset write access', { cloudKey, userId, error }) + return false + } +} + /** * Verify access to execution files * Modern format: execution/workspace_id/workflow_id/execution_id/filename diff --git a/apps/sim/app/api/files/delete/route.test.ts b/apps/sim/app/api/files/delete/route.test.ts index eed07748581..d843b573651 100644 --- a/apps/sim/app/api/files/delete/route.test.ts +++ b/apps/sim/app/api/files/delete/route.test.ts @@ -198,4 +198,19 @@ describe('File Delete API Route', () => { expect(data).toHaveProperty('error', 'InvalidRequestError') expect(data).toHaveProperty('message', 'No file path provided') }) + + it('rejects a client context that disagrees with the key prefix', async () => { + const req = createMockRequest('POST', { + filePath: '/api/files/serve/s3/workspace/victim-ws/1234-report.pdf', + context: 'og-images', + }) + + const response = await POST(req) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data).toHaveProperty('error', 'InvalidRequestError') + expect(mocks.mockVerifyFileAccess).not.toHaveBeenCalled() + expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/app/api/files/delete/route.ts b/apps/sim/app/api/files/delete/route.ts index c483faa3c6b..34903aa22e1 100644 --- a/apps/sim/app/api/files/delete/route.ts +++ b/apps/sim/app/api/files/delete/route.ts @@ -62,23 +62,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => { try { const key = extractStorageKeyFromPath(filePath) - const storageContext: StorageContext = context || inferContextFromKey(key) + // Derive context from the trusted key prefix, never the client-supplied value; a supplied context must agree with the key. + const storageContext: StorageContext = inferContextFromKey(key) + if (context && context !== storageContext) { + logger.warn('File delete context mismatch', { key, context, inferred: storageContext }) + throw new InvalidRequestError(`Provided context "${context}" does not match the file key`) + } - // Deletes require write/admin on the owning workspace (owner-scoped files - // like copilot/regular uploads still authorize by ownership). KB deletes - // are binding-only and never use the transitional read fallback that file - // serving allows. + // Deletes require write/admin on the owning workspace; KB deletes are binding-only with no read fallback. const hasAccess = storageContext === 'knowledge-base' ? await verifyKBFileWriteAccess(key, userId) - : await verifyFileAccess( - key, - userId, - undefined, // customConfig - storageContext, // context - !hasCloudStorage(), // isLocal - { requireWrite: true } - ) + : await verifyFileAccess(key, userId, undefined, storageContext, !hasCloudStorage(), { + requireWrite: true, + }) if (!hasAccess) { logger.warn('Unauthorized file delete attempt', { userId, key, context: storageContext }) diff --git a/apps/sim/app/api/folders/reorder/route.test.ts b/apps/sim/app/api/folders/reorder/route.test.ts new file mode 100644 index 00000000000..61b56e6fa50 --- /dev/null +++ b/apps/sim/app/api/folders/reorder/route.test.ts @@ -0,0 +1,124 @@ +/** + * Tests for the folder reorder API route. + * + * @vitest-environment node + */ +import { authMockFns, createMockRequest, permissionsMock, permissionsMockFns } from '@sim/testing' +import { drizzleOrmMock } from '@sim/testing/mocks' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockLogger } = vi.hoisted(() => ({ + mockLogger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + trace: vi.fn(), + fatal: vi.fn(), + child: vi.fn(), + }, +})) + +const mockGetUserEntityPermissions = permissionsMockFns.mockGetUserEntityPermissions + +vi.mock('drizzle-orm', () => drizzleOrmMock) +vi.mock('@sim/logger', () => ({ + createLogger: vi.fn().mockReturnValue(mockLogger), + runWithRequestContext: (_ctx: unknown, fn: () => T): T => fn(), + getRequestContext: () => undefined, +})) +vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock) + +import { db } from '@sim/db' +import { PUT } from '@/app/api/folders/reorder/route' + +const mockDb = db as any + +describe('PUT /api/folders/reorder', () => { + const mockFrom = vi.fn() + const mockWhere = vi.fn() + const mockTxUpdate = vi.fn() + + beforeEach(() => { + vi.clearAllMocks() + + authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-123' } }) + mockGetUserEntityPermissions.mockResolvedValue('admin') + + mockDb.select.mockReturnValue({ from: mockFrom }) + mockFrom.mockReturnValue({ where: mockWhere }) + + mockTxUpdate.mockReturnValue({ + set: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) }), + }) + mockDb.transaction.mockImplementation(async (cb: (tx: unknown) => Promise) => + cb({ update: mockTxUpdate }) + ) + }) + + it('reorders folders when updates are valid', async () => { + mockWhere + .mockReturnValueOnce([{ id: 'folder-1', workspaceId: 'workspace-123' }]) + .mockReturnValueOnce([{ id: 'folder-1', parentId: null }]) + + const req = createMockRequest('PUT', { + workspaceId: 'workspace-123', + updates: [{ id: 'folder-1', sortOrder: 2, parentId: null }], + }) + + const response = await PUT(req) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data).toMatchObject({ success: true, updated: 1 }) + }) + + it('rejects a parentId that belongs to another workspace', async () => { + mockWhere + .mockReturnValueOnce([{ id: 'folder-1', workspaceId: 'workspace-123' }]) + .mockReturnValueOnce([{ id: 'foreign', workspaceId: 'workspace-OTHER', archivedAt: null }]) + + const req = createMockRequest('PUT', { + workspaceId: 'workspace-123', + updates: [{ id: 'folder-1', sortOrder: 0, parentId: 'foreign' }], + }) + + const response = await PUT(req) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.error).toBe('Parent folder not found') + expect(mockDb.transaction).not.toHaveBeenCalled() + }) + + it('rejects a batch that would form a cycle', async () => { + mockWhere + .mockReturnValueOnce([ + { id: 'A', workspaceId: 'workspace-123' }, + { id: 'B', workspaceId: 'workspace-123' }, + ]) + .mockReturnValueOnce([ + { id: 'A', workspaceId: 'workspace-123', archivedAt: null }, + { id: 'B', workspaceId: 'workspace-123', archivedAt: null }, + ]) + .mockReturnValueOnce([ + { id: 'A', parentId: null }, + { id: 'B', parentId: null }, + ]) + + const req = createMockRequest('PUT', { + workspaceId: 'workspace-123', + updates: [ + { id: 'A', sortOrder: 0, parentId: 'B' }, + { id: 'B', sortOrder: 0, parentId: 'A' }, + ], + }) + + const response = await PUT(req) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.error).toBe('Cannot create circular folder reference') + expect(mockDb.transaction).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/folders/reorder/route.ts b/apps/sim/app/api/folders/reorder/route.ts index 87222b75b8d..274e2bc7784 100644 --- a/apps/sim/app/api/folders/reorder/route.ts +++ b/apps/sim/app/api/folders/reorder/route.ts @@ -51,6 +51,65 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { return NextResponse.json({ error: 'No valid folders to update' }, { status: 400 }) } + const targetParentIds = Array.from( + new Set(validUpdates.map((u) => u.parentId).filter((id): id is string => Boolean(id))) + ) + + if (targetParentIds.length > 0) { + const parentFolders = await db + .select({ + id: workflowFolder.id, + workspaceId: workflowFolder.workspaceId, + archivedAt: workflowFolder.archivedAt, + }) + .from(workflowFolder) + .where(inArray(workflowFolder.id, targetParentIds)) + + const validParentIds = new Set( + parentFolders.filter((f) => f.workspaceId === workspaceId && !f.archivedAt).map((f) => f.id) + ) + + for (const update of validUpdates) { + if (!update.parentId) continue + if (update.parentId === update.id) { + return NextResponse.json({ error: 'Folder cannot be its own parent' }, { status: 400 }) + } + if (!validParentIds.has(update.parentId)) { + return NextResponse.json({ error: 'Parent folder not found' }, { status: 400 }) + } + } + } + + const workspaceFolders = await db + .select({ id: workflowFolder.id, parentId: workflowFolder.parentId }) + .from(workflowFolder) + .where(eq(workflowFolder.workspaceId, workspaceId)) + + const parentById = new Map() + for (const folder of workspaceFolders) { + parentById.set(folder.id, folder.parentId) + } + for (const update of validUpdates) { + if (update.parentId !== undefined) { + parentById.set(update.id, update.parentId || null) + } + } + + for (const update of validUpdates) { + const visited = new Set() + let cursor: string | null = update.id + while (cursor) { + if (visited.has(cursor)) { + return NextResponse.json( + { error: 'Cannot create circular folder reference' }, + { status: 400 } + ) + } + visited.add(cursor) + cursor = parentById.get(cursor) ?? null + } + } + for (const update of validUpdates) { await assertFolderMutable(update.id) if (update.parentId !== undefined) { @@ -65,7 +124,7 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { updatedAt: new Date(), } if (update.parentId !== undefined) { - updateData.parentId = update.parentId + updateData.parentId = update.parentId || null } await tx.update(workflowFolder).set(updateData).where(eq(workflowFolder.id, update.id)) } diff --git a/apps/sim/app/api/folders/route.test.ts b/apps/sim/app/api/folders/route.test.ts index a2e145bf1e3..a72c0a7bbf8 100644 --- a/apps/sim/app/api/folders/route.test.ts +++ b/apps/sim/app/api/folders/route.test.ts @@ -127,6 +127,7 @@ describe('Folders API Route', () => { const mockSelect = mockDb.select const mockFrom = vi.fn() const mockWhere = vi.fn() + const mockLimit = vi.fn() const mockOrderBy = vi.fn() const mockInsert = mockDb.insert const mockValues = vi.fn() @@ -152,9 +153,12 @@ describe('Folders API Route', () => { mockFrom.mockReturnValue({ where: mockWhere }) const defaultWhereResult = [] as Array> & { orderBy: typeof mockOrderBy + limit: typeof mockLimit } defaultWhereResult.orderBy = mockOrderBy + defaultWhereResult.limit = mockLimit mockWhere.mockReturnValue(defaultWhereResult) + mockLimit.mockReturnValue([]) mockOrderBy.mockReturnValue(mockFolders) mockInsert.mockReturnValue({ values: mockValues }) @@ -367,6 +371,7 @@ describe('Folders API Route', () => { insertResult: [{ ...mockFolders[1] }], }) ) + mockLimit.mockReturnValueOnce([{ ...mockFolders[0] }]) mockReturning.mockReturnValueOnce([{ ...mockFolders[1] }]) const req = createMockRequest('POST', { @@ -385,6 +390,24 @@ describe('Folders API Route', () => { }) }) + it('should reject a parentId that does not resolve to a folder in the workspace', async () => { + mockAuthenticatedUser() + + mockLimit.mockReturnValueOnce([]) + + const req = createMockRequest('POST', { + name: 'Subfolder', + workspaceId: 'workspace-123', + parentId: 'folder-in-other-workspace', + }) + + const response = await POST(req) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.error).toBe('Parent folder not found') + }) + it('should return 401 for unauthenticated requests', async () => { mockUnauthenticated() diff --git a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts index c977a8c5edf..8e80e41f6e3 100644 --- a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts +++ b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts @@ -160,8 +160,6 @@ export const POST = withRouteHandler( ) } - // Allow manual chunk creation even if document is not fully processed - // but it should exist and not be in failed state if (doc.processingStatus === 'failed') { logger.warn(`[${requestId}] Document ${documentId} is in failed state, cannot add chunks`) return NextResponse.json({ error: 'Cannot add chunks to failed document' }, { status: 400 }) @@ -171,7 +169,6 @@ export const POST = withRouteHandler( const validatedData = createChunkBodySchema.parse(searchParams) const docTags = { - // Text tags (7 slots) tag1: doc.tag1 ?? null, tag2: doc.tag2 ?? null, tag3: doc.tag3 ?? null, @@ -179,16 +176,13 @@ export const POST = withRouteHandler( tag5: doc.tag5 ?? null, tag6: doc.tag6 ?? null, tag7: doc.tag7 ?? null, - // Number tags (5 slots) number1: doc.number1 ?? null, number2: doc.number2 ?? null, number3: doc.number3 ?? null, number4: doc.number4 ?? null, number5: doc.number5 ?? null, - // Date tags (2 slots) date1: doc.date1 ?? null, date2: doc.date2 ?? null, - // Boolean tags (3 slots) boolean1: doc.boolean1 ?? null, boolean2: doc.boolean2 ?? null, boolean3: doc.boolean3 ?? null, @@ -215,7 +209,6 @@ export const POST = withRouteHandler( logger.warn(`[${requestId}] Failed to calculate cost for chunk upload`, { error: getErrorMessage(error, 'Unknown error'), }) - // Continue without cost information rather than failing the upload } return NextResponse.json({ @@ -274,7 +267,7 @@ export const PATCH = withRouteHandler( } const userId = auth.userId - const accessCheck = await checkDocumentAccess(knowledgeBaseId, documentId, userId) + const accessCheck = await checkDocumentWriteAccess(knowledgeBaseId, documentId, userId) if (!accessCheck.hasAccess) { if (accessCheck.notFound) { diff --git a/apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts b/apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts index ae73a270745..7bbf9a5ea01 100644 --- a/apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts +++ b/apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts @@ -59,6 +59,12 @@ import type { const logger = createLogger('AdminWorkspaceImportAPI') +/** + * Body cap for admin bulk workflow imports, which can carry many serialized + * workflows and legitimately exceed the default contract-route limit. + */ +const ADMIN_IMPORT_MAX_BODY_BYTES = 100 * 1024 * 1024 + interface RouteParams { id: string } @@ -88,10 +94,13 @@ export const POST = withRouteHandler( let workflowsToImport: ParsedWorkflow[] = [] if (contentType.includes('application/json')) { - const rawBody = await parseJsonBody(request) + const rawBody = await parseJsonBody(request, 'response', ADMIN_IMPORT_MAX_BODY_BYTES) if (!rawBody.success) { - return badRequestResponse('Invalid JSON body. Expected { workflows: [...] }') + // Preserve the 413 for an oversized body; only invalid JSON maps to 400. + return rawBody.reason === 'too_large' + ? rawBody.response + : badRequestResponse('Invalid JSON body. Expected { workflows: [...] }') } const validation = adminV1WorkspaceImportBodySchema.safeParse(rawBody.data) diff --git a/apps/sim/app/api/v1/files/route.ts b/apps/sim/app/api/v1/files/route.ts index 06d965ac028..9e573a126f8 100644 --- a/apps/sim/app/api/v1/files/route.ts +++ b/apps/sim/app/api/v1/files/route.ts @@ -29,7 +29,7 @@ const logger = createLogger('V1FilesAPI') export const dynamic = 'force-dynamic' export const revalidate = 0 -const MAX_FILE_SIZE = 100 * 1024 * 1024 // 100MB +const MAX_FILE_SIZE = 100 * 1024 * 1024 const MAX_MULTIPART_OVERHEAD_BYTES = 1024 * 1024 /** GET /api/v1/files — List all files in a workspace. */ @@ -117,7 +117,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } const { workspaceId } = formFieldsResult.data - const scopeError = checkWorkspaceScope(rateLimit, workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, workspaceId) if (scopeError) return scopeError if (!file) { diff --git a/apps/sim/app/api/v1/logs/route.ts b/apps/sim/app/api/v1/logs/route.ts index c85031e3cc4..dd20072dc09 100644 --- a/apps/sim/app/api/v1/logs/route.ts +++ b/apps/sim/app/api/v1/logs/route.ts @@ -68,7 +68,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => { const params = parsed.data.query - const scopeError = checkWorkspaceScope(rateLimit, params.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, params.workspaceId) if (scopeError) return scopeError logger.info(`[${requestId}] Fetching logs for workspace ${params.workspaceId}`, { @@ -147,8 +147,6 @@ export const GET = withRouteHandler(async (request: NextRequest) => { }) } - // Only materialize externalized execution data when the response actually - // needs it (details=full + finalOutput/traceSpans requested). const needsMaterialize = params.details === 'full' && (params.includeFinalOutput || params.includeTraceSpans) @@ -179,9 +177,6 @@ export const GET = withRouteHandler(async (request: NextRequest) => { return result } - // Only run the bounded-concurrency materialization when the response actually - // needs object-storage reads; otherwise a plain synchronous map avoids the - // per-row worker/promise overhead. const formattedLogs = needsMaterialize ? await mapWithConcurrency(data, MATERIALIZE_CONCURRENCY, async (log) => { const result = buildBase(log) diff --git a/apps/sim/app/api/v1/middleware.ts b/apps/sim/app/api/v1/middleware.ts index 92aa72eb344..6472cbb1f76 100644 --- a/apps/sim/app/api/v1/middleware.ts +++ b/apps/sim/app/api/v1/middleware.ts @@ -5,6 +5,7 @@ import type { SubscriptionPlan } from '@/lib/core/rate-limiter' import { getRateLimit, RateLimiter } from '@/lib/core/rate-limiter' import { generateRequestId } from '@/lib/core/utils/request' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' +import { getWorkspaceBillingSettings } from '@/lib/workspaces/utils' import { authenticateV1Request } from '@/app/api/v1/auth' const logger = createLogger('V1Middleware') @@ -152,11 +153,19 @@ export function createRateLimitResponse(result: RateLimitResult): NextResponse { ) } -/** Verify that a workspace-scoped API key is only used for its own workspace. */ -export function checkWorkspaceScope( +/** + * Verify that the API key is allowed to access the requested workspace. + * + * Enforces two policies: + * - A workspace-scoped key may only target its own workspace. + * - A personal key is rejected when the workspace has disabled personal API + * keys (`allowPersonalApiKeys = false`), matching the workflow-execution + * surface in `app/api/workflows/middleware.ts`. + */ +export async function checkWorkspaceScope( rateLimit: RateLimitResult, requestedWorkspaceId: string -): NextResponse | null { +): Promise { if ( rateLimit.keyType === 'workspace' && rateLimit.workspaceId && @@ -167,6 +176,17 @@ export function checkWorkspaceScope( { status: 403 } ) } + + if (rateLimit.keyType === 'personal') { + const settings = await getWorkspaceBillingSettings(requestedWorkspaceId) + if (!settings?.allowPersonalApiKeys) { + return NextResponse.json( + { error: 'Personal API keys are not allowed for this workspace' }, + { status: 403 } + ) + } + } + return null } @@ -180,7 +200,7 @@ export async function validateWorkspaceAccess( workspaceId: string, level: 'read' | 'write' = 'read' ): Promise { - const scopeError = checkWorkspaceScope(rateLimit, workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, workspaceId) if (scopeError) return scopeError const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) diff --git a/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts b/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts index 657b4487200..0eeebfb99ce 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts @@ -49,7 +49,7 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Colum const { tableId } = parsed.data.params const validated = parsed.data.body - const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, validated.workspaceId) if (scopeError) return scopeError const result = await checkAccess(tableId, userId, 'write') @@ -116,7 +116,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu const { tableId } = parsed.data.params const validated = parsed.data.body - const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, validated.workspaceId) if (scopeError) return scopeError const result = await checkAccess(tableId, userId, 'write') @@ -224,7 +224,7 @@ export const DELETE = withRouteHandler( const { tableId } = parsed.data.params const validated = parsed.data.body - const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, validated.workspaceId) if (scopeError) return scopeError const result = await checkAccess(tableId, userId, 'write') diff --git a/apps/sim/app/api/v1/tables/[tableId]/route.ts b/apps/sim/app/api/v1/tables/[tableId]/route.ts index bf9c38b406d..a64831c857c 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/route.ts @@ -51,7 +51,7 @@ export const GET = withRouteHandler(async (request: NextRequest, context: TableR const { tableId } = parsed.data.params const { workspaceId } = parsed.data.query - const scopeError = checkWorkspaceScope(rateLimit, workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, workspaceId) if (scopeError) return scopeError const result = await checkAccess(tableId, userId, 'read') @@ -123,7 +123,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Tab const { tableId } = parsed.data.params const { workspaceId } = parsed.data.query - const scopeError = checkWorkspaceScope(rateLimit, workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, workspaceId) if (scopeError) return scopeError const result = await checkAccess(tableId, userId, 'write') diff --git a/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts index 122603b7133..4aa1d85f93f 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts @@ -55,7 +55,7 @@ export const GET = withRouteHandler(async (request: NextRequest, context: RowRou const { tableId, rowId } = parsed.data.params const { workspaceId } = parsed.data.query - const scopeError = checkWorkspaceScope(rateLimit, workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, workspaceId) if (scopeError) return scopeError const result = await checkAccess(tableId, userId, 'read') @@ -124,7 +124,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR const { tableId, rowId } = parsed.data.params const validated = parsed.data.body - const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, validated.workspaceId) if (scopeError) return scopeError const result = await checkAccess(tableId, userId, 'write') @@ -221,7 +221,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row const { tableId, rowId } = parsed.data.params const { workspaceId } = parsed.data.query - const scopeError = checkWorkspaceScope(rateLimit, workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, workspaceId) if (scopeError) return scopeError const result = await checkAccess(tableId, userId, 'write') diff --git a/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts b/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts index 28c4e209cd0..ecceb41b1e2 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/rows/route.ts @@ -149,7 +149,7 @@ export const GET = withRouteHandler(async (request: NextRequest, context: TableR const { tableId } = parsed.data.params const validated = parsed.data.query - const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, validated.workspaceId) if (scopeError) return scopeError const accessResult = await checkAccess(tableId, userId, 'read') @@ -229,14 +229,14 @@ export const POST = withRouteHandler( const { tableId } = parsed.data.params if ('rows' in parsed.data.body) { const batchValidated = parsed.data.body - const scopeError = checkWorkspaceScope(rateLimit, batchValidated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, batchValidated.workspaceId) if (scopeError) return scopeError return handleBatchInsert(requestId, tableId, batchValidated, userId) } const validated = parsed.data.body - const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, validated.workspaceId) if (scopeError) return scopeError const accessResult = await checkAccess(tableId, userId, 'write') @@ -321,7 +321,7 @@ export const PUT = withRouteHandler(async (request: NextRequest, context: TableR const { tableId } = parsed.data.params const validated = parsed.data.body - const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, validated.workspaceId) if (scopeError) return scopeError const accessResult = await checkAccess(tableId, userId, 'write') @@ -416,7 +416,7 @@ export const DELETE = withRouteHandler( const { tableId } = parsed.data.params const validated = parsed.data.body - const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, validated.workspaceId) if (scopeError) return scopeError const accessResult = await checkAccess(tableId, userId, 'write') diff --git a/apps/sim/app/api/v1/tables/[tableId]/rows/upsert/route.ts b/apps/sim/app/api/v1/tables/[tableId]/rows/upsert/route.ts index 22587546029..285cd6d1e31 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/rows/upsert/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/rows/upsert/route.ts @@ -45,7 +45,7 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Upser const { tableId } = parsed.data.params const validated = parsed.data.body - const scopeError = checkWorkspaceScope(rateLimit, validated.workspaceId) + const scopeError = await checkWorkspaceScope(rateLimit, validated.workspaceId) if (scopeError) return scopeError const result = await checkAccess(tableId, userId, 'write') diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index 800d4bd6873..c8e89c259b0 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.ts @@ -8,6 +8,7 @@ import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { executeWorkflowBodySchema } from '@/lib/api/contracts/workflows' import { AuthType, checkHybridAuth, hasExternalApiCredentials } from '@/lib/auth/hybrid' +import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation' import { admissionRejectedResponse, tryAdmit } from '@/lib/core/admission/gate' import { getJobQueue, shouldExecuteInline } from '@/lib/core/async-jobs' import { @@ -303,6 +304,11 @@ async function handleAsyncExecution(params: AsyncExecutionParams): Promise { for (const update of validUpdates) { await assertWorkflowMutable(update.id) if (update.folderId !== undefined) { + await assertFolderInWorkspace(update.folderId, workspaceId) await assertFolderMutable(update.folderId) } } @@ -82,7 +85,11 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { return NextResponse.json({ success: true, updated: validUpdates.length }) } catch (error) { - if (error instanceof WorkflowLockedError || error instanceof FolderLockedError) { + if ( + error instanceof WorkflowLockedError || + error instanceof FolderLockedError || + error instanceof FolderNotFoundError + ) { return NextResponse.json({ error: error.message }, { status: error.status }) } diff --git a/apps/sim/app/api/workflows/route.ts b/apps/sim/app/api/workflows/route.ts index b1437de78a5..6a8738869f7 100644 --- a/apps/sim/app/api/workflows/route.ts +++ b/apps/sim/app/api/workflows/route.ts @@ -190,7 +190,8 @@ export const POST = withRouteHandler(async (req: NextRequest) => { }) if (!result.success || !result.workflow) { - const status = result.errorCode === 'conflict' ? 409 : 500 + const status = + result.errorCode === 'conflict' ? 409 : result.errorCode === 'validation' ? 400 : 500 return NextResponse.json({ error: result.error }, { status }) } diff --git a/apps/sim/app/api/workspaces/[id]/environment/route.test.ts b/apps/sim/app/api/workspaces/[id]/environment/route.test.ts new file mode 100644 index 00000000000..6ad61921591 --- /dev/null +++ b/apps/sim/app/api/workspaces/[id]/environment/route.test.ts @@ -0,0 +1,143 @@ +/** + * @vitest-environment node + */ +import { createMockRequest } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockGetSession, + mockGetWorkspaceById, + mockGetUserEntityPermissions, + mockGetPersonalAndWorkspaceEnv, + mockGetWorkspaceEnvKeyAdminAccess, +} = vi.hoisted(() => ({ + mockGetSession: vi.fn(), + mockGetWorkspaceById: vi.fn(), + mockGetUserEntityPermissions: vi.fn(), + mockGetPersonalAndWorkspaceEnv: vi.fn(), + mockGetWorkspaceEnvKeyAdminAccess: vi.fn(), +})) + +vi.mock('@/lib/auth', () => ({ + auth: { api: { getSession: vi.fn() } }, + getSession: mockGetSession, +})) + +vi.mock('@/lib/workspaces/permissions/utils', () => ({ + getWorkspaceById: mockGetWorkspaceById, + getUserEntityPermissions: mockGetUserEntityPermissions, +})) + +vi.mock('@/lib/environment/utils', () => ({ + getPersonalAndWorkspaceEnv: mockGetPersonalAndWorkspaceEnv, + invalidateEffectiveDecryptedEnvCache: vi.fn(), +})) + +vi.mock('@/lib/credentials/environment', () => ({ + getWorkspaceEnvKeyAdminAccess: mockGetWorkspaceEnvKeyAdminAccess, + createWorkspaceEnvCredentials: vi.fn(), + deleteWorkspaceEnvCredentials: vi.fn(), +})) + +import { GET } from '@/app/api/workspaces/[id]/environment/route' + +const WORKSPACE_ID = 'ws-1' + +function buildParams() { + return { params: Promise.resolve({ id: WORKSPACE_ID }) } +} + +async function callGet() { + const request = createMockRequest('GET') + const response = await GET(request, buildParams()) + return { status: response.status, body: await response.json() } +} + +describe('GET /api/workspaces/[id]/environment', () => { + beforeEach(() => { + vi.clearAllMocks() + mockGetSession.mockResolvedValue({ user: { id: 'u-1' } }) + mockGetWorkspaceById.mockResolvedValue({ id: WORKSPACE_ID }) + mockGetPersonalAndWorkspaceEnv.mockResolvedValue({ + workspaceDecrypted: { OPENAI_API_KEY: 'sk-secret', DATABASE_URL: 'postgres://secret' }, + personalDecrypted: { PERSONAL: { value: 'p' } }, + conflicts: [], + }) + }) + + it('returns 401 when the caller has no workspace permission', async () => { + mockGetUserEntityPermissions.mockResolvedValue(null) + + const { status, body } = await callGet() + + expect(status).toBe(401) + expect(body.error).toBe('Unauthorized') + expect(mockGetPersonalAndWorkspaceEnv).not.toHaveBeenCalled() + }) + + it('masks workspace secret values for a read-only member', async () => { + mockGetUserEntityPermissions.mockResolvedValue('read') + mockGetWorkspaceEnvKeyAdminAccess.mockResolvedValue({ + adminKeys: new Set(), + knownKeys: new Set(['OPENAI_API_KEY', 'DATABASE_URL']), + }) + + const { status, body } = await callGet() + + expect(status).toBe(200) + expect(Object.keys(body.data.workspace).sort()).toEqual(['DATABASE_URL', 'OPENAI_API_KEY']) + expect(body.data.workspace.OPENAI_API_KEY).toBe('') + expect(body.data.workspace.DATABASE_URL).toBe('') + }) + + it('reveals only the workspace values the caller is a credential admin of', async () => { + mockGetUserEntityPermissions.mockResolvedValue('write') + mockGetWorkspaceEnvKeyAdminAccess.mockResolvedValue({ + adminKeys: new Set(['OPENAI_API_KEY']), + knownKeys: new Set(['OPENAI_API_KEY', 'DATABASE_URL']), + }) + + const { body } = await callGet() + + expect(body.data.workspace.OPENAI_API_KEY).toBe('sk-secret') + expect(body.data.workspace.DATABASE_URL).toBe('') + }) + + it('reveals legacy keys (no per-secret ACL) only to workspace admins', async () => { + mockGetUserEntityPermissions.mockResolvedValue('admin') + mockGetWorkspaceEnvKeyAdminAccess.mockResolvedValue({ + adminKeys: new Set(), + knownKeys: new Set(), + }) + + const { body } = await callGet() + + expect(body.data.workspace.OPENAI_API_KEY).toBe('sk-secret') + expect(body.data.workspace.DATABASE_URL).toBe('postgres://secret') + }) + + it('does not reveal legacy keys to a non-admin member', async () => { + mockGetUserEntityPermissions.mockResolvedValue('write') + mockGetWorkspaceEnvKeyAdminAccess.mockResolvedValue({ + adminKeys: new Set(), + knownKeys: new Set(), + }) + + const { body } = await callGet() + + expect(body.data.workspace.OPENAI_API_KEY).toBe('') + expect(body.data.workspace.DATABASE_URL).toBe('') + }) + + it('always returns personal values untouched', async () => { + mockGetUserEntityPermissions.mockResolvedValue('read') + mockGetWorkspaceEnvKeyAdminAccess.mockResolvedValue({ + adminKeys: new Set(), + knownKeys: new Set(['OPENAI_API_KEY', 'DATABASE_URL']), + }) + + const { body } = await callGet() + + expect(body.data.personal).toEqual({ PERSONAL: { value: 'p' } }) + }) +}) diff --git a/apps/sim/app/api/workspaces/[id]/environment/route.ts b/apps/sim/app/api/workspaces/[id]/environment/route.ts index cfcdf8a77db..c32065c49d3 100644 --- a/apps/sim/app/api/workspaces/[id]/environment/route.ts +++ b/apps/sim/app/api/workspaces/[id]/environment/route.ts @@ -25,10 +25,49 @@ import { invalidateEffectiveDecryptedEnvCache, } from '@/lib/environment/utils' import { captureServerEvent } from '@/lib/posthog/server' -import { getUserEntityPermissions, getWorkspaceById } from '@/lib/workspaces/permissions/utils' +import { + getUserEntityPermissions, + getWorkspaceById, + type PermissionType, +} from '@/lib/workspaces/permissions/utils' const logger = createLogger('WorkspaceEnvironmentAPI') +/** + * Restricts decrypted workspace env values to administrators. Members (including + * read-only) receive the variable names with empty values so editor autocomplete + * and conflict detection keep working without leaking secret values. A value is + * revealed when the caller is a credential admin of that key, or — for legacy + * keys predating per-secret ACLs — when they hold workspace `admin` permission. + * Mirrors the per-key edit gating in PUT/DELETE: if you can administer a secret, + * you can read it. + */ +async function maskWorkspaceEnvForViewer({ + workspaceDecrypted, + workspaceId, + userId, + permission, +}: { + workspaceDecrypted: Record + workspaceId: string + userId: string + permission: PermissionType +}): Promise> { + const workspaceKeys = Object.keys(workspaceDecrypted) + const { adminKeys, knownKeys } = await getWorkspaceEnvKeyAdminAccess({ + workspaceId, + envKeys: workspaceKeys, + userId, + }) + + const masked: Record = {} + for (const key of workspaceKeys) { + const canViewValue = adminKeys.has(key) || (!knownKeys.has(key) && permission === 'admin') + masked[key] = canViewValue ? workspaceDecrypted[key] : '' + } + return masked +} + export const GET = withRouteHandler( async (request: NextRequest, { params }: { params: Promise<{ id: string }> }) => { const requestId = generateRequestId() @@ -43,13 +82,11 @@ export const GET = withRouteHandler( const userId = session.user.id - // Validate workspace exists const ws = await getWorkspaceById(workspaceId) if (!ws) { return NextResponse.json({ error: 'Workspace not found' }, { status: 404 }) } - // Require any permission to read const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) if (!permission) { return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) @@ -60,10 +97,17 @@ export const GET = withRouteHandler( workspaceId ) + const workspace = await maskWorkspaceEnvForViewer({ + workspaceDecrypted, + workspaceId, + userId, + permission, + }) + return NextResponse.json( { data: { - workspace: workspaceDecrypted, + workspace, personal: personalDecrypted, conflicts, }, @@ -80,6 +124,12 @@ export const GET = withRouteHandler( } ) +/** + * Upserts workspace environment variables under tiered authorization: the caller + * needs some workspace permission, editing an existing secret requires + * credential-admin on that key, and adding a brand-new key requires workspace + * write/admin. + */ export const PUT = withRouteHandler( async (request: NextRequest, context: { params: Promise<{ id: string }> }) => { const requestId = generateRequestId() @@ -98,9 +148,6 @@ export const PUT = withRouteHandler( if (!parsed.success) return parsed.response const { variables } = parsed.data.body - // Caller must have workspace access at all (blocks non-member writes); - // per-key gating below then requires credential-admin to edit existing - // secrets and write/admin to add brand-new keys. const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) if (!permission) { return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) @@ -221,6 +268,11 @@ export const PUT = withRouteHandler( } ) +/** + * Removes workspace environment variables. Deleting an existing secret requires + * credential-admin on that key; a key with no credential yet (legacy) falls back + * to workspace write/admin. + */ export const DELETE = withRouteHandler( async (request: NextRequest, context: { params: Promise<{ id: string }> }) => { const requestId = generateRequestId() @@ -239,9 +291,6 @@ export const DELETE = withRouteHandler( if (!parsed.success) return parsed.response const { keys } = parsed.data.body - // Caller must have workspace access at all; deleting an existing secret then - // requires being its credential admin, while a key with no credential yet - // (legacy) falls back to workspace write/admin. const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) if (!permission) { return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) diff --git a/apps/sim/background/workflow-column-execution.ts b/apps/sim/background/workflow-column-execution.ts index 44c87461d8c..bba438c56dd 100644 --- a/apps/sim/background/workflow-column-execution.ts +++ b/apps/sim/background/workflow-column-execution.ts @@ -511,6 +511,7 @@ async function runWorkflowAndWriteTerminal( triggerType: 'workflow', checkDeployment: false, checkRateLimit: false, + skipConcurrencyReservation: true, logPreprocessingErrors: false, }) if (!preprocess.success) { diff --git a/apps/sim/components/emails/auth/existing-account-email.tsx b/apps/sim/components/emails/auth/existing-account-email.tsx new file mode 100644 index 00000000000..0fc8f7e7524 --- /dev/null +++ b/apps/sim/components/emails/auth/existing-account-email.tsx @@ -0,0 +1,46 @@ +import { Link, Text } from '@react-email/components' +import { baseStyles } from '@/components/emails/_styles' +import { EmailLayout } from '@/components/emails/components' +import { getBaseUrl } from '@/lib/core/utils/urls' +import { getBrandConfig } from '@/ee/whitelabeling' + +interface ExistingAccountEmailProps { + username?: string +} + +/** + * Sent out-of-band when someone attempts to sign up with an email that already + * has an account. The sign-up endpoint itself returns a generic success + * response to avoid account enumeration, so this email is how the real account + * owner learns of the attempt. + */ +export function ExistingAccountEmail({ username = '' }: ExistingAccountEmailProps) { + const brand = getBrandConfig() + const loginLink = `${getBaseUrl()}/login` + + return ( + + Hello {username}, + + Someone just tried to create a {brand.name} account using this email address, but an account + already exists. If this was you, sign in instead — or reset your password if you've + forgotten it. + + + + Sign In + + +
+ + + If this wasn't you, no action is needed — no account was created or changed. + + + ) +} + +export default ExistingAccountEmail diff --git a/apps/sim/components/emails/auth/index.ts b/apps/sim/components/emails/auth/index.ts index 3d5438dd69a..de9eff40e91 100644 --- a/apps/sim/components/emails/auth/index.ts +++ b/apps/sim/components/emails/auth/index.ts @@ -1,3 +1,4 @@ +export { ExistingAccountEmail } from './existing-account-email' export { OnboardingFollowupEmail } from './onboarding-followup-email' export { OTPVerificationEmail } from './otp-verification-email' export { ResetPasswordEmail } from './reset-password-email' diff --git a/apps/sim/components/emails/render.ts b/apps/sim/components/emails/render.ts index 645f5a056b0..94650c85f15 100644 --- a/apps/sim/components/emails/render.ts +++ b/apps/sim/components/emails/render.ts @@ -1,5 +1,6 @@ import { render } from '@react-email/render' import { + ExistingAccountEmail, OnboardingFollowupEmail, OTPVerificationEmail, ResetPasswordEmail, @@ -49,6 +50,10 @@ export async function renderOTPEmail( return await render(OTPVerificationEmail({ otp, email, type, chatTitle })) } +export async function renderExistingAccountEmail(username: string): Promise { + return await render(ExistingAccountEmail({ username })) +} + export async function renderPasswordResetEmail( username: string, resetLink: string diff --git a/apps/sim/components/emails/subjects.ts b/apps/sim/components/emails/subjects.ts index 347bf4074d0..a1ddbd3ed3b 100644 --- a/apps/sim/components/emails/subjects.ts +++ b/apps/sim/components/emails/subjects.ts @@ -7,6 +7,7 @@ export type EmailSubjectType = | 'change-email' | 'forget-password' | 'reset-password' + | 'existing-account' | 'invitation' | 'batch-invitation' | 'polling-group-invitation' @@ -41,6 +42,8 @@ export function getEmailSubject(type: EmailSubjectType): string { return `Reset your ${brandName} password` case 'reset-password': return `Reset your ${brandName} password` + case 'existing-account': + return `Sign-up attempt with your ${brandName} email` case 'invitation': return `You've been invited to join a team on ${brandName}` case 'batch-invitation': diff --git a/apps/sim/components/icons.tsx b/apps/sim/components/icons.tsx index 41732af3b29..bfd2ade63a7 100644 --- a/apps/sim/components/icons.tsx +++ b/apps/sim/components/icons.tsx @@ -2492,7 +2492,7 @@ export function LinkupIcon(props: SVGProps) { ) @@ -4967,7 +4967,7 @@ export function InfisicalIcon(props: SVGProps) { ) diff --git a/apps/sim/lib/api/contracts/chats.ts b/apps/sim/lib/api/contracts/chats.ts index c3e908121b6..d0840bca8a5 100644 --- a/apps/sim/lib/api/contracts/chats.ts +++ b/apps/sim/lib/api/contracts/chats.ts @@ -105,25 +105,36 @@ export const deployedChatConfigSchema = z.object({ export type DeployedChatConfig = z.output export const deployedChatAuthBodySchema = z.object({ - password: z.string().optional(), + password: z.string().max(1024, 'Password is too long').optional(), email: z.string().email('Invalid email format').optional().or(z.literal('')), }) export type DeployedChatAuthBody = z.input +const MAX_CHAT_INPUT_CHARS = 1_000_000 +const MAX_CHAT_FILE_DATA_CHARS = 14 * 1024 * 1024 +const MAX_CHAT_FILES = 15 + export const deployedChatFileSchema = z.object({ - name: z.string().min(1, 'File name is required'), - type: z.string().min(1, 'File type is required'), + name: z.string().min(1, 'File name is required').max(255, 'File name is too long'), + type: z.string().min(1, 'File type is required').max(255, 'File type is too long'), size: z.number().positive('File size must be positive'), - data: z.string().min(1, 'File data is required'), + data: z + .string() + .min(1, 'File data is required') + .max(MAX_CHAT_FILE_DATA_CHARS, 'File data exceeds the maximum allowed size'), lastModified: z.number().optional(), }) export const deployedChatPostBodySchema = z.object({ - input: z.string().optional(), - password: z.string().optional(), + input: z.string().max(MAX_CHAT_INPUT_CHARS, 'Input is too long').optional(), + password: z.string().max(1024, 'Password is too long').optional(), email: z.string().email('Invalid email format').optional().or(z.literal('')), - conversationId: z.string().optional(), - files: z.array(deployedChatFileSchema).optional().default([]), + conversationId: z.string().max(256, 'Conversation ID is too long').optional(), + files: z + .array(deployedChatFileSchema) + .max(MAX_CHAT_FILES, `A maximum of ${MAX_CHAT_FILES} files is allowed`) + .optional() + .default([]), }) export type DeployedChatPostBody = z.input diff --git a/apps/sim/lib/api/server/validation.ts b/apps/sim/lib/api/server/validation.ts index 4128efcfa90..75cd5cbce8d 100644 --- a/apps/sim/lib/api/server/validation.ts +++ b/apps/sim/lib/api/server/validation.ts @@ -8,6 +8,23 @@ import type { ContractParams, ContractQuery, } from '@/lib/api/contracts' +import { env } from '@/lib/core/config/env' +import { + assertContentLengthWithinLimit, + isPayloadSizeLimitError, + readStreamToBufferWithLimit, +} from '@/lib/core/utils/stream-limits' + +/** + * Default upper bound on the JSON request body that contract routes will read + * and parse into memory. Next.js App Router imposes no body cap, so without + * this an unauthenticated caller could buffer an arbitrarily large body before + * schema validation runs. Override per-route via `ParseRequestOptions.maxBodyBytes`. + * Falls back to 50 MB if the env value is missing or non-numeric so a misconfig + * can never silently disable the cap (a NaN limit would never reject). + */ +export const DEFAULT_MAX_JSON_BODY_BYTES = + Number.parseInt(env.API_MAX_JSON_BODY_BYTES, 10) || 50 * 1024 * 1024 export interface ValidationErrorBody { error: string @@ -35,6 +52,12 @@ export interface ParseRequestOptions { validationErrorResponse?: (error: z.ZodError) => NextResponse invalidJsonResponse?: () => NextResponse invalidJson?: 'response' | 'throw' + /** + * Maximum number of bytes to read for the JSON body before rejecting with a + * 413. Defaults to {@link DEFAULT_MAX_JSON_BODY_BYTES}. Raise this only for + * routes that legitimately accept large JSON payloads (e.g. inline file uploads). + */ + maxBodyBytes?: number } export function serializeZodIssues(error: z.ZodError): z.core.$ZodIssue[] { @@ -69,18 +92,61 @@ export function validationErrorResponseFromError( return validationErrorResponse(error, message, status) } +const REQUEST_BODY_LABEL = 'Request body' + +/** + * Reads the JSON body while enforcing a byte cap. The body is read through a + * size-limited stream so chunked/streamed bodies are bounded even when the + * `content-length` header is absent or lies about the true size. When no + * readable stream is available (e.g. a mocked request) the content-length guard + * is the only bound and parsing falls back to {@link Request.json}. Decoding + * uses {@link TextDecoder} so a leading UTF-8 BOM is stripped, matching the spec + * "UTF-8 decode" behavior of `request.json()`. + */ +async function readJsonBodyWithLimit(request: Request, maxBytes: number): Promise { + assertContentLengthWithinLimit(request.headers, maxBytes, REQUEST_BODY_LABEL) + + const stream = request.body + if (!stream) { + return request.json() + } + + const buffer = await readStreamToBufferWithLimit(stream, { + maxBytes, + label: REQUEST_BODY_LABEL, + }) + return JSON.parse(new TextDecoder().decode(buffer)) +} + export async function parseJsonBody( request: Request, - invalidJson: ParseRequestOptions['invalidJson'] = 'response' + invalidJson: ParseRequestOptions['invalidJson'] = 'response', + maxBytes: number = DEFAULT_MAX_JSON_BODY_BYTES ): Promise< - { success: true; data: unknown } | { success: false; response: NextResponse<{ error: string }> } + | { success: true; data: unknown } + | { + success: false + reason: 'too_large' | 'invalid_json' + response: NextResponse<{ error: string }> + } > { try { - return { success: true, data: await request.json() } + return { success: true, data: await readJsonBodyWithLimit(request, maxBytes) } } catch (error) { if (invalidJson === 'throw') throw error + if (isPayloadSizeLimitError(error)) { + return { + success: false, + reason: 'too_large', + response: NextResponse.json( + { error: `Request body exceeds the maximum allowed size of ${maxBytes} bytes` }, + { status: 413 } + ), + } + } return { success: false, + reason: 'invalid_json', response: NextResponse.json({ error: 'Request body must be valid JSON' }, { status: 400 }), } } @@ -133,9 +199,9 @@ export async function parseRequest( let body: unknown if (shouldReadJsonBody(contract)) { - const parsedBody = await parseJsonBody(request, options?.invalidJson) + const parsedBody = await parseJsonBody(request, options?.invalidJson, options?.maxBodyBytes) if (!parsedBody.success) { - return options?.invalidJsonResponse + return options?.invalidJsonResponse && parsedBody.reason === 'invalid_json' ? { success: false, response: options.invalidJsonResponse() } : parsedBody } diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 7a65dfa10eb..5037e795657 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -7,7 +7,7 @@ import * as schema from '@sim/db/schema' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { betterAuth } from 'better-auth' +import { betterAuth, type User } from 'better-auth' import { drizzleAdapter } from 'better-auth/adapters/drizzle' import { APIError, createAuthMiddleware } from 'better-auth/api' import { nextCookies } from 'better-auth/next-js' @@ -26,6 +26,7 @@ import { headers } from 'next/headers' import Stripe from 'stripe' import { getEmailSubject, + renderExistingAccountEmail, renderOTPEmail, renderPasswordResetEmail, renderWelcomeEmail, @@ -756,6 +757,65 @@ export const auth = betterAuth({ emailAndPassword: { enabled: true, requireEmailVerification: isEmailVerificationEnabled, + /** + * When someone signs up with an already-registered email, better-auth returns a + * generic success response (OWASP enumeration protection) instead of leaking that + * the account exists. This callback notifies the real account owner out-of-band, + * mirroring the privacy-preserving forget-password flow. Errors are swallowed so the + * response is indistinguishable from a genuine new sign-up. + */ + onExistingUserSignUp: async ({ user }: { user: User }) => { + try { + const html = await renderExistingAccountEmail(user.name || '') + const result = await sendEmail({ + to: user.email, + subject: getEmailSubject('existing-account'), + html, + from: getFromEmailAddress(), + emailType: 'transactional', + }) + if (!result.success) { + logger.warn('[onExistingUserSignUp] Failed to send existing-account email', { + message: result.message, + }) + } + } catch (error) { + logger.error('[onExistingUserSignUp] Error sending existing-account email', { error }) + } + }, + /** + * The synthetic user returned for the generic duplicate-sign-up response must carry + * the exact same set of returned fields a real freshly-created user would, otherwise + * the differing response shape re-opens the enumeration oracle. The admin plugin + * (always loaded) adds role/banned/banReason/banExpires, and the Stripe plugin — loaded + * only when billing is enabled — adds stripeCustomerId (null on a new user). The + * harmony plugin's normalizedEmail is `returned: false`, so it is intentionally omitted. + */ + customSyntheticUser: ({ + coreFields, + additionalFields, + id, + }: { + coreFields: { + name: string + email: string + emailVerified: boolean + image: string | null + createdAt: Date + updatedAt: Date + } + additionalFields: Record + id: string + }) => ({ + ...coreFields, + role: 'user', + banned: false, + banReason: null, + banExpires: null, + ...(isBillingEnabled && stripeClient ? { stripeCustomerId: null } : {}), + ...additionalFields, + id, + }), sendResetPassword: async ({ user, url, token }, request) => { const username = user.name || '' @@ -849,22 +909,6 @@ export const auth = betterAuth({ } } - if (ctx.path === '/sign-up/email' && ctx.body?.email) { - const signupEmail = ctx.body.email.toLowerCase() - const [existingUser] = await db - .select({ id: schema.user.id }) - .from(schema.user) - .where(eq(schema.user.email, signupEmail)) - .limit(1) - - if (existingUser) { - throw new APIError('UNPROCESSABLE_ENTITY', { - message: 'User already exists', - code: 'USER_ALREADY_EXISTS', - }) - } - } - return }), }, diff --git a/apps/sim/lib/billing/calculations/usage-reservation.test.ts b/apps/sim/lib/billing/calculations/usage-reservation.test.ts new file mode 100644 index 00000000000..a6435aff69c --- /dev/null +++ b/apps/sim/lib/billing/calculations/usage-reservation.test.ts @@ -0,0 +1,152 @@ +/** + * @vitest-environment node + */ +import { redisConfigMock, redisConfigMockFns } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockFlags } = vi.hoisted(() => ({ + mockFlags: { isBillingEnabled: true }, +})) + +vi.mock('@/lib/core/config/feature-flags', () => ({ + get isBillingEnabled() { + return mockFlags.isBillingEnabled + }, + isHosted: true, +})) + +vi.mock('@/lib/core/config/redis', () => redisConfigMock) + +import { + releaseExecutionSlot, + reserveExecutionSlot, + resolveBillingEntityKey, +} from '@/lib/billing/calculations/usage-reservation' + +const evalMock = vi.fn() +const getdelMock = vi.fn() +const zremMock = vi.fn() +const fakeRedis = { eval: evalMock, getdel: getdelMock, zrem: zremMock } + +const baseParams = { + userId: 'user-1', + executionId: 'exec-1', + subscription: { plan: 'free' as const, referenceId: 'user-1' }, + currentUsage: 0, + limit: 5, +} + +describe('usage-reservation', () => { + beforeEach(() => { + vi.clearAllMocks() + mockFlags.isBillingEnabled = true + redisConfigMockFns.mockGetRedisClient.mockReturnValue(fakeRedis) + }) + + describe('resolveBillingEntityKey', () => { + it('keys personal subscriptions by user', () => { + expect(resolveBillingEntityKey('user-1', { referenceId: 'user-1' })).toBe('user:user-1') + }) + + it('keys org-scoped subscriptions by organization', () => { + expect(resolveBillingEntityKey('user-1', { referenceId: 'org-9' })).toBe('org:org-9') + }) + }) + + describe('reserveExecutionSlot', () => { + it('admits when the reservation script returns 1', async () => { + evalMock.mockResolvedValueOnce(1) + const result = await reserveExecutionSlot(baseParams) + expect(result.reserved).toBe(true) + expect(evalMock).toHaveBeenCalledTimes(1) + }) + + it('rejects when the reservation script returns 0 (slots full)', async () => { + evalMock.mockResolvedValueOnce(0) + const result = await reserveExecutionSlot(baseParams) + expect(result.reserved).toBe(false) + }) + + it('passes the free-tier concurrency cap and headroom slots to the script', async () => { + evalMock.mockResolvedValueOnce(1) + await reserveExecutionSlot(baseParams) + const args = evalMock.mock.calls[0] + // eval(script, numKeys, inflightKey, pointerKey, now, expiry, maxConc, headroomSlots, member, entityKey, pttl) + expect(args[2]).toBe('usage:inflight:user:user-1') + expect(args[3]).toBe('usage:reservation:exec-1') + expect(args[6]).toBe('15') + expect(args[7]).toBe('1000') + expect(args[8]).toBe('exec-1') + expect(args[9]).toBe('user:user-1') + }) + + it('reserves against the org entity for org-scoped subscriptions', async () => { + evalMock.mockResolvedValueOnce(1) + await reserveExecutionSlot({ + ...baseParams, + subscription: { plan: 'team', referenceId: 'org-9' }, + }) + const args = evalMock.mock.calls[0] + expect(args[2]).toBe('usage:inflight:org:org-9') + expect(args[6]).toBe('150') + }) + + it('clamps negative headroom to zero slots', async () => { + evalMock.mockResolvedValueOnce(0) + await reserveExecutionSlot({ ...baseParams, currentUsage: 10, limit: 5 }) + expect(evalMock.mock.calls[0][7]).toBe('0') + }) + + it('fails open (admits) when billing enforcement is disabled', async () => { + mockFlags.isBillingEnabled = false + const result = await reserveExecutionSlot(baseParams) + expect(result.reserved).toBe(true) + expect(evalMock).not.toHaveBeenCalled() + }) + + it('fails open (admits) when Redis is unavailable', async () => { + redisConfigMockFns.mockGetRedisClient.mockReturnValue(null) + const result = await reserveExecutionSlot(baseParams) + expect(result.reserved).toBe(true) + expect(evalMock).not.toHaveBeenCalled() + }) + + it('fails open (admits) when the reservation script throws', async () => { + evalMock.mockRejectedValueOnce(new Error('connection lost')) + const result = await reserveExecutionSlot(baseParams) + expect(result.reserved).toBe(true) + }) + }) + + describe('releaseExecutionSlot', () => { + it('reads the pointer then removes the slot from that entity set', async () => { + getdelMock.mockResolvedValueOnce('org:org-9') + await releaseExecutionSlot('exec-1') + expect(getdelMock).toHaveBeenCalledWith('usage:reservation:exec-1') + expect(zremMock).toHaveBeenCalledWith('usage:inflight:org:org-9', 'exec-1') + }) + + it('does not touch the in-flight set when the pointer is already gone', async () => { + getdelMock.mockResolvedValueOnce(null) + await releaseExecutionSlot('exec-1') + expect(zremMock).not.toHaveBeenCalled() + }) + + it('uses only single-key commands (cluster-safe; no key built inside Lua)', async () => { + getdelMock.mockResolvedValueOnce('user:user-1') + await releaseExecutionSlot('exec-1') + expect(evalMock).not.toHaveBeenCalled() + }) + + it('is a no-op when billing enforcement is disabled', async () => { + mockFlags.isBillingEnabled = false + await releaseExecutionSlot('exec-1') + expect(getdelMock).not.toHaveBeenCalled() + }) + + it('swallows release errors', async () => { + getdelMock.mockRejectedValueOnce(new Error('boom')) + await expect(releaseExecutionSlot('exec-1')).resolves.toBeUndefined() + }) + }) +}) diff --git a/apps/sim/lib/billing/calculations/usage-reservation.ts b/apps/sim/lib/billing/calculations/usage-reservation.ts new file mode 100644 index 00000000000..c448c88776d --- /dev/null +++ b/apps/sim/lib/billing/calculations/usage-reservation.ts @@ -0,0 +1,210 @@ +import { createLogger } from '@sim/logger' +import { toError } from '@sim/utils/errors' +import { BASE_EXECUTION_CHARGE } from '@/lib/billing/constants' +import { getPlanTypeForLimits } from '@/lib/billing/plan-helpers' +import { isOrgScopedSubscription } from '@/lib/billing/subscriptions/utils' +import { isBillingEnabled } from '@/lib/core/config/feature-flags' +import { getRedisClient } from '@/lib/core/config/redis' +import { getMaxExecutionTimeout } from '@/lib/core/execution-limits' +import type { SubscriptionPlan } from '@/lib/core/rate-limiter/types' + +const logger = createLogger('UsageReservation') + +/** + * Maximum number of simultaneously in-flight (admitted but not-yet-costed) + * executions a single billing entity may hold at once. + * + * The usage-cap admission gate reads already-recorded cost, but cost is only + * written when an execution finishes. Without a reservation, N parallel + * executions all read the same pre-burst usage, all pass the cap, and all run — + * collectively spending far past the cap before any cost lands in the ledger + * (free-tier abuse / hard-cap defeat). Bounding the number of in-flight + * executions per billing entity bounds the worst-case overshoot to roughly this + * many executions' worth of spend. + */ +const MAX_CONCURRENT_EXECUTIONS: Record = { + free: 15, + pro: 75, + team: 150, + enterprise: 300, +} + +/** + * Per-slot reserved cost estimate (dollars). The guaranteed-minimum charge + * every execution incurs, used to taper admission as recorded usage approaches + * the cap: an entity may hold at most `floor(headroom / estimate)` concurrent + * slots, keeping `recordedUsage + reservedSlots * estimate <= limit`. A lone + * execution is never blocked on headroom alone — the recorded-usage gate + * (`isExceeded`) governs the single-execution case, so the only residual + * overshoot is the one already inherent to admission (cost is unknown until the + * execution finishes). + */ +const SLOT_COST_ESTIMATE = BASE_EXECUTION_CHARGE + +/** Safety buffer added to the reservation TTL beyond the max execution timeout. */ +const RESERVATION_TTL_BUFFER_MS = 60_000 + +const INFLIGHT_KEY_PREFIX = 'usage:inflight:' +const POINTER_KEY_PREFIX = 'usage:reservation:' + +/** + * Atomically admit an execution only when both the per-entity concurrency cap + * and the remaining usage headroom permit it, then record the in-flight slot. + * + * Prune expired members (crash safety) -> `count = ZCARD` -> reject when + * `count >= min(maxConcurrency, max(1, headroomSlots))` -> otherwise `ZADD` the + * slot, refresh the set TTL, and write the per-execution pointer for release. + * The `max(1, ...)` floor guarantees a lone execution is never blocked on + * headroom alone; concurrency above the first slot still tapers with headroom. + */ +const RESERVE_SCRIPT = ` +local now = tonumber(ARGV[1]) +local expiryScore = tonumber(ARGV[2]) +local maxConcurrency = tonumber(ARGV[3]) +local headroomSlots = tonumber(ARGV[4]) +local pttl = tonumber(ARGV[7]) +redis.call('ZREMRANGEBYSCORE', KEYS[1], '-inf', now) +local count = redis.call('ZCARD', KEYS[1]) +if headroomSlots < 1 then headroomSlots = 1 end +local allowed = maxConcurrency +if headroomSlots < allowed then allowed = headroomSlots end +if count >= allowed then + return 0 +end +redis.call('ZADD', KEYS[1], expiryScore, ARGV[5]) +redis.call('PEXPIRE', KEYS[1], pttl) +redis.call('SET', KEYS[2], ARGV[6], 'PX', pttl) +return 1 +` + +/** + * Stable per-entity reservation key. Org-scoped subscriptions reserve against + * the organization's pooled cap; everyone else against their personal cap — + * mirroring the entity the usage limit itself is enforced on. + */ +export function resolveBillingEntityKey( + userId: string, + subscription: { referenceId?: string | null } | null | undefined +): string { + if (isOrgScopedSubscription(subscription, userId) && subscription?.referenceId) { + return `org:${subscription.referenceId}` + } + return `user:${userId}` +} + +function getMaxConcurrentExecutions(plan: string | null | undefined): number { + return MAX_CONCURRENT_EXECUTIONS[getPlanTypeForLimits(plan) as SubscriptionPlan] +} + +export interface ReserveExecutionSlotParams { + userId: string + executionId: string + subscription: { plan?: string | null; referenceId?: string | null } | null | undefined + /** Recorded usage for the billing entity at admission time (dollars). */ + currentUsage: number + /** The entity's usage cap (dollars). */ + limit: number +} + +export interface ReserveExecutionSlotResult { + reserved: boolean +} + +/** + * Atomic admission reservation that closes the usage-cap check-then-use race. + * + * No-ops (admits) when billing enforcement is off or Redis is unavailable — + * the caller's recorded-usage check still runs in those cases, and failing open + * here matches the rate limiter rather than turning a Redis blip into a full + * execution outage. + */ +export async function reserveExecutionSlot( + params: ReserveExecutionSlotParams +): Promise { + if (!isBillingEnabled) { + return { reserved: true } + } + + const redis = getRedisClient() + if (!redis) { + return { reserved: true } + } + + const { userId, executionId, subscription, currentUsage, limit } = params + const entityKey = resolveBillingEntityKey(userId, subscription) + const maxConcurrency = getMaxConcurrentExecutions(subscription?.plan) + const headroom = Math.max(0, limit - currentUsage) + const headroomSlots = Math.floor(headroom / SLOT_COST_ESTIMATE) + const ttlMs = getMaxExecutionTimeout() + RESERVATION_TTL_BUFFER_MS + const now = Date.now() + const expiryScore = now + ttlMs + + try { + const result = await redis.eval( + RESERVE_SCRIPT, + 2, + `${INFLIGHT_KEY_PREFIX}${entityKey}`, + `${POINTER_KEY_PREFIX}${executionId}`, + now.toString(), + expiryScore.toString(), + maxConcurrency.toString(), + headroomSlots.toString(), + executionId, + entityKey, + ttlMs.toString() + ) + + const reserved = result === 1 + if (!reserved) { + logger.warn('Execution admission throttled — concurrency/usage reservation full', { + entityKey, + executionId, + maxConcurrency, + headroomSlots, + }) + } + return { reserved } + } catch (error) { + logger.error('Usage reservation error — failing open (admitting execution)', { + error: toError(error).message, + entityKey, + executionId, + }) + return { reserved: true } + } +} + +/** + * Release the in-flight reservation held for an execution. Best-effort and + * idempotent — safe to call for executions that never reserved (Redis down, + * billing disabled) or are released more than once. Must NOT be called for a + * paused execution that may still resume. + * + * Uses discrete single-key commands rather than a Lua script that rebuilds the + * in-flight key from the pointer value: the entity that owns the slot is only + * known after reading the pointer, and constructing a key inside Lua bypasses + * the `KEYS` declaration that Redis Cluster relies on for slot routing. + */ +export async function releaseExecutionSlot(executionId: string): Promise { + if (!isBillingEnabled) { + return + } + + const redis = getRedisClient() + if (!redis) { + return + } + + try { + const pointerKey = `${POINTER_KEY_PREFIX}${executionId}` + const entityKey = await redis.getdel(pointerKey) + if (entityKey) { + await redis.zrem(`${INFLIGHT_KEY_PREFIX}${entityKey}`, executionId) + } + } catch (error) { + logger.warn('Failed to release usage reservation', { + error: toError(error).message, + executionId, + }) + } +} diff --git a/apps/sim/lib/core/config/env.ts b/apps/sim/lib/core/config/env.ts index a4fd479f447..87af1a75f35 100644 --- a/apps/sim/lib/core/config/env.ts +++ b/apps/sim/lib/core/config/env.ts @@ -243,6 +243,8 @@ export const env = createEnv({ // Admission & Burst Protection ADMISSION_GATE_MAX_INFLIGHT: z.string().optional().default('500'), // Max concurrent in-flight execution requests per pod + API_MAX_JSON_BODY_BYTES: z.string().optional().default('52428800'),// Default max JSON request body size for contract routes (50 MB) + CHAT_MAX_REQUEST_BYTES: z.string().optional().default('230686720'),// Max request body size for the public deployed-chat endpoint (220 MB; covers 15 base64 file attachments) // Rate Limiting Configuration RATE_LIMIT_WINDOW_MS: z.string().optional().default('60000'), // Rate limit window duration in milliseconds (default: 1 minute) diff --git a/apps/sim/lib/execution/preprocessing.ts b/apps/sim/lib/execution/preprocessing.ts index 833d7b9ab17..0ff0d9eba5d 100644 --- a/apps/sim/lib/execution/preprocessing.ts +++ b/apps/sim/lib/execution/preprocessing.ts @@ -5,6 +5,7 @@ import { checkOrgMemberUsageLimit, checkServerSideUsageLimits, } from '@/lib/billing/calculations/usage-monitor' +import { reserveExecutionSlot } from '@/lib/billing/calculations/usage-reservation' import type { HighestPrioritySubscription } from '@/lib/billing/core/plan' import { getHighestPrioritySubscription } from '@/lib/billing/core/subscription' import { @@ -38,6 +39,14 @@ export interface PreprocessExecutionOptions { checkRateLimit?: boolean // Default: false for manual/chat, true for others checkDeployment?: boolean // Default: true for non-manual triggers skipUsageLimits?: boolean // Default: false (only use for test mode) + /** + * Skip the atomic in-flight concurrency reservation while still enforcing the + * usage-cost cap. Default: false. Set by surfaces that already bound and pace + * their own fan-out (e.g. table-cell dispatch, which is row-bounded, async + * rate-limited, and surfaces a graceful "wait/upgrade" state) so the + * reservation's 429 can't surface as a hard error there. + */ + skipConcurrencyReservation?: boolean logPreprocessingErrors?: boolean // Default: true. When false, skip writing workflow_execution_logs error rows (caller surfaces failures itself, e.g. table cells) // Context information @@ -93,6 +102,7 @@ export async function preprocessExecution( checkRateLimit = triggerType !== 'manual' && triggerType !== 'chat', checkDeployment = triggerType !== 'manual', skipUsageLimits = false, + skipConcurrencyReservation = false, logPreprocessingErrors = true, workspaceId: providedWorkspaceId, loggingSession: providedLoggingSession, @@ -315,9 +325,12 @@ export async function preprocessExecution( const userSubscription = await getHighestPrioritySubscription(actorUserId) // ========== STEP 5: Check Usage Limits ========== + // Snapshot reused by the STEP 7 admission reservation. + let usageSnapshot: { currentUsage: number; limit: number } | null = null if (!skipUsageLimits) { try { const usageCheck = await checkServerSideUsageLimits(actorUserId, userSubscription) + usageSnapshot = { currentUsage: usageCheck.currentUsage, limit: usageCheck.limit } if (usageCheck.isExceeded) { logger.warn( `[${requestId}] User ${actorUserId} has exceeded usage limits. Blocking execution.`, @@ -496,6 +509,62 @@ export async function preprocessExecution( } } + /** + * STEP 7: Atomic admission reservation. Cost is only recorded once an + * execution finishes, so without this a burst of concurrent executions all + * observe the same pre-burst usage and all pass the gate above. Reserving + * bounds in-flight (un-costed) executions per billing entity. Done last so an + * earlier rejection never leaves a slot held; the slot is released at + * execution completion (see {@link LoggingSession}). + */ + if (!skipUsageLimits && !skipConcurrencyReservation && usageSnapshot) { + try { + const { reserved } = await reserveExecutionSlot({ + userId: actorUserId, + executionId, + subscription: userSubscription, + currentUsage: usageSnapshot.currentUsage, + limit: usageSnapshot.limit, + }) + + if (!reserved) { + logger.warn(`[${requestId}] Admission reservation full for user ${actorUserId}`, { + workflowId, + triggerType, + }) + + await recordPreprocessingError({ + workflowId, + executionId, + triggerType, + requestId, + userId: actorUserId, + workspaceId, + errorMessage: + 'Too many concurrent executions in flight for this account. Please wait for in-progress runs to finish and try again.', + loggingSession: providedLoggingSession, + triggerData, + }) + + return { + success: false, + error: { + message: + 'Too many concurrent executions in flight. Please wait for in-progress runs to finish and try again.', + statusCode: 429, + logCreated: true, + retryable: true, + }, + } + } + } catch (error) { + logger.error(`[${requestId}] Unexpected error reserving admission slot`, { + error, + actorUserId, + }) + } + } + // ========== SUCCESS: All Checks Passed ========== logger.info(`[${requestId}] All preprocessing checks passed`, { workflowId, diff --git a/apps/sim/lib/logs/execution/logging-session.ts b/apps/sim/lib/logs/execution/logging-session.ts index a0fd011dc7d..a63aa3fb309 100644 --- a/apps/sim/lib/logs/execution/logging-session.ts +++ b/apps/sim/lib/logs/execution/logging-session.ts @@ -3,6 +3,7 @@ import { workflowExecutionLogs } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { describeError, toError } from '@sim/utils/errors' import { and, eq, sql } from 'drizzle-orm' +import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation' import { isRetryableInfrastructureError } from '@/lib/core/errors/retryable-infrastructure' import { executionLogger } from '@/lib/logs/execution/logger' import { @@ -266,6 +267,18 @@ export class LoggingSession { level: params.level, status: params.status, }) + + // Release the admission reservation from preprocessing. Skipped on pause: a + // paused execution keeps its slot until it terminates (or the TTL expires). + if (params.finalizationPath !== 'paused') { + try { + await releaseExecutionSlot(this.executionId) + } catch (error) { + logger.warn(`Failed to release admission reservation for ${this.executionId}:`, { + error: toError(error).message, + }) + } + } } async onBlockComplete( diff --git a/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts b/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts index f5bc7fa668a..9dc16615a0e 100644 --- a/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts +++ b/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts @@ -54,6 +54,33 @@ export interface PerformUpdateFolderResult { folder?: typeof workflowFolder.$inferSelect } +/** + * Verifies that a prospective parent folder exists, belongs to the target + * workspace, and is not archived. Mirrors the validation in the duplicate + * route's `assertTargetParentFolderMutable` so a caller cannot reparent a + * folder to a non-existent id or to a folder in another workspace. Returns + * an error result when invalid, or `null` when the parent is acceptable. + */ +async function assertParentFolderInWorkspace( + parentId: string, + workspaceId: string +): Promise<{ error: string; errorCode: OrchestrationErrorCode } | null> { + const [parent] = await db + .select({ + workspaceId: workflowFolder.workspaceId, + archivedAt: workflowFolder.archivedAt, + }) + .from(workflowFolder) + .where(eq(workflowFolder.id, parentId)) + .limit(1) + + if (!parent || parent.workspaceId !== workspaceId || parent.archivedAt) { + return { error: 'Parent folder not found', errorCode: 'validation' } + } + + return null +} + async function nextFolderSortOrder( workspaceId: string, parentId: string | null | undefined @@ -93,6 +120,19 @@ export async function performCreateFolder( try { const folderId = params.id || generateId() const parentId = params.parentId || null + + if (parentId) { + if (parentId === folderId) { + return { + success: false, + error: 'Folder cannot be its own parent', + errorCode: 'validation', + } + } + const parentError = await assertParentFolderInWorkspace(parentId, params.workspaceId) + if (parentError) return { success: false, ...parentError } + } + const sortOrder = params.sortOrder !== undefined ? params.sortOrder @@ -146,6 +186,9 @@ export async function performUpdateFolder( } if (params.parentId) { + const parentError = await assertParentFolderInWorkspace(params.parentId, params.workspaceId) + if (parentError) return { success: false, ...parentError } + const wouldCreateCycle = await checkForCircularReference(params.folderId, params.parentId) if (wouldCreateCycle) { return { diff --git a/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts b/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts index 7be2d049711..5f5524f2846 100644 --- a/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts +++ b/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts @@ -4,6 +4,7 @@ import { workflow, workflowFolder } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' +import { isFolderInWorkspace } from '@sim/workflow-authz' import { and, eq, isNull, min, ne } from 'drizzle-orm' import { generateRequestId } from '@/lib/core/utils/request' import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults' @@ -182,6 +183,10 @@ export async function performCreateWorkflow( const folderId = params.folderId || null try { + if (!(await isFolderInWorkspace(folderId, params.workspaceId))) { + return { success: false, error: 'Target folder not found', errorCode: 'validation' } + } + const name = params.deduplicate ? await deduplicateWorkflowName(params.name, params.workspaceId, folderId) : params.name @@ -278,6 +283,13 @@ export async function performUpdateWorkflow( const targetFolderId = params.folderId !== undefined ? params.folderId || null : params.currentFolderId || null + if ( + params.folderId !== undefined && + !(await isFolderInWorkspace(targetFolderId, params.workspaceId)) + ) { + return { success: false, error: 'Target folder not found', errorCode: 'validation' } + } + if (params.name !== undefined || params.folderId !== undefined) { const duplicate = await workflowNameExistsInFolder({ workspaceId: params.workspaceId, diff --git a/apps/sim/tools/agiloft/attachment_info.ts b/apps/sim/tools/agiloft/attachment_info.ts index 07986303fe8..549bbd665a8 100644 --- a/apps/sim/tools/agiloft/attachment_info.ts +++ b/apps/sim/tools/agiloft/attachment_info.ts @@ -2,7 +2,8 @@ import type { AgiloftAttachmentInfoParams, AgiloftAttachmentInfoResponse, } from '@/tools/agiloft/types' -import { buildAttachmentInfoUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildAttachmentInfoUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftAttachmentInfoTool: ToolConfig< diff --git a/apps/sim/tools/agiloft/create_record.ts b/apps/sim/tools/agiloft/create_record.ts index f4763f55bad..c9b852659b9 100644 --- a/apps/sim/tools/agiloft/create_record.ts +++ b/apps/sim/tools/agiloft/create_record.ts @@ -1,5 +1,6 @@ import type { AgiloftCreateRecordParams, AgiloftRecordResponse } from '@/tools/agiloft/types' -import { buildCreateRecordUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildCreateRecordUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftCreateRecordTool: ToolConfig = diff --git a/apps/sim/tools/agiloft/delete_record.ts b/apps/sim/tools/agiloft/delete_record.ts index 42538104961..3a6744b4f1e 100644 --- a/apps/sim/tools/agiloft/delete_record.ts +++ b/apps/sim/tools/agiloft/delete_record.ts @@ -1,5 +1,6 @@ import type { AgiloftDeleteRecordParams, AgiloftDeleteResponse } from '@/tools/agiloft/types' -import { buildDeleteRecordUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildDeleteRecordUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftDeleteRecordTool: ToolConfig = diff --git a/apps/sim/tools/agiloft/get_choice_line_id.ts b/apps/sim/tools/agiloft/get_choice_line_id.ts index 11df1040565..596cd8898a9 100644 --- a/apps/sim/tools/agiloft/get_choice_line_id.ts +++ b/apps/sim/tools/agiloft/get_choice_line_id.ts @@ -2,7 +2,8 @@ import type { AgiloftGetChoiceLineIdParams, AgiloftGetChoiceLineIdResponse, } from '@/tools/agiloft/types' -import { buildGetChoiceLineIdUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildGetChoiceLineIdUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftGetChoiceLineIdTool: ToolConfig< diff --git a/apps/sim/tools/agiloft/lock_record.ts b/apps/sim/tools/agiloft/lock_record.ts index d79777b1962..4c46d9cb758 100644 --- a/apps/sim/tools/agiloft/lock_record.ts +++ b/apps/sim/tools/agiloft/lock_record.ts @@ -1,5 +1,6 @@ import type { AgiloftLockRecordParams, AgiloftLockResponse } from '@/tools/agiloft/types' -import { buildLockRecordUrl, executeAgiloftRequest, getLockHttpMethod } from '@/tools/agiloft/utils' +import { buildLockRecordUrl, getLockHttpMethod } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftLockRecordTool: ToolConfig = { diff --git a/apps/sim/tools/agiloft/read_record.ts b/apps/sim/tools/agiloft/read_record.ts index 70b015c43bf..ce59238e1f4 100644 --- a/apps/sim/tools/agiloft/read_record.ts +++ b/apps/sim/tools/agiloft/read_record.ts @@ -1,5 +1,6 @@ import type { AgiloftReadRecordParams, AgiloftRecordResponse } from '@/tools/agiloft/types' -import { buildReadRecordUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildReadRecordUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftReadRecordTool: ToolConfig = { diff --git a/apps/sim/tools/agiloft/remove_attachment.ts b/apps/sim/tools/agiloft/remove_attachment.ts index 7e9a9d6f2d4..7f90e8d6c87 100644 --- a/apps/sim/tools/agiloft/remove_attachment.ts +++ b/apps/sim/tools/agiloft/remove_attachment.ts @@ -2,7 +2,8 @@ import type { AgiloftRemoveAttachmentParams, AgiloftRemoveAttachmentResponse, } from '@/tools/agiloft/types' -import { buildRemoveAttachmentUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildRemoveAttachmentUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftRemoveAttachmentTool: ToolConfig< diff --git a/apps/sim/tools/agiloft/saved_search.ts b/apps/sim/tools/agiloft/saved_search.ts index 8d645d871d0..86232d88220 100644 --- a/apps/sim/tools/agiloft/saved_search.ts +++ b/apps/sim/tools/agiloft/saved_search.ts @@ -1,5 +1,6 @@ import type { AgiloftSavedSearchParams, AgiloftSavedSearchResponse } from '@/tools/agiloft/types' -import { buildSavedSearchUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildSavedSearchUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftSavedSearchTool: ToolConfig< diff --git a/apps/sim/tools/agiloft/search_records.ts b/apps/sim/tools/agiloft/search_records.ts index b05465c0be5..352124787f9 100644 --- a/apps/sim/tools/agiloft/search_records.ts +++ b/apps/sim/tools/agiloft/search_records.ts @@ -1,5 +1,6 @@ import type { AgiloftSearchRecordsParams, AgiloftSearchResponse } from '@/tools/agiloft/types' -import { buildSearchRecordsUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildSearchRecordsUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftSearchRecordsTool: ToolConfig< diff --git a/apps/sim/tools/agiloft/select_records.ts b/apps/sim/tools/agiloft/select_records.ts index de4be3139cb..5878551d44b 100644 --- a/apps/sim/tools/agiloft/select_records.ts +++ b/apps/sim/tools/agiloft/select_records.ts @@ -1,5 +1,6 @@ import type { AgiloftSelectRecordsParams, AgiloftSelectResponse } from '@/tools/agiloft/types' -import { buildSelectRecordsUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildSelectRecordsUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftSelectRecordsTool: ToolConfig< diff --git a/apps/sim/tools/agiloft/update_record.ts b/apps/sim/tools/agiloft/update_record.ts index 661be1b3a8a..a264b272906 100644 --- a/apps/sim/tools/agiloft/update_record.ts +++ b/apps/sim/tools/agiloft/update_record.ts @@ -1,5 +1,6 @@ import type { AgiloftRecordResponse, AgiloftUpdateRecordParams } from '@/tools/agiloft/types' -import { buildUpdateRecordUrl, executeAgiloftRequest } from '@/tools/agiloft/utils' +import { buildUpdateRecordUrl } from '@/tools/agiloft/utils' +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' import type { ToolConfig } from '@/tools/types' export const agiloftUpdateRecordTool: ToolConfig = diff --git a/apps/sim/tools/agiloft/utils.test.ts b/apps/sim/tools/agiloft/utils.server.test.ts similarity index 51% rename from apps/sim/tools/agiloft/utils.test.ts rename to apps/sim/tools/agiloft/utils.server.test.ts index b80eb2a33ba..9d207b06276 100644 --- a/apps/sim/tools/agiloft/utils.test.ts +++ b/apps/sim/tools/agiloft/utils.server.test.ts @@ -1,8 +1,19 @@ /** * @vitest-environment node */ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { executeAgiloftRequest } from '@/tools/agiloft/utils' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockValidateUrlWithDNS, mockSecureFetch } = vi.hoisted(() => ({ + mockValidateUrlWithDNS: vi.fn(), + mockSecureFetch: vi.fn(), +})) + +vi.mock('@/lib/core/security/input-validation.server', () => ({ + validateUrlWithDNS: mockValidateUrlWithDNS, + secureFetchWithPinnedIP: mockSecureFetch, +})) + +import { executeAgiloftRequest } from '@/tools/agiloft/utils.server' const baseParams = { instanceUrl: 'https://example.agiloft.com', @@ -12,34 +23,31 @@ const baseParams = { table: 'contracts', } -function mockFetchResponse(body: { ok?: boolean; status?: number; json?: unknown; text?: string }) { +function mockResponse(body: { ok?: boolean; status?: number; json?: unknown; text?: string }) { return { ok: body.ok ?? true, status: body.status ?? 200, statusText: '', - headers: new Headers(), + headers: { get: () => null, getSetCookie: () => [], toRecord: () => ({}) }, + body: null, text: async () => body.text ?? '', json: async () => body.json ?? {}, - } as unknown as Response + arrayBuffer: async () => new ArrayBuffer(0), + } } -const fetchSpy = vi.fn() - beforeEach(() => { - fetchSpy.mockReset() - vi.stubGlobal('fetch', fetchSpy) -}) - -afterEach(() => { - vi.unstubAllGlobals() + mockValidateUrlWithDNS.mockReset() + mockSecureFetch.mockReset() + mockValidateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '203.0.113.10' }) }) describe('executeAgiloftRequest', () => { - it('logs in, runs the operation with the bearer token, then logs out', async () => { - fetchSpy - .mockResolvedValueOnce(mockFetchResponse({ json: { access_token: 'tok-1' } })) - .mockResolvedValueOnce(mockFetchResponse({ json: { id: 42, fields: { name: 'foo' } } })) - .mockResolvedValueOnce(mockFetchResponse({})) + it('resolves DNS once, logs in, runs the operation with the bearer token, then logs out — all pinned', async () => { + mockSecureFetch + .mockResolvedValueOnce(mockResponse({ json: { access_token: 'tok-1' } })) + .mockResolvedValueOnce(mockResponse({ json: { id: 42, fields: { name: 'foo' } } })) + .mockResolvedValueOnce(mockResponse({})) const result = await executeAgiloftRequest( baseParams, @@ -59,24 +67,33 @@ describe('executeAgiloftRequest', () => { expect(result).toEqual({ success: true, output: { id: '42', fields: { name: 'foo' } } }) - const calls = fetchSpy.mock.calls + expect(mockValidateUrlWithDNS).toHaveBeenCalledWith( + 'https://example.agiloft.com', + 'instanceUrl' + ) + + const calls = mockSecureFetch.mock.calls expect(calls).toHaveLength(3) expect(calls[0][0]).toBe( 'https://example.agiloft.com/ewws/EWLogin?$KB=demo&$login=admin&$password=secret' ) expect(calls[1][0]).toBe('https://example.agiloft.com/ewws/REST/demo/contracts/42') - expect(calls[1][1]).toMatchObject({ + expect(calls[2][0]).toBe('https://example.agiloft.com/ewws/EWLogout?$KB=demo') + + for (const call of calls) { + expect(call[1]).toBe('203.0.113.10') + } + expect(calls[1][2]).toMatchObject({ method: 'GET', headers: { Accept: 'application/json', Authorization: 'Bearer tok-1' }, }) - expect(calls[2][0]).toBe('https://example.agiloft.com/ewws/EWLogout?$KB=demo') }) - it('still calls logout when the operation throws', async () => { - fetchSpy - .mockResolvedValueOnce(mockFetchResponse({ json: { access_token: 'tok-2' } })) - .mockResolvedValueOnce(mockFetchResponse({ ok: false, status: 500 })) - .mockResolvedValueOnce(mockFetchResponse({})) + it('still logs out when the operation throws', async () => { + mockSecureFetch + .mockResolvedValueOnce(mockResponse({ json: { access_token: 'tok-2' } })) + .mockResolvedValueOnce(mockResponse({ ok: false, status: 500 })) + .mockResolvedValueOnce(mockResponse({})) await expect( executeAgiloftRequest( @@ -89,14 +106,14 @@ describe('executeAgiloftRequest', () => { ) ).rejects.toThrow('operation failed') - expect(fetchSpy).toHaveBeenCalledTimes(3) - expect(fetchSpy.mock.calls[2][0]).toContain('/ewws/EWLogout') + expect(mockSecureFetch).toHaveBeenCalledTimes(3) + expect(mockSecureFetch.mock.calls[2][0]).toContain('/ewws/EWLogout') }) it('swallows logout failures (best-effort)', async () => { - fetchSpy - .mockResolvedValueOnce(mockFetchResponse({ json: { access_token: 'tok-3' } })) - .mockResolvedValueOnce(mockFetchResponse({ json: { ok: true } })) + mockSecureFetch + .mockResolvedValueOnce(mockResponse({ json: { access_token: 'tok-3' } })) + .mockResolvedValueOnce(mockResponse({ json: { ok: true } })) .mockRejectedValueOnce(new Error('logout network error')) const result = await executeAgiloftRequest( @@ -109,7 +126,7 @@ describe('executeAgiloftRequest', () => { }) it('throws when login does not return an access token', async () => { - fetchSpy.mockResolvedValueOnce(mockFetchResponse({ json: {} })) + mockSecureFetch.mockResolvedValueOnce(mockResponse({ json: {} })) await expect( executeAgiloftRequest( @@ -119,18 +136,23 @@ describe('executeAgiloftRequest', () => { ) ).rejects.toThrow('Agiloft login did not return an access token') - expect(fetchSpy).toHaveBeenCalledTimes(1) + expect(mockSecureFetch).toHaveBeenCalledTimes(1) }) - it('rejects an instance URL that fails synchronous URL validation', async () => { + it('rejects an instance URL that resolves to a blocked IP without issuing any request', async () => { + mockValidateUrlWithDNS.mockResolvedValue({ + isValid: false, + error: 'instanceUrl resolves to a blocked IP address', + }) + await expect( executeAgiloftRequest( - { ...baseParams, instanceUrl: 'not-a-valid-url' }, + { ...baseParams, instanceUrl: 'https://internal.attacker.com' }, (base) => ({ url: `${base}/ewws/REST/demo/contracts/42`, method: 'GET' }), async () => ({ success: true, output: {} }) ) - ).rejects.toThrow(/Invalid Agiloft instance URL/) + ).rejects.toThrow(/blocked IP address/) - expect(fetchSpy).not.toHaveBeenCalled() + expect(mockSecureFetch).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/tools/agiloft/utils.server.ts b/apps/sim/tools/agiloft/utils.server.ts index 3aaa0c62b71..4e48db85254 100644 --- a/apps/sim/tools/agiloft/utils.server.ts +++ b/apps/sim/tools/agiloft/utils.server.ts @@ -5,9 +5,17 @@ import { validateUrlWithDNS, } from '@/lib/core/security/input-validation.server' import type { AgiloftBaseParams } from '@/tools/agiloft/types' +import type { HttpMethod, ToolResponse } from '@/tools/types' const logger = createLogger('AgiloftAuthServer') +interface AgiloftRequestConfig { + url: string + method: HttpMethod + headers?: Record + body?: string +} + /** * Validates the Agiloft instance URL and resolves its DNS once, returning the * resolved IP so subsequent requests can pin to it. This prevents DNS-rebinding @@ -76,4 +84,49 @@ export async function agiloftLogoutPinned( } } +/** + * Shared wrapper that handles the full Agiloft auth lifecycle behind the + * codebase's SSRF-safe fetch path. The instance URL is validated and resolved + * to a concrete IP once via `validateUrlWithDNS` (which rejects hostnames that + * resolve to private/reserved addresses), and every hop — login, the operation + * request, and logout — is issued through `secureFetchWithPinnedIP` so the + * connection is pinned to that validated IP. This defeats DNS-rebinding (TOCTOU) + * SSRF where a hostname could resolve to an internal address on a later lookup. + * + * 1. Validate + resolve the instance URL once. + * 2. Login to obtain a Bearer token. + * 3. Execute the operation request with the token. + * 4. Logout to clean up the session (best-effort). + * + * The `buildRequest` callback receives the base URL and returns the request + * config. The `transformResponse` callback converts the raw response into the + * tool's output format. + * + * Server-only — uses node:dns/promises and node:http(s) via the pinned fetch. + */ +export async function executeAgiloftRequest( + params: AgiloftBaseParams, + buildRequest: (base: string) => AgiloftRequestConfig, + transformResponse: (response: SecureFetchResponse) => Promise +): Promise { + const resolvedIP = await resolveAgiloftInstance(params.instanceUrl) + const token = await agiloftLoginPinned(params, resolvedIP) + const base = params.instanceUrl.replace(/\/$/, '') + + try { + const req = buildRequest(base) + const response = await secureFetchWithPinnedIP(req.url, resolvedIP, { + method: req.method, + headers: { + ...req.headers, + Authorization: `Bearer ${token}`, + }, + body: req.body, + }) + return await transformResponse(response) + } finally { + await agiloftLogoutPinned(params.instanceUrl, params.knowledgeBase, token, resolvedIP) + } +} + export type { SecureFetchResponse } diff --git a/apps/sim/tools/agiloft/utils.ts b/apps/sim/tools/agiloft/utils.ts index 811187ab833..34efd86efc9 100644 --- a/apps/sim/tools/agiloft/utils.ts +++ b/apps/sim/tools/agiloft/utils.ts @@ -1,5 +1,3 @@ -import { createLogger } from '@sim/logger' -import { validateExternalUrl } from '@/lib/core/security/input-validation' import type { AgiloftAttachmentInfoParams, AgiloftBaseParams, @@ -13,108 +11,7 @@ import type { AgiloftSearchRecordsParams, AgiloftSelectRecordsParams, } from '@/tools/agiloft/types' -import type { HttpMethod, ToolResponse } from '@/tools/types' - -const logger = createLogger('AgiloftAuth') - -interface AgiloftRequestConfig { - url: string - method: HttpMethod - headers?: Record - body?: BodyInit -} - -/** - * Exchanges login/password for a short-lived Bearer token via EWLogin. - */ -async function agiloftLogin(params: AgiloftBaseParams): Promise { - const base = params.instanceUrl.replace(/\/$/, '') - - const urlValidation = validateExternalUrl(params.instanceUrl, 'instanceUrl') - if (!urlValidation.isValid) { - throw new Error(`Invalid Agiloft instance URL: ${urlValidation.error}`) - } - - const kb = encodeURIComponent(params.knowledgeBase) - const login = encodeURIComponent(params.login) - const password = encodeURIComponent(params.password) - - const url = `${base}/ewws/EWLogin?$KB=${kb}&$login=${login}&$password=${password}` - const response = await fetch(url, { method: 'POST' }) - - if (!response.ok) { - const errorText = await response.text() - throw new Error(`Agiloft login failed: ${response.status} - ${errorText}`) - } - - const data = (await response.json()) as { access_token?: string } - const token = data.access_token - - if (!token) { - throw new Error('Agiloft login did not return an access token') - } - - return token -} - -/** - * Cleans up the server session. Best-effort — failures are logged but not thrown. - */ -async function agiloftLogout( - instanceUrl: string, - knowledgeBase: string, - token: string -): Promise { - try { - const base = instanceUrl.replace(/\/$/, '') - const kb = encodeURIComponent(knowledgeBase) - await fetch(`${base}/ewws/EWLogout?$KB=${kb}`, { - method: 'POST', - headers: { Authorization: `Bearer ${token}` }, - }) - } catch (error) { - logger.warn('Agiloft logout failed (best-effort)', { error }) - } -} - -/** - * Shared wrapper that handles the full auth lifecycle: - * 1. Login to get Bearer token - * 2. Execute the request with the token - * 3. Logout to clean up the session - * - * The `buildRequest` callback receives the token and base URL, and returns - * the request config. The `transformResponse` callback converts the raw - * Response into the tool's output format. - */ -export async function executeAgiloftRequest( - params: AgiloftBaseParams, - buildRequest: (base: string) => AgiloftRequestConfig, - transformResponse: (response: Response) => Promise -): Promise { - const token = await agiloftLogin(params) - const base = params.instanceUrl.replace(/\/$/, '') - - try { - const req = buildRequest(base) - const response = await fetch(req.url, { - method: req.method, - headers: { - ...req.headers, - Authorization: `Bearer ${token}`, - }, - body: req.body, - }) - return await transformResponse(response) - } finally { - await agiloftLogout(params.instanceUrl, params.knowledgeBase, token) - } -} - -/** - * Login helper exported for use in the attach file API route. - */ -export { agiloftLogin, agiloftLogout } +import type { HttpMethod } from '@/tools/types' /** URL builders (credential-free -- auth is via Bearer token header) */ diff --git a/apps/sim/tools/grafana/update_alert_rule.ts b/apps/sim/tools/grafana/update_alert_rule.ts index 19f2bf8164d..ba474e2ed90 100644 --- a/apps/sim/tools/grafana/update_alert_rule.ts +++ b/apps/sim/tools/grafana/update_alert_rule.ts @@ -1,9 +1,11 @@ -import { validateExternalUrl } from '@/lib/core/security/input-validation' +import { + secureFetchWithPinnedIP, + validateUrlWithDNS, +} from '@/lib/core/security/input-validation.server' import { ALERT_RULE_OUTPUT_FIELDS, type GrafanaUpdateAlertRuleParams } from '@/tools/grafana/types' import { mapAlertRule } from '@/tools/grafana/utils' import type { ToolConfig, ToolResponse } from '@/tools/types' -// Using ToolResponse for intermediate state since this tool fetches existing data first export const updateAlertRuleTool: ToolConfig = { id: 'grafana_update_alert_rule', name: 'Grafana Update Alert Rule', @@ -134,7 +136,6 @@ export const updateAlertRuleTool: ToolConfig `${params.baseUrl.replace(/\/$/, '')}/api/v1/provisioning/alert-rules/${params.alertRuleUid}`, method: 'GET', @@ -151,7 +152,6 @@ export const updateAlertRuleTool: ToolConfig { - // Store the existing rule data for postProcess to use const data = await response.json() return { success: true, @@ -162,7 +162,6 @@ export const updateAlertRuleTool: ToolConfig { - // Merge user changes with existing rule and PUT the complete object const existingRule = result.output._existingRule if (!existingRule || !existingRule.uid) { @@ -173,12 +172,10 @@ export const updateAlertRuleTool: ToolConfig = { ...existingRule, } - // Apply user's changes if (params.title) updatedRule.title = params.title if (params.folderUid) updatedRule.folderUID = params.folderUid if (params.ruleGroup) updatedRule.ruleGroup = params.ruleGroup @@ -258,7 +255,6 @@ export const updateAlertRuleTool: ToolConfig = { 'Content-Type': 'application/json', Authorization: `Bearer ${params.apiKey}`, @@ -270,8 +266,9 @@ export const updateAlertRuleTool: ToolConfig = { id: 'grafana_update_dashboard', name: 'Grafana Update Dashboard', @@ -87,7 +89,6 @@ export const updateDashboardTool: ToolConfig `${params.baseUrl.replace(/\/$/, '')}/api/dashboards/uid/${params.dashboardUid}`, method: 'GET', @@ -104,7 +105,6 @@ export const updateDashboardTool: ToolConfig { - // Store the existing dashboard data for postProcess to use const data = await response.json() return { success: true, @@ -116,7 +116,6 @@ export const updateDashboardTool: ToolConfig { - // Merge user changes with existing dashboard and POST the complete object const existingDashboard = result.output._existingDashboard const existingMeta = result.output._existingMeta @@ -128,12 +127,10 @@ export const updateDashboardTool: ToolConfig = { ...existingDashboard, } - // Apply user's changes if (params.title) updatedDashboard.title = params.title if (params.timezone) updatedDashboard.timezone = params.timezone if (params.refresh) updatedDashboard.refresh = params.refresh @@ -148,23 +145,18 @@ export const updateDashboardTool: ToolConfig = { dashboard: updatedDashboard, overwrite: params.overwrite === true, } - // Use existing folder if not specified if (params.folderUid) { body.folderUid = params.folderUid } else if (existingMeta?.folderUid) { @@ -175,7 +167,6 @@ export const updateDashboardTool: ToolConfig = { 'Content-Type': 'application/json', Authorization: `Bearer ${params.apiKey}`, @@ -184,8 +175,9 @@ export const updateDashboardTool: ToolConfig { + if (!folderId) return true + + const [folder] = await db + .select({ + workspaceId: workflowFolder.workspaceId, + archivedAt: workflowFolder.archivedAt, + }) + .from(workflowFolder) + .where(eq(workflowFolder.id, folderId)) + .limit(1) + + return Boolean(folder && folder.workspaceId === workspaceId && !folder.archivedAt) +} + +/** + * Throws {@link FolderNotFoundError} (HTTP 400) when `folderId` does not belong to + * `workspaceId` (or is archived/missing). No-op for a null/undefined folderId. + */ +export async function assertFolderInWorkspace( + folderId: string | null | undefined, + workspaceId: string +): Promise { + if (!(await isFolderInWorkspace(folderId, workspaceId))) { + throw new FolderNotFoundError() + } +} + export interface WorkflowWorkspaceAuthorizationResult { allowed: boolean status: number