From 6d09bddfaf384e702c9b3c3c40f366cf9fc8302e Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:23:12 -0700 Subject: [PATCH 01/36] fix(knowledge): require write access for batch chunk operations The PATCH /api/knowledge/[id]/documents/[documentId]/chunks handler performs enable/disable/delete operations but authorized callers with only read-level access (checkDocumentAccess). This let read-only workspace members destroy or disable indexed chunks. Switch to checkDocumentWriteAccess (write/admin required), matching the sibling POST/PUT/DELETE chunk mutation endpoints. --- .../api/knowledge/[id]/documents/[documentId]/chunks/route.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c977a8c5ed..46aa0f5d3e 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 @@ -274,7 +274,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) { From de605ebd3e0a1b363a067b0d9b5ecea86acb4ab1 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:34:06 -0700 Subject: [PATCH 02/36] fix(env): restrict decrypted workspace env vars to secret admins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/workspaces/:id/environment returned decrypted workspace environment variables to any member, including read-only collaborators, leaking API tokens, database URLs, and other secrets. Mask workspace variable values for non-admin viewers while preserving the variable names, so editor autocomplete and conflict detection keep working. A value is revealed only when the caller is a credential admin of that key, or — for legacy keys with no per-secret ACL — holds workspace admin permission. This mirrors the per-key edit gating already enforced by PUT/DELETE: if you can administer a secret, you can read it. Personal variables and execution-time resolution are unchanged. --- .../workspaces/[id]/environment/route.test.ts | 145 ++++++++++++++++++ .../api/workspaces/[id]/environment/route.ts | 21 ++- 2 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 apps/sim/app/api/workspaces/[id]/environment/route.test.ts 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 0000000000..1e60969c9e --- /dev/null +++ b/apps/sim/app/api/workspaces/[id]/environment/route.test.ts @@ -0,0 +1,145 @@ +/** + * @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) + // Variable names are preserved so editor autocomplete keeps working... + expect(Object.keys(body.data.workspace).sort()).toEqual(['DATABASE_URL', 'OPENAI_API_KEY']) + // ...but plaintext values are withheld. + 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 cfcdf8a77d..d45c8f2465 100644 --- a/apps/sim/app/api/workspaces/[id]/environment/route.ts +++ b/apps/sim/app/api/workspaces/[id]/environment/route.ts @@ -60,10 +60,29 @@ export const GET = withRouteHandler( workspaceId ) + // Plaintext workspace secrets are restricted 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 caller may view a value if they are a credential admin + // of that key, or — for legacy keys predating per-secret ACLs — if they + // hold workspace `admin` permission. This mirrors the per-key edit gating + // in PUT/DELETE: if you can administer a secret, you can read it. + const workspaceKeys = Object.keys(workspaceDecrypted) + const { adminKeys, knownKeys } = await getWorkspaceEnvKeyAdminAccess({ + workspaceId, + envKeys: workspaceKeys, + userId, + }) + const workspaceMasked: Record = {} + for (const key of workspaceKeys) { + const canViewValue = adminKeys.has(key) || (!knownKeys.has(key) && permission === 'admin') + workspaceMasked[key] = canViewValue ? workspaceDecrypted[key] : '' + } + return NextResponse.json( { data: { - workspace: workspaceDecrypted, + workspace: workspaceMasked, personal: personalDecrypted, conflicts, }, From b8c7f3d49662936f6da3e876c04404c7273ada44 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:36:45 -0700 Subject: [PATCH 03/36] fix(files): block cross-tenant deletion via client-controlled context POST /api/files/delete trusted a client-supplied `context`, letting any authenticated user delete another tenant's file by naming an arbitrary key with `context: "og-images"`. verifyFileAccess() short-circuited the three public contexts (profile-pictures, og-images, workspace-logos) to `true` before any ownership/requireWrite check. - Derive the storage context strictly from the trusted key prefix in the delete route; reject a supplied `context` that disagrees with the key. - Gate the public-context short-circuit to reads only. Destructive ops (requireWrite) now prove ownership via verifyPublicAssetWriteAccess: workspace-logos require write/admin on the bound workspace, profile-pictures require an exact owner match, og-images always deny. Reads of public assets are unchanged. --- apps/sim/app/api/files/authorization.test.ts | 72 ++++++++++++++++++-- apps/sim/app/api/files/authorization.ts | 69 ++++++++++++++++++- apps/sim/app/api/files/delete/route.test.ts | 18 +++++ apps/sim/app/api/files/delete/route.ts | 10 ++- 4 files changed, 162 insertions(+), 7 deletions(-) diff --git a/apps/sim/app/api/files/authorization.test.ts b/apps/sim/app/api/files/authorization.test.ts index 463a1bdf0c..5cc58aa848 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,62 @@ 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 () => { + // The reported attack: a workspace file key passed with context og-images. + // og-images has no user-facing delete path, so the destructive op is denied + // without ever short-circuiting. + 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 b2cc73c429..21d76c5eae 100644 --- a/apps/sim/app/api/files/authorization.ts +++ b/apps/sim/app/api/files/authorization.ts @@ -144,12 +144,18 @@ 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. Destructive operations + // (`requireWrite`) must NOT short-circuit — they require proof of ownership + // of the user/workspace the object belongs to, never an unconditional allow. 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 +273,67 @@ 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 + } + 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 eed0774858..9379bbf1a3 100644 --- a/apps/sim/app/api/files/delete/route.test.ts +++ b/apps/sim/app/api/files/delete/route.test.ts @@ -198,4 +198,22 @@ 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 () => { + // Reported attack: a workspace file key passed with a public context to dodge + // the ownership check. The context is derived from the key, so this is denied + // before authorization or deletion ever runs. + 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 c483faa3c6..04a955edbc 100644 --- a/apps/sim/app/api/files/delete/route.ts +++ b/apps/sim/app/api/files/delete/route.ts @@ -62,7 +62,15 @@ export const POST = withRouteHandler(async (request: NextRequest) => { try { const key = extractStorageKeyFromPath(filePath) - const storageContext: StorageContext = context || inferContextFromKey(key) + // The context is derived from the trusted key prefix, never the + // client-supplied `context`. A caller cannot name a workspace key while + // claiming a public context (e.g. `og-images`) to dodge the ownership + // check. If a `context` is supplied it 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 From 3ed97a440b7d2a31645e9bba4861e49acb82dead Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:37:05 -0700 Subject: [PATCH 04/36] fix(telegram): verify X-Telegram-Bot-Api-Secret-Token on inbound webhooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Telegram triggers accepted any forged update from anyone who knew the webhook URL path: verifyAuth was a no-op that always returned null, and setWebhook registered no secret_token. Generate a per-webhook secret in createSubscription, register it with Telegram as secret_token, and persist it to providerConfig. verifyAuth now fails closed — rejects when no token is configured, when the X-Telegram-Bot-Api-Secret-Token header is absent, or when it does not match via constant-time safeCompare. --- .../lib/webhooks/providers/telegram.test.ts | 62 +++++++++++++++++++ apps/sim/lib/webhooks/providers/telegram.ts | 34 ++++++++-- 2 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 apps/sim/lib/webhooks/providers/telegram.test.ts diff --git a/apps/sim/lib/webhooks/providers/telegram.test.ts b/apps/sim/lib/webhooks/providers/telegram.test.ts new file mode 100644 index 0000000000..98c941fa2d --- /dev/null +++ b/apps/sim/lib/webhooks/providers/telegram.test.ts @@ -0,0 +1,62 @@ +import { NextRequest } from 'next/server' +import { describe, expect, it } from 'vitest' +import { telegramHandler } from '@/lib/webhooks/providers/telegram' + +function reqWithHeaders(headers: Record): NextRequest { + return new NextRequest('http://localhost/test', { headers }) +} + +describe('Telegram webhook provider', () => { + it('verifyAuth rejects when secretToken is not configured', () => { + const res = telegramHandler.verifyAuth!({ + request: reqWithHeaders({ 'x-telegram-bot-api-secret-token': 'anything' }), + rawBody: '{}', + requestId: 't1', + providerConfig: {}, + webhook: {}, + workflow: {}, + }) + expect((res as { status?: number })?.status).toBe(401) + }) + + it('verifyAuth rejects when the secret token header is missing', () => { + const res = telegramHandler.verifyAuth!({ + request: reqWithHeaders({}), + rawBody: '{}', + requestId: 't2', + providerConfig: { secretToken: 'super-secret' }, + webhook: {}, + workflow: {}, + }) + expect((res as { status?: number })?.status).toBe(401) + }) + + it('verifyAuth rejects when the secret token does not match', () => { + const res = telegramHandler.verifyAuth!({ + request: reqWithHeaders({ 'x-telegram-bot-api-secret-token': 'wrong' }), + rawBody: '{}', + requestId: 't3', + providerConfig: { secretToken: 'super-secret' }, + webhook: {}, + workflow: {}, + }) + expect((res as { status?: number })?.status).toBe(401) + }) + + it('verifyAuth accepts a matching secret token', () => { + const res = telegramHandler.verifyAuth!({ + request: reqWithHeaders({ 'x-telegram-bot-api-secret-token': 'super-secret' }), + rawBody: '{}', + requestId: 't4', + providerConfig: { secretToken: 'super-secret' }, + webhook: {}, + workflow: {}, + }) + expect(res).toBeNull() + }) + + it('extractIdempotencyId keys on update_id', () => { + expect(telegramHandler.extractIdempotencyId!({ update_id: 42 })).toBe('telegram:42') + expect(telegramHandler.extractIdempotencyId!({})).toBeNull() + }) +}) diff --git a/apps/sim/lib/webhooks/providers/telegram.ts b/apps/sim/lib/webhooks/providers/telegram.ts index 9720bccd60..c74dc4d42c 100644 --- a/apps/sim/lib/webhooks/providers/telegram.ts +++ b/apps/sim/lib/webhooks/providers/telegram.ts @@ -1,6 +1,9 @@ import { db, webhook, workflowDeploymentVersion } from '@sim/db' import { createLogger } from '@sim/logger' +import { safeCompare } from '@sim/security/compare' +import { generateId } from '@sim/utils/id' import { and, eq, isNull, ne } from 'drizzle-orm' +import { NextResponse } from 'next/server' import { getNotificationUrl, getProviderConfig } from '@/lib/webhooks/provider-subscription-utils' import type { AuthContext, @@ -15,13 +18,29 @@ import type { const logger = createLogger('WebhookProvider:Telegram') export const telegramHandler: WebhookProviderHandler = { - verifyAuth({ request, requestId }: AuthContext) { - const userAgent = request.headers.get('user-agent') - if (!userAgent) { + verifyAuth({ request, requestId, providerConfig }: AuthContext): NextResponse | null { + const secretToken = (providerConfig.secretToken as string | undefined)?.trim() + if (!secretToken) { logger.warn( - `[${requestId}] Telegram webhook request has empty User-Agent header. This may be blocked by middleware.` + `[${requestId}] Telegram webhook missing secretToken in providerConfig — rejecting request. Re-save the trigger so a secret token can be registered with Telegram.` ) + return new NextResponse( + 'Unauthorized - Telegram webhook secret token is not configured. Re-save the trigger so a webhook can be registered.', + { status: 401 } + ) + } + + const providedToken = request.headers.get('x-telegram-bot-api-secret-token') + if (!providedToken) { + logger.warn(`[${requestId}] Telegram webhook missing secret token header — rejecting request`) + return new NextResponse('Unauthorized - Missing Telegram secret token', { status: 401 }) } + + if (!safeCompare(providedToken, secretToken)) { + logger.warn(`[${requestId}] Telegram secret token verification failed`) + return new NextResponse('Unauthorized - Invalid Telegram secret token', { status: 401 }) + } + return null }, @@ -125,6 +144,9 @@ export const telegramHandler: WebhookProviderHandler = { const notificationUrl = getNotificationUrl(ctx.webhook) const telegramApiUrl = `https://api.telegram.org/bot${botToken}/setWebhook` + const existingSecretToken = (config.secretToken as string | undefined)?.trim() + const secretToken = existingSecretToken || generateId() + try { const telegramResponse = await fetch(telegramApiUrl, { method: 'POST', @@ -132,7 +154,7 @@ export const telegramHandler: WebhookProviderHandler = { 'Content-Type': 'application/json', 'User-Agent': 'TelegramBot/1.0', }, - body: JSON.stringify({ url: notificationUrl }), + body: JSON.stringify({ url: notificationUrl, secret_token: secretToken }), }) const responseBody = await telegramResponse.json() @@ -156,7 +178,7 @@ export const telegramHandler: WebhookProviderHandler = { logger.info( `[${ctx.requestId}] Successfully created Telegram webhook for webhook ${ctx.webhook.id}` ) - return {} + return { providerConfigUpdates: { secretToken } } } catch (error: unknown) { if ( error instanceof Error && From ec3e66e894112279b8d585f0b03002ed32d26005 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:38:34 -0700 Subject: [PATCH 05/36] fix(security): pin DNS for Agiloft directExecution and Grafana update tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Agiloft directExecution tools (read/create/search/update/delete/lock/ saved_search/select/get_choice_line_id/remove_attachment/attachment_info) and the Grafana update_dashboard/update_alert_rule postProcess hooks issued outbound HTTP to a fully user-controlled host (instanceUrl/baseUrl) via the global fetch(), guarded only by the synchronous validateExternalUrl() — which never resolves DNS, so a hostname resolving to an internal/reserved IP passed validation (SSRF). Route all of these through the codebase's standard SSRF-safe path: - Agiloft: moved executeAgiloftRequest into utils.server.ts where the existing pinned helpers live. It now resolves+validates the instance URL once and pins every hop (login, operation, logout) to that IP via secureFetchWithPinnedIP. The 11 tool configs now import it from utils.server; URL builders stay in the client-safe utils.ts. - Grafana: the postProcess POST/PUT now uses validateUrlWithDNS + secureFetchWithPinnedIP, matching the already-pinned initial GET. This completes the Agiloft SSRF pinning started in #4639 (which covered the attach/retrieve API routes) by closing the directExecution path, and extends the same guard to the Grafana update tools. --- apps/sim/tools/agiloft/attachment_info.ts | 3 +- apps/sim/tools/agiloft/create_record.ts | 3 +- apps/sim/tools/agiloft/delete_record.ts | 3 +- apps/sim/tools/agiloft/get_choice_line_id.ts | 3 +- apps/sim/tools/agiloft/lock_record.ts | 3 +- apps/sim/tools/agiloft/read_record.ts | 3 +- apps/sim/tools/agiloft/remove_attachment.ts | 3 +- apps/sim/tools/agiloft/saved_search.ts | 3 +- apps/sim/tools/agiloft/search_records.ts | 3 +- apps/sim/tools/agiloft/select_records.ts | 3 +- apps/sim/tools/agiloft/update_record.ts | 3 +- .../{utils.test.ts => utils.server.test.ts} | 97 ++++++++++------ apps/sim/tools/agiloft/utils.server.ts | 53 +++++++++ apps/sim/tools/agiloft/utils.ts | 105 +----------------- apps/sim/tools/grafana/update_alert_rule.ts | 23 ++-- apps/sim/tools/grafana/update_dashboard.ts | 12 +- 16 files changed, 156 insertions(+), 167 deletions(-) rename apps/sim/tools/agiloft/{utils.test.ts => utils.server.test.ts} (51%) diff --git a/apps/sim/tools/agiloft/attachment_info.ts b/apps/sim/tools/agiloft/attachment_info.ts index 07986303fe..549bbd665a 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 f4763f55ba..c9b852659b 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 4253810496..3a6744b4f1 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 11df104056..596cd8898a 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 d79777b196..4c46d9cb75 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 70b015c43b..ce59238e1f 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 7e9a9d6f2d..7f90e8d6c8 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 8d645d871d..86232d8822 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 b05465c0be..352124787f 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 de4be3139c..5878551d44 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 661be1b3a8..a264b27290 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 b80eb2a33b..aa1e2e6214 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,34 @@ 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') + + // Every hop pins to the single resolved IP. + 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 +107,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 +127,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 +137,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 3aaa0c62b7..4e48db8525 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 811187ab83..34efd86efc 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 19f2bf8164..8cb723c0cd 100644 --- a/apps/sim/tools/grafana/update_alert_rule.ts +++ b/apps/sim/tools/grafana/update_alert_rule.ts @@ -1,4 +1,7 @@ -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' @@ -270,8 +273,9 @@ export const updateAlertRuleTool: ToolConfig Date: Wed, 10 Jun 2026 08:39:24 -0700 Subject: [PATCH 06/36] fix(api): enforce workspace allowPersonalApiKeys policy on v1 surface The external v1 API authenticated API keys without evaluating the per-workspace allowPersonalApiKeys setting, so a personal API key could read and mutate a workspace's resources (workflows, tables, files, knowledge, logs) even when the workspace had explicitly disabled personal keys. The same control is already enforced on the workflow-execution surface. Enforce the policy in checkWorkspaceScope (covering validateWorkspaceAccess too): reject personal keys with 403 when the workspace has allowPersonalApiKeys=false. checkWorkspaceScope becomes async; all v1 route callsites updated to await it. --- apps/sim/app/api/v1/files/route.ts | 2 +- apps/sim/app/api/v1/logs/route.ts | 2 +- apps/sim/app/api/v1/middleware.ts | 28 ++++++++++++++++--- .../api/v1/tables/[tableId]/columns/route.ts | 6 ++-- apps/sim/app/api/v1/tables/[tableId]/route.ts | 4 +-- .../v1/tables/[tableId]/rows/[rowId]/route.ts | 6 ++-- .../app/api/v1/tables/[tableId]/rows/route.ts | 10 +++---- .../v1/tables/[tableId]/rows/upsert/route.ts | 2 +- 8 files changed, 40 insertions(+), 20 deletions(-) diff --git a/apps/sim/app/api/v1/files/route.ts b/apps/sim/app/api/v1/files/route.ts index 06d965ac02..b5e6213045 100644 --- a/apps/sim/app/api/v1/files/route.ts +++ b/apps/sim/app/api/v1/files/route.ts @@ -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 c85031e3cc..91bd67c900 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}`, { diff --git a/apps/sim/app/api/v1/middleware.ts b/apps/sim/app/api/v1/middleware.ts index 92aa72eb34..6472cbb1f7 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 657b448720..0eeebfb99c 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 bf9c38b406..a64831c857 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 122603b713..4aa1d85f93 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 28c4e209cd..ecceb41b1e 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 2258754602..285cd6d1e3 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') From 2e9a09c950d65bcef5dc7979130a5647d4a06e22 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:40:00 -0700 Subject: [PATCH 07/36] fix(billing): close usage-cap admission race with atomic reservation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server-side usage-limit gate read already-recorded cost, but cost is only written when an execution finishes. A burst of concurrent executions all observed the same pre-burst usage, all passed the cap, and all ran — collectively spending far past the limit before any cost landed in the ledger (free-tier abuse / hard-cap defeat). manual/chat triggers also skip rate limiting, removing the only throttle. Add an atomic check-then-reserve admission step (Redis Lua) that bounds in-flight, un-costed executions per billing entity by both a per-plan concurrency cap and remaining usage headroom, so recordedUsage + reservedSlots * estimate <= limit always holds. The slot is released at execution completion via LoggingSession (skipped on pause; TTL self-heals crashes). Runs for all trigger types, covering the previously-unthrottled manual/chat paths. Fails open when billing is disabled or Redis is unavailable, matching the rate limiter — a Redis blip can't turn into an execution outage, and the recorded-usage gate still runs. --- .../calculations/usage-reservation.test.ts | 139 ++++++++++++ .../billing/calculations/usage-reservation.ts | 207 ++++++++++++++++++ apps/sim/lib/execution/preprocessing.ts | 63 ++++++ .../sim/lib/logs/execution/logging-session.ts | 16 ++ 4 files changed, 425 insertions(+) create mode 100644 apps/sim/lib/billing/calculations/usage-reservation.test.ts create mode 100644 apps/sim/lib/billing/calculations/usage-reservation.ts 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 0000000000..4d5fa2b090 --- /dev/null +++ b/apps/sim/lib/billing/calculations/usage-reservation.test.ts @@ -0,0 +1,139 @@ +/** + * @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 fakeRedis = { eval: evalMock } + +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') // free maxConcurrency + expect(args[7]).toBe('1000') // floor((5 - 0) / 0.005) + 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') // team maxConcurrency + }) + + 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('runs the release script for the execution pointer', async () => { + evalMock.mockResolvedValueOnce(1) + await releaseExecutionSlot('exec-1') + const args = evalMock.mock.calls[0] + expect(args[2]).toBe('usage:reservation:exec-1') + expect(args[3]).toBe('exec-1') + }) + + it('is a no-op when billing enforcement is disabled', async () => { + mockFlags.isBillingEnabled = false + await releaseExecutionSlot('exec-1') + expect(evalMock).not.toHaveBeenCalled() + }) + + it('swallows release errors', async () => { + evalMock.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 0000000000..becd0851c0 --- /dev/null +++ b/apps/sim/lib/billing/calculations/usage-reservation.ts @@ -0,0 +1,207 @@ +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)` slots, so + * `recordedUsage + reservedSlots * estimate <= limit` always holds. + */ +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, headroomSlots)` -> otherwise `ZADD` the slot, + * refresh the set TTL, and write the per-execution pointer for release. + */ +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]) +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 +` + +/** + * Release a previously reserved slot. Idempotent: a no-op when the pointer is + * absent (already released or expired). The in-flight set key is rebuilt from + * the pointer value with a fixed prefix (single-instance Redis). + */ +const RELEASE_SCRIPT = ` +local entity = redis.call('GET', KEYS[1]) +if not entity then return 0 end +redis.call('DEL', KEYS[1]) +redis.call('ZREM', '${INFLIGHT_KEY_PREFIX}' .. entity, ARGV[1]) +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. + */ +export async function releaseExecutionSlot(executionId: string): Promise { + if (!isBillingEnabled) { + return + } + + const redis = getRedisClient() + if (!redis) { + return + } + + try { + await redis.eval(RELEASE_SCRIPT, 1, `${POINTER_KEY_PREFIX}${executionId}`, executionId) + } catch (error) { + logger.warn('Failed to release usage reservation', { + error: toError(error).message, + executionId, + }) + } +} diff --git a/apps/sim/lib/execution/preprocessing.ts b/apps/sim/lib/execution/preprocessing.ts index 833d7b9ab1..fb28f552f8 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 { @@ -315,9 +316,14 @@ export async function preprocessExecution( const userSubscription = await getHighestPrioritySubscription(actorUserId) // ========== STEP 5: Check Usage Limits ========== + // Recorded-usage snapshot captured here and reused by the admission + // reservation (STEP 7) so concurrent executions can't all pass the cap before + // any of their costs are recorded. + 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 +502,63 @@ export async function preprocessExecution( } } + // ========== STEP 7: Reserve Admission Slot ========== + // Atomic check-then-reserve that closes the usage-cap race: cost is only + // recorded when an execution finishes, so without a reservation a burst of + // concurrent executions all observe the same pre-burst usage and all pass the + // gate above. Reserving here 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 LoggingSession). + if (!skipUsageLimits && 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) { + // reserveExecutionSlot fails open internally; a throw here is unexpected. + // Don't block execution the recorded-usage gate already admitted. + 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 a0fd011dc7..44cb2f6910 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,21 @@ export class LoggingSession { level: params.level, status: params.status, }) + + // Release the admission reservation taken at preprocessing (STEP 7) so the + // entity's in-flight slot is freed. Skip on pause: a paused execution may + // still resume and should keep holding its slot until it terminates (or the + // reservation TTL-expires). Best-effort — never let a Redis hiccup turn a + // successful completion into a failure. + 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( From 9417348845e39617fe4d740e6250bb0fb10ab9da Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:40:41 -0700 Subject: [PATCH 08/36] fix(workflows): validate folderId belongs to workflow's workspace on create/update/reorder Reject a folderId that references a folder in a different workspace (or an archived/non-existent folder) before writing it to workflow.folderId. Previously create, update, and reorder only checked workspace permission on the workflow and the folder's lock status, never that the folder lived in the workflow's own workspace, allowing a dangling cross-workspace folder reference. Adds isFolderInWorkspace/assertFolderInWorkspace + FolderNotFoundError to @sim/workflow-authz (mirroring assertTargetFolderMutable in the duplicate path), enforced in performCreateWorkflow, performUpdateWorkflow, and the reorder route. Invalid folders now return 400. --- apps/sim/app/api/workflows/[id]/route.ts | 8 +++- apps/sim/app/api/workflows/reorder/route.ts | 9 +++- apps/sim/app/api/workflows/route.ts | 3 +- .../orchestration/workflow-lifecycle.ts | 12 +++++ .../testing/src/mocks/workflow-authz.mock.ts | 19 ++++++++ packages/workflow-authz/src/index.ts | 46 +++++++++++++++++++ 6 files changed, 94 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 3aff417906..d42e75b67e 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -325,7 +325,13 @@ export const PUT = withRouteHandler( if (!result.success || !result.workflow) { const status = - result.errorCode === 'not_found' ? 404 : result.errorCode === 'conflict' ? 409 : 500 + result.errorCode === 'not_found' + ? 404 + : result.errorCode === 'conflict' + ? 409 + : result.errorCode === 'validation' + ? 400 + : 500 return NextResponse.json({ error: result.error }, { status }) } diff --git a/apps/sim/app/api/workflows/reorder/route.ts b/apps/sim/app/api/workflows/reorder/route.ts index fdd049f440..5be0f62d3e 100644 --- a/apps/sim/app/api/workflows/reorder/route.ts +++ b/apps/sim/app/api/workflows/reorder/route.ts @@ -2,9 +2,11 @@ import { db } from '@sim/db' import { workflow } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { + assertFolderInWorkspace, assertFolderMutable, assertWorkflowMutable, FolderLockedError, + FolderNotFoundError, WorkflowLockedError, } from '@sim/workflow-authz' import { eq, inArray } from 'drizzle-orm' @@ -59,6 +61,7 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { 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 b1437de78a..6a8738869f 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/lib/workflows/orchestration/workflow-lifecycle.ts b/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts index 7be2d04971..5f5524f284 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/packages/testing/src/mocks/workflow-authz.mock.ts b/packages/testing/src/mocks/workflow-authz.mock.ts index 15779c8342..59322e1c10 100644 --- a/packages/testing/src/mocks/workflow-authz.mock.ts +++ b/packages/testing/src/mocks/workflow-authz.mock.ts @@ -28,6 +28,20 @@ export class MockFolderLockedError extends Error { } } +/** + * Real `FolderNotFoundError` subclass used by tests so `instanceof` checks in + * route handlers behave the same as in production. Mirrors the shape exported + * by `@sim/workflow-authz`. + */ +export class MockFolderNotFoundError extends Error { + readonly status = 400 + + constructor(message = 'Target folder not found') { + super(message) + this.name = 'FolderNotFoundError' + } +} + const unlockedStatus = { locked: false, directLocked: false, @@ -63,6 +77,8 @@ export const workflowAuthzMockFns = { mockGetWorkflowLockStatus: vi.fn().mockResolvedValue(unlockedStatus), mockAssertWorkflowMutable: vi.fn().mockResolvedValue(undefined), mockAssertFolderMutable: vi.fn().mockResolvedValue(undefined), + mockIsFolderInWorkspace: vi.fn().mockResolvedValue(true), + mockAssertFolderInWorkspace: vi.fn().mockResolvedValue(undefined), } /** @@ -83,6 +99,9 @@ export const workflowAuthzMock = { getWorkflowLockStatus: workflowAuthzMockFns.mockGetWorkflowLockStatus, assertWorkflowMutable: workflowAuthzMockFns.mockAssertWorkflowMutable, assertFolderMutable: workflowAuthzMockFns.mockAssertFolderMutable, + isFolderInWorkspace: workflowAuthzMockFns.mockIsFolderInWorkspace, + assertFolderInWorkspace: workflowAuthzMockFns.mockAssertFolderInWorkspace, WorkflowLockedError: MockWorkflowLockedError, FolderLockedError: MockFolderLockedError, + FolderNotFoundError: MockFolderNotFoundError, } diff --git a/packages/workflow-authz/src/index.ts b/packages/workflow-authz/src/index.ts index f04a5ffb47..5eef77f8e0 100644 --- a/packages/workflow-authz/src/index.ts +++ b/packages/workflow-authz/src/index.ts @@ -191,6 +191,52 @@ export async function assertFolderMutable(folderId: string | null): Promise { + 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 From 5a482a4ffffbf49d494a4eb98da4ddd016ce6282 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:42:15 -0700 Subject: [PATCH 09/36] fix(folders): validate parentId against workspace on create/update/reorder Folder write endpoints accepted a caller-supplied parentId and persisted it without verifying the parent existed in the same workspace, and the create and reorder paths had no cycle guard. A workspace member with write access could reparent a folder to a foreign-workspace folder, a non-existent id, or (via reorder) into a cycle, hiding the folder and its workflows from all members. - performCreateFolder: reject self-parenting and validate the parent exists in the workspace and is not archived (mirrors the duplicate route). - performUpdateFolder: add the same workspace/archived parent check alongside the existing circular-reference guard. - folders/reorder: validate every target parent against the workspace, detect cycles in the resulting parent graph (catches batch cycles), and normalize falsy parentId to null to prevent orphaning. Adds tests for cross-workspace parent rejection and batch-cycle rejection. --- .../sim/app/api/folders/reorder/route.test.ts | 124 ++++++++++++++++++ apps/sim/app/api/folders/reorder/route.ts | 61 ++++++++- apps/sim/app/api/folders/route.test.ts | 23 ++++ .../orchestration/folder-lifecycle.ts | 43 ++++++ 4 files changed, 250 insertions(+), 1 deletion(-) create mode 100644 apps/sim/app/api/folders/reorder/route.test.ts 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 0000000000..61b56e6fa5 --- /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 87222b75b8..274e2bc778 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 a2e145bf1e..a72c0a7bbf 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/lib/workflows/orchestration/folder-lifecycle.ts b/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts index f5bc7fa668..9dc16615a0 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 { From 61d8f49f463d7fa0eb4880e0dce4e2da72d45a1e Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:43:45 -0700 Subject: [PATCH 10/36] chore(knowledge): drop non-TSDoc inline comments from chunks route --- .../knowledge/[id]/documents/[documentId]/chunks/route.ts | 7 ------- 1 file changed, 7 deletions(-) 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 46aa0f5d3e..8e80e41f6e 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({ From 5b6cae91207e8a0153984d0ec4b52c973ed05915 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:44:31 -0700 Subject: [PATCH 11/36] fix(webhooks): fail closed when HMAC signing secret is not configured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inbound webhook signature verification failed open for HMAC providers (GitHub, Intercom, Jira, JSM, Confluence, Cal.com, Notion, Greenhouse, Typeform, Fireflies, Circleback): when no signing secret was stored, verifyAuth returned null and the workflow executed on a fully attacker-controlled body. Reject these deliveries with 401 instead, matching the fail-closed Stripe/WhatsApp/Vercel providers. Run provider reachability/verification handshakes (Notion verification_token, Grain/Intercom ping) ahead of auth so the pre-secret setup handshake still completes — those return a canned 200 without executing the workflow, and real event payloads fall through to fail-closed verification. Update the trigger secret-field copy to state the secret is required for deliveries to be accepted (was misleadingly marked optional). --- apps/sim/app/api/webhooks/trigger/[path]/route.ts | 10 +++++----- apps/sim/lib/webhooks/providers/github.ts | 7 ++++++- apps/sim/lib/webhooks/providers/intercom.ts | 7 ++++++- apps/sim/lib/webhooks/providers/utils.ts | 11 ++++++++++- apps/sim/triggers/circleback/webhook.ts | 5 +++-- apps/sim/triggers/confluence/utils.ts | 2 +- apps/sim/triggers/fireflies/transcription_complete.ts | 3 ++- apps/sim/triggers/github/webhook.ts | 3 ++- apps/sim/triggers/greenhouse/utils.ts | 8 ++++---- apps/sim/triggers/jira/webhook.ts | 3 ++- apps/sim/triggers/jsm/utils.ts | 3 ++- apps/sim/triggers/typeform/webhook.ts | 4 ++-- 12 files changed, 45 insertions(+), 21 deletions(-) diff --git a/apps/sim/app/api/webhooks/trigger/[path]/route.ts b/apps/sim/app/api/webhooks/trigger/[path]/route.ts index 73e71b1f3d..76f789e6c1 100644 --- a/apps/sim/app/api/webhooks/trigger/[path]/route.ts +++ b/apps/sim/app/api/webhooks/trigger/[path]/route.ts @@ -111,6 +111,11 @@ async function handleWebhookPost( const responses: NextResponse[] = [] for (const { webhook: foundWebhook, workflow: foundWorkflow } of webhooksForPath) { + const reachabilityResponse = handleProviderReachabilityTest(foundWebhook, body, requestId) + if (reachabilityResponse) { + return reachabilityResponse + } + const authError = await verifyProviderAuth( foundWebhook, foundWorkflow, @@ -126,11 +131,6 @@ async function handleWebhookPost( return authError } - const reachabilityResponse = handleProviderReachabilityTest(foundWebhook, body, requestId) - if (reachabilityResponse) { - return reachabilityResponse - } - const preprocessResult = await checkWebhookPreprocessing(foundWorkflow, foundWebhook, requestId) if (preprocessResult.error) { if (webhooksForPath.length > 1) { diff --git a/apps/sim/lib/webhooks/providers/github.ts b/apps/sim/lib/webhooks/providers/github.ts index 28c0a782e1..0730f28e27 100644 --- a/apps/sim/lib/webhooks/providers/github.ts +++ b/apps/sim/lib/webhooks/providers/github.ts @@ -48,7 +48,12 @@ export const githubHandler: WebhookProviderHandler = { verifyAuth({ request, rawBody, requestId, providerConfig }: AuthContext) { const secret = providerConfig.webhookSecret as string | undefined if (!secret) { - return null + logger.warn( + `[${requestId}] GitHub webhook missing webhookSecret in providerConfig — rejecting request` + ) + return new NextResponse('Unauthorized - GitHub signing secret not configured', { + status: 401, + }) } const signature = diff --git a/apps/sim/lib/webhooks/providers/intercom.ts b/apps/sim/lib/webhooks/providers/intercom.ts index 61214df8cd..119e57107a 100644 --- a/apps/sim/lib/webhooks/providers/intercom.ts +++ b/apps/sim/lib/webhooks/providers/intercom.ts @@ -49,7 +49,12 @@ export const intercomHandler: WebhookProviderHandler = { verifyAuth({ request, rawBody, requestId, providerConfig }: AuthContext) { const secret = providerConfig.webhookSecret as string | undefined if (!secret) { - return null + logger.warn( + `[${requestId}] Intercom webhook missing webhookSecret in providerConfig — rejecting request` + ) + return new NextResponse('Unauthorized - Intercom signing secret not configured', { + status: 401, + }) } const signature = request.headers.get('X-Hub-Signature') diff --git a/apps/sim/lib/webhooks/providers/utils.ts b/apps/sim/lib/webhooks/providers/utils.ts index dff3db7ce1..0e13c9b000 100644 --- a/apps/sim/lib/webhooks/providers/utils.ts +++ b/apps/sim/lib/webhooks/providers/utils.ts @@ -16,6 +16,10 @@ interface HmacVerifierOptions { /** * Factory that creates a `verifyAuth` implementation for HMAC-signature-based providers. * Covers the common pattern: get secret → check header → validate signature → return 401 or null. + * + * Fails closed: when no signing secret is configured the request is rejected (401), matching + * Stripe/WhatsApp/Vercel. A signed-provider webhook with no secret would otherwise accept any + * unauthenticated body that knows the URL, downgrading the provider's mandatory signature check. */ export function createHmacVerifier({ configKey, @@ -31,7 +35,12 @@ export function createHmacVerifier({ }: AuthContext): Promise => { const secret = providerConfig[configKey] as string | undefined if (!secret) { - return null + logger.warn( + `[${requestId}] ${providerLabel} webhook missing signing secret in providerConfig — rejecting request` + ) + return new NextResponse(`Unauthorized - ${providerLabel} signing secret not configured`, { + status: 401, + }) } const signature = request.headers.get(headerName) diff --git a/apps/sim/triggers/circleback/webhook.ts b/apps/sim/triggers/circleback/webhook.ts index 8fc8fe1af3..1553a855ca 100644 --- a/apps/sim/triggers/circleback/webhook.ts +++ b/apps/sim/triggers/circleback/webhook.ts @@ -38,8 +38,9 @@ export const circlebackWebhookTrigger: TriggerConfig = { id: 'webhookSecret', title: 'Signing Secret', type: 'short-input', - placeholder: 'Paste signing secret from Circleback (optional)', - description: 'Validates that webhook deliveries originate from Circleback using HMAC-SHA256.', + placeholder: 'Paste signing secret from Circleback', + description: + 'Validates that webhook deliveries originate from Circleback using HMAC-SHA256. Required: deliveries are rejected until this is set.', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/confluence/utils.ts b/apps/sim/triggers/confluence/utils.ts index cc722457a9..54f298239b 100644 --- a/apps/sim/triggers/confluence/utils.ts +++ b/apps/sim/triggers/confluence/utils.ts @@ -54,7 +54,7 @@ export function buildConfluenceExtraFields(triggerId: string): SubBlockConfig[] type: 'short-input', placeholder: 'Enter a strong secret', description: - 'Optional secret to validate webhook deliveries from Confluence using HMAC signature', + 'Secret to validate webhook deliveries from Confluence using HMAC signature. Required: deliveries are rejected until this is set.', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/fireflies/transcription_complete.ts b/apps/sim/triggers/fireflies/transcription_complete.ts index 49d3e1fe52..ea55f92991 100644 --- a/apps/sim/triggers/fireflies/transcription_complete.ts +++ b/apps/sim/triggers/fireflies/transcription_complete.ts @@ -25,7 +25,8 @@ export const firefliesTranscriptionCompleteTrigger: TriggerConfig = { title: 'Webhook Secret', type: 'short-input', placeholder: 'Enter your 16-32 character secret', - description: 'Secret key for HMAC signature verification (set in Fireflies dashboard)', + description: + 'Secret key for HMAC signature verification (set in Fireflies dashboard). Required: deliveries are rejected until this is set.', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/github/webhook.ts b/apps/sim/triggers/github/webhook.ts index 5b1b4c630e..9c1aeacd9b 100644 --- a/apps/sim/triggers/github/webhook.ts +++ b/apps/sim/triggers/github/webhook.ts @@ -46,7 +46,8 @@ export const githubWebhookTrigger: TriggerConfig = { title: 'Webhook Secret', type: 'short-input', placeholder: 'Generate or enter a strong secret', - description: 'Validates that webhook deliveries originate from GitHub.', + description: + 'Validates that webhook deliveries originate from GitHub. Required: deliveries are rejected until this is set.', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/greenhouse/utils.ts b/apps/sim/triggers/greenhouse/utils.ts index 8210761dc8..93204ba3c0 100644 --- a/apps/sim/triggers/greenhouse/utils.ts +++ b/apps/sim/triggers/greenhouse/utils.ts @@ -62,17 +62,17 @@ export function isGreenhouseEventMatch(triggerId: string, action: string): boole /** * Builds extra fields for Greenhouse triggers. - * Includes an optional secret key for HMAC signature verification. + * Includes the secret key used for HMAC signature verification. */ export function buildGreenhouseExtraFields(triggerId: string): SubBlockConfig[] { return [ { id: 'secretKey', - title: 'Secret Key (Optional)', + title: 'Secret Key', type: 'short-input', placeholder: 'Enter the same secret key configured in Greenhouse', description: - 'When set, requests must include a valid Signature header (HMAC-SHA256). If left empty, the endpoint does not verify signatures—only use on a private URL you fully control.', + 'Used to verify the HMAC-SHA256 Signature header. Required: deliveries are rejected until a secret key is configured here and in Greenhouse.', password: true, mode: 'trigger', condition: { field: 'selectedTriggerId', value: triggerId }, @@ -101,7 +101,7 @@ export function greenhouseSetupInstructions(eventType: string): string { 'In Greenhouse, go to Configure > Dev Center > Webhooks.', 'Click Create New Webhook.', 'Paste the Webhook URL into the Endpoint URL field.', - 'Enter a Secret Key for HMAC signature verification (recommended). Leave empty only if you accept unauthenticated POSTs to this URL.', + 'Enter a Secret Key for HMAC signature verification. This is required — deliveries without a valid signature are rejected.', `Under When, select the appropriate ${eventType}.`, 'Click Create Webhook to save.', 'Click "Save" above to activate your trigger.', diff --git a/apps/sim/triggers/jira/webhook.ts b/apps/sim/triggers/jira/webhook.ts index 57c9475193..b1ac9dc531 100644 --- a/apps/sim/triggers/jira/webhook.ts +++ b/apps/sim/triggers/jira/webhook.ts @@ -34,7 +34,8 @@ export const jiraWebhookTrigger: TriggerConfig = { title: 'Webhook Secret', type: 'short-input', placeholder: 'Enter a strong secret', - description: 'Optional secret to validate webhook deliveries from Jira using HMAC signature', + description: + 'Secret to validate webhook deliveries from Jira using HMAC signature. Required: deliveries are rejected until this is set.', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/jsm/utils.ts b/apps/sim/triggers/jsm/utils.ts index 984784bf9c..24e037206d 100644 --- a/apps/sim/triggers/jsm/utils.ts +++ b/apps/sim/triggers/jsm/utils.ts @@ -49,7 +49,8 @@ function jsmWebhookSecretField(triggerId: string): SubBlockConfig { title: 'Webhook Secret', type: 'short-input', placeholder: 'Enter a strong secret', - description: 'Optional secret to validate webhook deliveries from Jira using HMAC signature', + description: + 'Secret to validate webhook deliveries from Jira using HMAC signature. Required: deliveries are rejected until this is set.', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/typeform/webhook.ts b/apps/sim/triggers/typeform/webhook.ts index 53bec744e3..29bcdd97ad 100644 --- a/apps/sim/triggers/typeform/webhook.ts +++ b/apps/sim/triggers/typeform/webhook.ts @@ -45,9 +45,9 @@ export const typeformWebhookTrigger: TriggerConfig = { id: 'secret', title: 'Webhook Secret', type: 'short-input', - placeholder: 'Enter a secret for webhook signature verification (optional)', + placeholder: 'Enter a secret for webhook signature verification', description: - 'A secret string used to verify webhook authenticity. Highly recommended for security. Generate a secure random string (min 20 characters recommended).', + 'A secret string used to verify webhook authenticity. Required: deliveries are rejected until this is set. Generate a secure random string (min 20 characters recommended).', password: true, required: false, mode: 'trigger', From 00e29d08e5547ea7b4f0ae9b9be786e9f61252fa Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:45:16 -0700 Subject: [PATCH 12/36] style(files): trim verbose inline comments on delete authorization fix --- apps/sim/app/api/files/authorization.test.ts | 4 +--- apps/sim/app/api/files/authorization.ts | 5 +---- apps/sim/app/api/files/delete/route.test.ts | 4 +--- apps/sim/app/api/files/delete/route.ts | 21 +++++--------------- 4 files changed, 8 insertions(+), 26 deletions(-) diff --git a/apps/sim/app/api/files/authorization.test.ts b/apps/sim/app/api/files/authorization.test.ts index 5cc58aa848..ce92c16191 100644 --- a/apps/sim/app/api/files/authorization.test.ts +++ b/apps/sim/app/api/files/authorization.test.ts @@ -177,9 +177,7 @@ describe('public-context access (profile-pictures / og-images / workspace-logos) }) it('denies a cross-tenant delete that names a workspace key under a public context', async () => { - // The reported attack: a workspace file key passed with context og-images. - // og-images has no user-facing delete path, so the destructive op is denied - // without ever short-circuiting. + // Reported attack: a workspace key passed under a public context to dodge the ownership check. await expect(write('workspace/victim-ws/123-report.pdf', 'og-images')).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 21d76c5eae..0d4a756162 100644 --- a/apps/sim/app/api/files/authorization.ts +++ b/apps/sim/app/api/files/authorization.ts @@ -144,10 +144,7 @@ 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 - // world-readable, so reads short-circuit. Destructive operations - // (`requireWrite`) must NOT short-circuit — they require proof of ownership - // of the user/workspace the object belongs to, never an unconditional allow. + // 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' || diff --git a/apps/sim/app/api/files/delete/route.test.ts b/apps/sim/app/api/files/delete/route.test.ts index 9379bbf1a3..5b2f88dcff 100644 --- a/apps/sim/app/api/files/delete/route.test.ts +++ b/apps/sim/app/api/files/delete/route.test.ts @@ -200,9 +200,7 @@ describe('File Delete API Route', () => { }) it('rejects a client context that disagrees with the key prefix', async () => { - // Reported attack: a workspace file key passed with a public context to dodge - // the ownership check. The context is derived from the key, so this is denied - // before authorization or deletion ever runs. + // Reported attack: a workspace key passed under a public context to dodge the ownership check. const req = createMockRequest('POST', { filePath: '/api/files/serve/s3/workspace/victim-ws/1234-report.pdf', context: 'og-images', diff --git a/apps/sim/app/api/files/delete/route.ts b/apps/sim/app/api/files/delete/route.ts index 04a955edbc..34903aa22e 100644 --- a/apps/sim/app/api/files/delete/route.ts +++ b/apps/sim/app/api/files/delete/route.ts @@ -62,31 +62,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => { try { const key = extractStorageKeyFromPath(filePath) - // The context is derived from the trusted key prefix, never the - // client-supplied `context`. A caller cannot name a workspace key while - // claiming a public context (e.g. `og-images`) to dodge the ownership - // check. If a `context` is supplied it must agree with the 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 }) From be0963c3e3df163280a2fe5539f25fc9848f81c0 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:45:30 -0700 Subject: [PATCH 13/36] fix(auth): close account-enumeration oracle on email sign-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The custom before-hook pre-check threw a distinguishing 422/USER_ALREADY_EXISTS for already-registered emails, letting an unauthenticated attacker enumerate accounts — defeating better-auth's own OWASP enumeration protection (active under requireEmailVerification). Remove the pre-check and rely on better-auth's generic duplicate-sign-up response, wiring: - onExistingUserSignUp: notify the real account owner out-of-band, mirroring the privacy-preserving forget-password flow. - customSyntheticUser: include admin (role/banned/banReason/banExpires) and Stripe (stripeCustomerId, billing-gated) user fields so the fake response shape is byte-identical to a real new-user response. Adds an ExistingAccountEmail template + 'existing-account' subject. --- .../emails/auth/existing-account-email.tsx | 46 +++++++++++ apps/sim/components/emails/auth/index.ts | 1 + apps/sim/components/emails/render.ts | 5 ++ apps/sim/components/emails/subjects.ts | 3 + apps/sim/lib/auth/auth.ts | 78 +++++++++++++++---- 5 files changed, 116 insertions(+), 17 deletions(-) create mode 100644 apps/sim/components/emails/auth/existing-account-email.tsx 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 0000000000..0fc8f7e752 --- /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 3d5438dd69..de9eff40e9 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 645f5a056b..94650c85f1 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 347bf4074d..a1ddbd3ed3 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/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 7a65dfa10e..5037e79565 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 }), }, From 6d50823442a865878a65b9c98c24dd5845566b9a Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:49:46 -0700 Subject: [PATCH 14/36] style(tools): drop non-TSDoc inline comments from Grafana/Agiloft SSRF tools --- apps/sim/tools/agiloft/utils.server.test.ts | 1 - apps/sim/tools/grafana/update_alert_rule.ts | 7 ------- apps/sim/tools/grafana/update_dashboard.ts | 14 +------------- 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/apps/sim/tools/agiloft/utils.server.test.ts b/apps/sim/tools/agiloft/utils.server.test.ts index aa1e2e6214..9d207b0627 100644 --- a/apps/sim/tools/agiloft/utils.server.test.ts +++ b/apps/sim/tools/agiloft/utils.server.test.ts @@ -80,7 +80,6 @@ describe('executeAgiloftRequest', () => { expect(calls[1][0]).toBe('https://example.agiloft.com/ewws/REST/demo/contracts/42') expect(calls[2][0]).toBe('https://example.agiloft.com/ewws/EWLogout?$KB=demo') - // Every hop pins to the single resolved IP. for (const call of calls) { expect(call[1]).toBe('203.0.113.10') } diff --git a/apps/sim/tools/grafana/update_alert_rule.ts b/apps/sim/tools/grafana/update_alert_rule.ts index 8cb723c0cd..ba474e2ed9 100644 --- a/apps/sim/tools/grafana/update_alert_rule.ts +++ b/apps/sim/tools/grafana/update_alert_rule.ts @@ -6,7 +6,6 @@ import { ALERT_RULE_OUTPUT_FIELDS, type GrafanaUpdateAlertRuleParams } from '@/t 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', @@ -137,7 +136,6 @@ export const updateAlertRuleTool: ToolConfig `${params.baseUrl.replace(/\/$/, '')}/api/v1/provisioning/alert-rules/${params.alertRuleUid}`, method: 'GET', @@ -154,7 +152,6 @@ export const updateAlertRuleTool: ToolConfig { - // Store the existing rule data for postProcess to use const data = await response.json() return { success: true, @@ -165,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) { @@ -176,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 @@ -261,7 +255,6 @@ export const updateAlertRuleTool: ToolConfig = { 'Content-Type': 'application/json', Authorization: `Bearer ${params.apiKey}`, diff --git a/apps/sim/tools/grafana/update_dashboard.ts b/apps/sim/tools/grafana/update_dashboard.ts index 588f106a36..8441a52a39 100644 --- a/apps/sim/tools/grafana/update_dashboard.ts +++ b/apps/sim/tools/grafana/update_dashboard.ts @@ -5,7 +5,6 @@ import { import type { GrafanaUpdateDashboardParams } from '@/tools/grafana/types' import type { ToolConfig, ToolResponse } from '@/tools/types' -// Using ToolResponse for intermediate state since this tool fetches existing data first export const updateDashboardTool: ToolConfig = { id: 'grafana_update_dashboard', name: 'Grafana Update Dashboard', @@ -90,7 +89,6 @@ export const updateDashboardTool: ToolConfig `${params.baseUrl.replace(/\/$/, '')}/api/dashboards/uid/${params.dashboardUid}`, method: 'GET', @@ -107,7 +105,6 @@ export const updateDashboardTool: ToolConfig { - // Store the existing dashboard data for postProcess to use const data = await response.json() return { success: true, @@ -119,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 @@ -131,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 @@ -151,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) { @@ -178,7 +167,6 @@ export const updateDashboardTool: ToolConfig = { 'Content-Type': 'application/json', Authorization: `Bearer ${params.apiKey}`, From b9aeab6b7c061d8ae32428381d9803d90916dd1d Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:50:22 -0700 Subject: [PATCH 15/36] chore(api): trim extraneous inline comments in v1 logs/files routes Remove a redundant size annotation and two verbose multi-line materialization comments whose intent is already clear from the code. Load-bearing comments (race-condition and key-translation notes) kept. --- apps/sim/app/api/v1/files/route.ts | 2 +- apps/sim/app/api/v1/logs/route.ts | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/apps/sim/app/api/v1/files/route.ts b/apps/sim/app/api/v1/files/route.ts index b5e6213045..9e573a126f 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. */ diff --git a/apps/sim/app/api/v1/logs/route.ts b/apps/sim/app/api/v1/logs/route.ts index 91bd67c900..dd20072dc0 100644 --- a/apps/sim/app/api/v1/logs/route.ts +++ b/apps/sim/app/api/v1/logs/route.ts @@ -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) From 2979061283e8887c513b0289992efa3a12870892 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:50:28 -0700 Subject: [PATCH 16/36] fix(billing): exclude table-cell dispatch from admission reservation Table-cell dispatch is row-bounded, async rate-limited, and already surfaces a graceful usage state. Applying the in-flight concurrency reservation there turned its 429 into a hard cell error on a normal >15-concurrent-cell run (only 402 was handled gracefully). Skip the reservation for that surface via a new skipConcurrencyReservation option (the usage-cost cap is still enforced), and tidy the reservation comments to TSDoc. --- .../background/workflow-column-execution.ts | 1 + .../calculations/usage-reservation.test.ts | 6 ++-- apps/sim/lib/execution/preprocessing.ts | 32 +++++++++++-------- .../sim/lib/logs/execution/logging-session.ts | 7 ++-- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/apps/sim/background/workflow-column-execution.ts b/apps/sim/background/workflow-column-execution.ts index 44c87461d8..bba438c56d 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/lib/billing/calculations/usage-reservation.test.ts b/apps/sim/lib/billing/calculations/usage-reservation.test.ts index 4d5fa2b090..f610d930ad 100644 --- a/apps/sim/lib/billing/calculations/usage-reservation.test.ts +++ b/apps/sim/lib/billing/calculations/usage-reservation.test.ts @@ -72,8 +72,8 @@ describe('usage-reservation', () => { // 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') // free maxConcurrency - expect(args[7]).toBe('1000') // floor((5 - 0) / 0.005) + expect(args[6]).toBe('15') + expect(args[7]).toBe('1000') expect(args[8]).toBe('exec-1') expect(args[9]).toBe('user:user-1') }) @@ -86,7 +86,7 @@ describe('usage-reservation', () => { }) const args = evalMock.mock.calls[0] expect(args[2]).toBe('usage:inflight:org:org-9') - expect(args[6]).toBe('150') // team maxConcurrency + expect(args[6]).toBe('150') }) it('clamps negative headroom to zero slots', async () => { diff --git a/apps/sim/lib/execution/preprocessing.ts b/apps/sim/lib/execution/preprocessing.ts index fb28f552f8..0ff0d9eba5 100644 --- a/apps/sim/lib/execution/preprocessing.ts +++ b/apps/sim/lib/execution/preprocessing.ts @@ -39,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 @@ -94,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, @@ -316,9 +325,7 @@ export async function preprocessExecution( const userSubscription = await getHighestPrioritySubscription(actorUserId) // ========== STEP 5: Check Usage Limits ========== - // Recorded-usage snapshot captured here and reused by the admission - // reservation (STEP 7) so concurrent executions can't all pass the cap before - // any of their costs are recorded. + // Snapshot reused by the STEP 7 admission reservation. let usageSnapshot: { currentUsage: number; limit: number } | null = null if (!skipUsageLimits) { try { @@ -502,14 +509,15 @@ export async function preprocessExecution( } } - // ========== STEP 7: Reserve Admission Slot ========== - // Atomic check-then-reserve that closes the usage-cap race: cost is only - // recorded when an execution finishes, so without a reservation a burst of - // concurrent executions all observe the same pre-burst usage and all pass the - // gate above. Reserving here 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 LoggingSession). - if (!skipUsageLimits && usageSnapshot) { + /** + * 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, @@ -550,8 +558,6 @@ export async function preprocessExecution( } } } catch (error) { - // reserveExecutionSlot fails open internally; a throw here is unexpected. - // Don't block execution the recorded-usage gate already admitted. logger.error(`[${requestId}] Unexpected error reserving admission slot`, { error, actorUserId, diff --git a/apps/sim/lib/logs/execution/logging-session.ts b/apps/sim/lib/logs/execution/logging-session.ts index 44cb2f6910..a63aa3fb30 100644 --- a/apps/sim/lib/logs/execution/logging-session.ts +++ b/apps/sim/lib/logs/execution/logging-session.ts @@ -268,11 +268,8 @@ export class LoggingSession { status: params.status, }) - // Release the admission reservation taken at preprocessing (STEP 7) so the - // entity's in-flight slot is freed. Skip on pause: a paused execution may - // still resume and should keep holding its slot until it terminates (or the - // reservation TTL-expires). Best-effort — never let a Redis hiccup turn a - // successful completion into a failure. + // 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) From 86e26b8128d9d8256107634b9ef0917ac8ee957a Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:51:34 -0700 Subject: [PATCH 17/36] fix(chat): rate-limit and constant-time password auth for public chats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Password-protected public chat (POST /api/chat/[identifier]) had no throttling on the password check and compared with a non-constant-time !==, allowing unlimited brute-force and per-character timing leaks. - Add per-IP rate limiting (10 / 15min) to the password branch of validateChatAuth, mirroring the OTP/SSO endpoints; return 429 with Retry-After. Only explicit unlock attempts consume tokens — message sends carry no password and ride the auth cookie. - Replace password !== decrypted with safeCompare. - Fails open on rate-limiter storage errors; no availability regression. --- apps/sim/app/api/chat/[identifier]/route.ts | 9 ++++- apps/sim/app/api/chat/utils.test.ts | 35 +++++++++++++++++++ apps/sim/app/api/chat/utils.ts | 37 +++++++++++++++++++-- 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/chat/[identifier]/route.ts b/apps/sim/app/api/chat/[identifier]/route.ts index 7a4a12d575..21a12b30f8 100644 --- a/apps/sim/app/api/chat/[identifier]/route.ts +++ b/apps/sim/app/api/chat/[identifier]/route.ts @@ -125,7 +125,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 diff --git a/apps/sim/app/api/chat/utils.test.ts b/apps/sim/app/api/chat/utils.test.ts index 86e46340a9..d12867a1d3 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: vi.fn().mockImplementation(() => ({ + 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 5a3d0750e8..70e4a657ac 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' } } From 595b678d4054f1c1f93509637bd688cea79e8f4a Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:51:36 -0700 Subject: [PATCH 18/36] fix(security): cap JSON request body size and gate public chat endpoint The shared parseJsonBody helper (behind parseRequest, used by nearly every contract route) read request bodies with no size limit, buffering the full body into memory before validation. The unauthenticated public deployed-chat endpoint reached this sink with no admission gate, enabling an anonymous memory-exhaustion DoS. - parseRequest/parseJsonBody now enforce a byte cap via a size-limited stream read (content-length precheck + streamed cap), returning 413. Default is API_MAX_JSON_BODY_BYTES (50 MB), overridable per route via maxBodyBytes. Decoding uses TextDecoder to match request.json() BOM handling. - Public chat POST is wrapped with the admission gate (tryAdmit) and passes an explicit CHAT_MAX_REQUEST_BYTES (20 MB) cap. - Chat body contract gains .max() bounds on input, password, conversationId, file data/name/type, and files array length. - Admin bulk workspace import opts into a higher 100 MB cap to avoid regressing large multi-workflow imports. --- apps/sim/app/api/chat/[identifier]/route.ts | 21 ++++-- apps/sim/app/api/chat/utils.test.ts | 35 --------- apps/sim/app/api/chat/utils.ts | 37 +--------- .../v1/admin/workspaces/[id]/import/route.ts | 8 +- apps/sim/lib/api/contracts/chats.ts | 27 +++++-- apps/sim/lib/api/server/validation.ts | 73 +++++++++++++++++-- apps/sim/lib/core/config/env.ts | 2 + 7 files changed, 111 insertions(+), 92 deletions(-) diff --git a/apps/sim/app/api/chat/[identifier]/route.ts b/apps/sim/app/api/chat/[identifier]/route.ts index 21a12b30f8..eb836911f1 100644 --- a/apps/sim/app/api/chat/[identifier]/route.ts +++ b/apps/sim/app/api/chat/[identifier]/route.ts @@ -6,6 +6,8 @@ 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 { 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 +42,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) + 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,14 +135,7 @@ export const POST = withRouteHandler( const authResult = await validateChatAuth(requestId, deployment, request, parsedBody) if (!authResult.authorized) { - 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 + return createErrorResponse(authResult.error || 'Authentication required', 401) } const { input, password, email, conversationId, files } = parsedBody @@ -295,6 +298,8 @@ export const POST = withRouteHandler( } 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 d12867a1d3..86e46340a9 100644 --- a/apps/sim/app/api/chat/utils.test.ts +++ b/apps/sim/app/api/chat/utils.test.ts @@ -19,7 +19,6 @@ const { mockSetDeploymentAuthCookie, mockIsEmailAllowed, mockGetSession, - mockCheckRateLimitDirect, } = vi.hoisted(() => ({ mockMergeSubblockStateWithValues: vi.fn().mockReturnValue({}), mockMergeSubBlockValues: vi.fn().mockReturnValue({}), @@ -27,13 +26,6 @@ const { mockSetDeploymentAuthCookie: vi.fn(), mockIsEmailAllowed: vi.fn(), mockGetSession: vi.fn(), - mockCheckRateLimitDirect: vi.fn().mockResolvedValue({ allowed: true }), -})) - -vi.mock('@/lib/core/rate-limiter', () => ({ - RateLimiter: vi.fn().mockImplementation(() => ({ - checkRateLimitDirect: mockCheckRateLimitDirect, - })), })) vi.mock('@/lib/auth', () => ({ @@ -157,7 +149,6 @@ 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 () => { @@ -244,32 +235,6 @@ 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 70e4a657ac..5a3d0750e8 100644 --- a/apps/sim/app/api/chat/utils.ts +++ b/apps/sim/app/api/chat/utils.ts @@ -1,34 +1,18 @@ 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, @@ -104,7 +88,7 @@ export async function validateChatAuth( deployment: any, request: NextRequest, parsedBody?: any -): Promise<{ authorized: boolean; error?: string; status?: number; retryAfterMs?: number }> { +): Promise<{ authorized: boolean; error?: string }> { const authType = deployment.authType || 'public' if (authType === 'public') { @@ -145,25 +129,8 @@ 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 (!safeCompare(password, decrypted)) { + if (password !== decrypted) { return { authorized: false, error: 'Invalid password' } } 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 ae73a27074..ad7780ef14 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,7 +94,7 @@ 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: [...] }') diff --git a/apps/sim/lib/api/contracts/chats.ts b/apps/sim/lib/api/contracts/chats.ts index c3e908121b..94777002f2 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 = 100_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 4128efcfa9..a312fbadbf 100644 --- a/apps/sim/lib/api/server/validation.ts +++ b/apps/sim/lib/api/server/validation.ts @@ -8,6 +8,20 @@ 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`. + */ +export const DEFAULT_MAX_JSON_BODY_BYTES = Number.parseInt(env.API_MAX_JSON_BODY_BYTES, 10) export interface ValidationErrorBody { error: string @@ -35,6 +49,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 +89,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 +196,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/core/config/env.ts b/apps/sim/lib/core/config/env.ts index a4fd479f44..0d93a8dfb6 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('20971520'),// Max request body size for the public deployed-chat endpoint (20 MB) // Rate Limiting Configuration RATE_LIMIT_WINDOW_MS: z.string().optional().default('60000'), // Rate limit window duration in milliseconds (default: 1 minute) From 62764cbd406667eea6e4680a4fd64468234a4c7c Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:53:47 -0700 Subject: [PATCH 19/36] fix(chat): rate-limit and constant-time password auth for public chats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Password-protected public chat (POST /api/chat/[identifier]) had no throttling on the password check and compared with a non-constant-time !==, allowing unlimited brute-force and per-character timing leaks. - Add per-IP rate limiting (10 / 15min) to the password branch of validateChatAuth, mirroring the OTP/SSO endpoints; return 429 with Retry-After. Only explicit unlock attempts consume tokens — message sends carry no password and ride the auth cookie. - Replace password !== decrypted with safeCompare. - Fails open on rate-limiter storage errors; no availability regression. Reinstates the fix reverted by an intervening commit. --- apps/sim/app/api/chat/[identifier]/route.ts | 9 ++++- apps/sim/app/api/chat/utils.test.ts | 35 +++++++++++++++++++ apps/sim/app/api/chat/utils.ts | 37 +++++++++++++++++++-- 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/chat/[identifier]/route.ts b/apps/sim/app/api/chat/[identifier]/route.ts index eb836911f1..9f69975510 100644 --- a/apps/sim/app/api/chat/[identifier]/route.ts +++ b/apps/sim/app/api/chat/[identifier]/route.ts @@ -135,7 +135,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 diff --git a/apps/sim/app/api/chat/utils.test.ts b/apps/sim/app/api/chat/utils.test.ts index 86e46340a9..d12867a1d3 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: vi.fn().mockImplementation(() => ({ + 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 5a3d0750e8..70e4a657ac 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' } } From ac565253a43eba8e032d5c7daa26ee24cc407eb6 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:55:32 -0700 Subject: [PATCH 20/36] fix(billing): never block a lone execution on usage headroom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admission reservation tapered allowed concurrency by remaining usage headroom. With under one credit of headroom left (but not yet over the cap), floor(headroom / estimate) hit zero and rejected even a single, zero-concurrency execution — stricter than the recorded-usage gate, which would have allowed that last run, and with a misleading "too many concurrent executions" message. Floor the headroom term at 1 so a lone execution is governed only by the cost gate; concurrency above the first slot still tapers with headroom. --- apps/sim/app/api/chat/utils.test.ts | 35 ------------------ apps/sim/app/api/chat/utils.ts | 37 +------------------ .../billing/calculations/usage-reservation.ts | 15 ++++++-- 3 files changed, 13 insertions(+), 74 deletions(-) diff --git a/apps/sim/app/api/chat/utils.test.ts b/apps/sim/app/api/chat/utils.test.ts index d12867a1d3..86e46340a9 100644 --- a/apps/sim/app/api/chat/utils.test.ts +++ b/apps/sim/app/api/chat/utils.test.ts @@ -19,7 +19,6 @@ const { mockSetDeploymentAuthCookie, mockIsEmailAllowed, mockGetSession, - mockCheckRateLimitDirect, } = vi.hoisted(() => ({ mockMergeSubblockStateWithValues: vi.fn().mockReturnValue({}), mockMergeSubBlockValues: vi.fn().mockReturnValue({}), @@ -27,13 +26,6 @@ const { mockSetDeploymentAuthCookie: vi.fn(), mockIsEmailAllowed: vi.fn(), mockGetSession: vi.fn(), - mockCheckRateLimitDirect: vi.fn().mockResolvedValue({ allowed: true }), -})) - -vi.mock('@/lib/core/rate-limiter', () => ({ - RateLimiter: vi.fn().mockImplementation(() => ({ - checkRateLimitDirect: mockCheckRateLimitDirect, - })), })) vi.mock('@/lib/auth', () => ({ @@ -157,7 +149,6 @@ 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 () => { @@ -244,32 +235,6 @@ 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 70e4a657ac..5a3d0750e8 100644 --- a/apps/sim/app/api/chat/utils.ts +++ b/apps/sim/app/api/chat/utils.ts @@ -1,34 +1,18 @@ 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, @@ -104,7 +88,7 @@ export async function validateChatAuth( deployment: any, request: NextRequest, parsedBody?: any -): Promise<{ authorized: boolean; error?: string; status?: number; retryAfterMs?: number }> { +): Promise<{ authorized: boolean; error?: string }> { const authType = deployment.authType || 'public' if (authType === 'public') { @@ -145,25 +129,8 @@ 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 (!safeCompare(password, decrypted)) { + if (password !== decrypted) { return { authorized: false, error: 'Invalid password' } } diff --git a/apps/sim/lib/billing/calculations/usage-reservation.ts b/apps/sim/lib/billing/calculations/usage-reservation.ts index becd0851c0..5ef1f5d798 100644 --- a/apps/sim/lib/billing/calculations/usage-reservation.ts +++ b/apps/sim/lib/billing/calculations/usage-reservation.ts @@ -32,8 +32,12 @@ const MAX_CONCURRENT_EXECUTIONS: Record = { /** * 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)` slots, so - * `recordedUsage + reservedSlots * estimate <= limit` always holds. + * 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 @@ -48,8 +52,10 @@ const POINTER_KEY_PREFIX = 'usage:reservation:' * 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, headroomSlots)` -> otherwise `ZADD` the slot, - * refresh the set TTL, and write the per-execution pointer for release. + * `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]) @@ -59,6 +65,7 @@ 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 From 26f3cba84fa47fc4c8b3281ef33fdd0fa6caab6a Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:56:01 -0700 Subject: [PATCH 21/36] refactor(env): document workspace env masking, drop inline comments Extract the workspace-env value masking into a TSDoc-documented maskWorkspaceEnvForViewer helper and remove the redundant inline comments from the GET handler and its test. No behavior change. --- .../workspaces/[id]/environment/route.test.ts | 2 - .../api/workspaces/[id]/environment/route.ts | 63 +++++++++++++------ 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/apps/sim/app/api/workspaces/[id]/environment/route.test.ts b/apps/sim/app/api/workspaces/[id]/environment/route.test.ts index 1e60969c9e..6ad6192159 100644 --- a/apps/sim/app/api/workspaces/[id]/environment/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/environment/route.test.ts @@ -85,9 +85,7 @@ describe('GET /api/workspaces/[id]/environment', () => { const { status, body } = await callGet() expect(status).toBe(200) - // Variable names are preserved so editor autocomplete keeps working... expect(Object.keys(body.data.workspace).sort()).toEqual(['DATABASE_URL', 'OPENAI_API_KEY']) - // ...but plaintext values are withheld. expect(body.data.workspace.OPENAI_API_KEY).toBe('') expect(body.data.workspace.DATABASE_URL).toBe('') }) diff --git a/apps/sim/app/api/workspaces/[id]/environment/route.ts b/apps/sim/app/api/workspaces/[id]/environment/route.ts index d45c8f2465..0641aee4ec 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,29 +97,17 @@ export const GET = withRouteHandler( workspaceId ) - // Plaintext workspace secrets are restricted 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 caller may view a value if they are a credential admin - // of that key, or — for legacy keys predating per-secret ACLs — if they - // hold workspace `admin` permission. This mirrors the per-key edit gating - // in PUT/DELETE: if you can administer a secret, you can read it. - const workspaceKeys = Object.keys(workspaceDecrypted) - const { adminKeys, knownKeys } = await getWorkspaceEnvKeyAdminAccess({ + const workspace = await maskWorkspaceEnvForViewer({ + workspaceDecrypted, workspaceId, - envKeys: workspaceKeys, userId, + permission, }) - const workspaceMasked: Record = {} - for (const key of workspaceKeys) { - const canViewValue = adminKeys.has(key) || (!knownKeys.has(key) && permission === 'admin') - workspaceMasked[key] = canViewValue ? workspaceDecrypted[key] : '' - } return NextResponse.json( { data: { - workspace: workspaceMasked, + workspace, personal: personalDecrypted, conflicts, }, From 87dface081402c8b72009dbea343c145d9e3a74e Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 08:58:48 -0700 Subject: [PATCH 22/36] refactor(env): convert PUT/DELETE authz comments to TSDoc Move the tiered-authorization rationale for the workspace env upsert and delete handlers into TSDoc blocks and drop the inline comments. No behavior change. --- .../api/workspaces/[id]/environment/route.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/apps/sim/app/api/workspaces/[id]/environment/route.ts b/apps/sim/app/api/workspaces/[id]/environment/route.ts index 0641aee4ec..c32065c49d 100644 --- a/apps/sim/app/api/workspaces/[id]/environment/route.ts +++ b/apps/sim/app/api/workspaces/[id]/environment/route.ts @@ -124,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() @@ -142,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 }) @@ -265,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() @@ -283,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 }) From 41f133a9d72dcd1c975a8b649820555d5d0f96ae Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 09:02:22 -0700 Subject: [PATCH 23/36] fix(telegram): keep legacy webhooks working via Telegram source-IP fallback The secret-token check rejected every webhook registered before secret_token support, breaking live triggers until re-saved. Fall back to verifying the request originates from Telegram's published webhook IP ranges when no secret is configured, so existing triggers keep firing with no re-save or migration while forged updates from arbitrary hosts are still rejected. Webhooks with a registered secret continue to use strict constant-time token verification. --- .../lib/webhooks/providers/telegram.test.ts | 35 +++++++++- apps/sim/lib/webhooks/providers/telegram.ts | 67 ++++++++++++++----- 2 files changed, 84 insertions(+), 18 deletions(-) diff --git a/apps/sim/lib/webhooks/providers/telegram.test.ts b/apps/sim/lib/webhooks/providers/telegram.test.ts index 98c941fa2d..6809deb572 100644 --- a/apps/sim/lib/webhooks/providers/telegram.test.ts +++ b/apps/sim/lib/webhooks/providers/telegram.test.ts @@ -1,5 +1,6 @@ +import { requestUtilsMockFns } from '@sim/testing' import { NextRequest } from 'next/server' -import { describe, expect, it } from 'vitest' +import { beforeEach, describe, expect, it } from 'vitest' import { telegramHandler } from '@/lib/webhooks/providers/telegram' function reqWithHeaders(headers: Record): NextRequest { @@ -7,7 +8,11 @@ function reqWithHeaders(headers: Record): NextRequest { } describe('Telegram webhook provider', () => { - it('verifyAuth rejects when secretToken is not configured', () => { + beforeEach(() => { + requestUtilsMockFns.mockGetClientIp.mockReturnValue('203.0.113.7') + }) + + it('verifyAuth rejects an unconfigured webhook when the source IP is not Telegram', () => { const res = telegramHandler.verifyAuth!({ request: reqWithHeaders({ 'x-telegram-bot-api-secret-token': 'anything' }), rawBody: '{}', @@ -19,6 +24,32 @@ describe('Telegram webhook provider', () => { expect((res as { status?: number })?.status).toBe(401) }) + it('verifyAuth accepts a legacy webhook (no secret) from a Telegram source IP', () => { + requestUtilsMockFns.mockGetClientIp.mockReturnValue('149.154.167.197') + const res = telegramHandler.verifyAuth!({ + request: reqWithHeaders({}), + rawBody: '{}', + requestId: 't1b', + providerConfig: {}, + webhook: {}, + workflow: {}, + }) + expect(res).toBeNull() + }) + + it('verifyAuth rejects a legacy webhook (no secret) when the source IP is unknown', () => { + requestUtilsMockFns.mockGetClientIp.mockReturnValue('unknown') + const res = telegramHandler.verifyAuth!({ + request: reqWithHeaders({}), + rawBody: '{}', + requestId: 't1c', + providerConfig: {}, + webhook: {}, + workflow: {}, + }) + expect((res as { status?: number })?.status).toBe(401) + }) + it('verifyAuth rejects when the secret token header is missing', () => { const res = telegramHandler.verifyAuth!({ request: reqWithHeaders({}), diff --git a/apps/sim/lib/webhooks/providers/telegram.ts b/apps/sim/lib/webhooks/providers/telegram.ts index c74dc4d42c..3cf3f93829 100644 --- a/apps/sim/lib/webhooks/providers/telegram.ts +++ b/apps/sim/lib/webhooks/providers/telegram.ts @@ -3,7 +3,9 @@ import { createLogger } from '@sim/logger' import { safeCompare } from '@sim/security/compare' import { generateId } from '@sim/utils/id' import { and, eq, isNull, ne } from 'drizzle-orm' +import * as ipaddr from 'ipaddr.js' import { NextResponse } from 'next/server' +import { getClientIp } from '@/lib/core/utils/request' import { getNotificationUrl, getProviderConfig } from '@/lib/webhooks/provider-subscription-utils' import type { AuthContext, @@ -17,28 +19,61 @@ import type { const logger = createLogger('WebhookProvider:Telegram') +/** + * Telegram's published source ranges for webhook delivery. + * @see https://core.telegram.org/bots/webhooks + */ +const TELEGRAM_WEBHOOK_CIDRS: ReadonlyArray = [ + ['149.154.160.0', 20], + ['91.108.4.0', 22], +] as const + +/** Whether a client IP falls inside Telegram's documented webhook source ranges. */ +function isTelegramWebhookIp(ip: string): boolean { + if (!ip || ip === 'unknown' || !ipaddr.isValid(ip)) { + return false + } + try { + const addr = ipaddr.process(ip) + if (addr.kind() !== 'ipv4') { + return false + } + return TELEGRAM_WEBHOOK_CIDRS.some(([network, prefix]) => + addr.match([ipaddr.parse(network), prefix]) + ) + } catch { + return false + } +} + export const telegramHandler: WebhookProviderHandler = { verifyAuth({ request, requestId, providerConfig }: AuthContext): NextResponse | null { const secretToken = (providerConfig.secretToken as string | undefined)?.trim() - if (!secretToken) { - logger.warn( - `[${requestId}] Telegram webhook missing secretToken in providerConfig — rejecting request. Re-save the trigger so a secret token can be registered with Telegram.` - ) - return new NextResponse( - 'Unauthorized - Telegram webhook secret token is not configured. Re-save the trigger so a webhook can be registered.', - { status: 401 } - ) - } - const providedToken = request.headers.get('x-telegram-bot-api-secret-token') - if (!providedToken) { - logger.warn(`[${requestId}] Telegram webhook missing secret token header — rejecting request`) - return new NextResponse('Unauthorized - Missing Telegram secret token', { status: 401 }) + if (secretToken) { + const providedToken = request.headers.get('x-telegram-bot-api-secret-token') + if (!providedToken) { + logger.warn( + `[${requestId}] Telegram webhook missing secret token header — rejecting request` + ) + return new NextResponse('Unauthorized - Missing Telegram secret token', { status: 401 }) + } + + if (!safeCompare(providedToken, secretToken)) { + logger.warn(`[${requestId}] Telegram secret token verification failed`) + return new NextResponse('Unauthorized - Invalid Telegram secret token', { status: 401 }) + } + + return null } - if (!safeCompare(providedToken, secretToken)) { - logger.warn(`[${requestId}] Telegram secret token verification failed`) - return new NextResponse('Unauthorized - Invalid Telegram secret token', { status: 401 }) + const clientIp = getClientIp(request) + if (!isTelegramWebhookIp(clientIp)) { + logger.warn( + `[${requestId}] Telegram webhook without a registered secret token rejected — source IP is not in Telegram's published ranges. Re-save the trigger to enable secret-token verification.`, + { clientIp } + ) + return new NextResponse('Unauthorized - Untrusted Telegram webhook source', { status: 401 }) } return null From 536af73cc5604cae593b924c3fb7a9f31b9c81ad Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 09:13:18 -0700 Subject: [PATCH 24/36] fix(chat): restore constant-time password auth and IP rate limit A billing commit (ac565253a4) reverted the public-chat auth hardening as collateral, leaving HEAD with a timing-oracle password comparison (password !== decrypted) and no per-IP brute-force rate limit. Restore safeCompare and the password-attempt rate limiter, and re-add the 429 test. --- apps/sim/app/api/chat/utils.test.ts | 35 +++++++++++++++++++++++++++ apps/sim/app/api/chat/utils.ts | 37 +++++++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/apps/sim/app/api/chat/utils.test.ts b/apps/sim/app/api/chat/utils.test.ts index 86e46340a9..d12867a1d3 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: vi.fn().mockImplementation(() => ({ + 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 5a3d0750e8..70e4a657ac 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' } } From 9b246bae88b8a00f24525a05b88e758bfa32e7b2 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 09:18:27 -0700 Subject: [PATCH 25/36] revert(webhooks): undo trigger auth hardening pending compat plan Reverts the Telegram inbound-token verification (3ed97a440b, 41f133a9d7) and the HMAC fail-closed change (5b6cae9120). Production data shows ~79 live webhooks have no signing secret configured (63 GitHub, 9 Fireflies, 3 Jira, 2 Circleback, 1 Confluence, 1 Cal.com), so failing closed would 401 them. Restoring fail-open behavior until a backwards-compatible rollout (grandfather existing secretless webhooks / migration) is designed. Other security fixes on this branch are unaffected. --- .../app/api/webhooks/trigger/[path]/route.ts | 10 +- apps/sim/lib/webhooks/providers/github.ts | 7 +- apps/sim/lib/webhooks/providers/intercom.ts | 7 +- .../lib/webhooks/providers/telegram.test.ts | 93 ------------------- apps/sim/lib/webhooks/providers/telegram.ts | 69 ++------------ apps/sim/lib/webhooks/providers/utils.ts | 11 +-- apps/sim/triggers/circleback/webhook.ts | 5 +- apps/sim/triggers/confluence/utils.ts | 2 +- .../fireflies/transcription_complete.ts | 3 +- apps/sim/triggers/github/webhook.ts | 3 +- apps/sim/triggers/greenhouse/utils.ts | 8 +- apps/sim/triggers/jira/webhook.ts | 3 +- apps/sim/triggers/jsm/utils.ts | 3 +- apps/sim/triggers/typeform/webhook.ts | 4 +- 14 files changed, 27 insertions(+), 201 deletions(-) delete mode 100644 apps/sim/lib/webhooks/providers/telegram.test.ts diff --git a/apps/sim/app/api/webhooks/trigger/[path]/route.ts b/apps/sim/app/api/webhooks/trigger/[path]/route.ts index 76f789e6c1..73e71b1f3d 100644 --- a/apps/sim/app/api/webhooks/trigger/[path]/route.ts +++ b/apps/sim/app/api/webhooks/trigger/[path]/route.ts @@ -111,11 +111,6 @@ async function handleWebhookPost( const responses: NextResponse[] = [] for (const { webhook: foundWebhook, workflow: foundWorkflow } of webhooksForPath) { - const reachabilityResponse = handleProviderReachabilityTest(foundWebhook, body, requestId) - if (reachabilityResponse) { - return reachabilityResponse - } - const authError = await verifyProviderAuth( foundWebhook, foundWorkflow, @@ -131,6 +126,11 @@ async function handleWebhookPost( return authError } + const reachabilityResponse = handleProviderReachabilityTest(foundWebhook, body, requestId) + if (reachabilityResponse) { + return reachabilityResponse + } + const preprocessResult = await checkWebhookPreprocessing(foundWorkflow, foundWebhook, requestId) if (preprocessResult.error) { if (webhooksForPath.length > 1) { diff --git a/apps/sim/lib/webhooks/providers/github.ts b/apps/sim/lib/webhooks/providers/github.ts index 0730f28e27..28c0a782e1 100644 --- a/apps/sim/lib/webhooks/providers/github.ts +++ b/apps/sim/lib/webhooks/providers/github.ts @@ -48,12 +48,7 @@ export const githubHandler: WebhookProviderHandler = { verifyAuth({ request, rawBody, requestId, providerConfig }: AuthContext) { const secret = providerConfig.webhookSecret as string | undefined if (!secret) { - logger.warn( - `[${requestId}] GitHub webhook missing webhookSecret in providerConfig — rejecting request` - ) - return new NextResponse('Unauthorized - GitHub signing secret not configured', { - status: 401, - }) + return null } const signature = diff --git a/apps/sim/lib/webhooks/providers/intercom.ts b/apps/sim/lib/webhooks/providers/intercom.ts index 119e57107a..61214df8cd 100644 --- a/apps/sim/lib/webhooks/providers/intercom.ts +++ b/apps/sim/lib/webhooks/providers/intercom.ts @@ -49,12 +49,7 @@ export const intercomHandler: WebhookProviderHandler = { verifyAuth({ request, rawBody, requestId, providerConfig }: AuthContext) { const secret = providerConfig.webhookSecret as string | undefined if (!secret) { - logger.warn( - `[${requestId}] Intercom webhook missing webhookSecret in providerConfig — rejecting request` - ) - return new NextResponse('Unauthorized - Intercom signing secret not configured', { - status: 401, - }) + return null } const signature = request.headers.get('X-Hub-Signature') diff --git a/apps/sim/lib/webhooks/providers/telegram.test.ts b/apps/sim/lib/webhooks/providers/telegram.test.ts deleted file mode 100644 index 6809deb572..0000000000 --- a/apps/sim/lib/webhooks/providers/telegram.test.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { requestUtilsMockFns } from '@sim/testing' -import { NextRequest } from 'next/server' -import { beforeEach, describe, expect, it } from 'vitest' -import { telegramHandler } from '@/lib/webhooks/providers/telegram' - -function reqWithHeaders(headers: Record): NextRequest { - return new NextRequest('http://localhost/test', { headers }) -} - -describe('Telegram webhook provider', () => { - beforeEach(() => { - requestUtilsMockFns.mockGetClientIp.mockReturnValue('203.0.113.7') - }) - - it('verifyAuth rejects an unconfigured webhook when the source IP is not Telegram', () => { - const res = telegramHandler.verifyAuth!({ - request: reqWithHeaders({ 'x-telegram-bot-api-secret-token': 'anything' }), - rawBody: '{}', - requestId: 't1', - providerConfig: {}, - webhook: {}, - workflow: {}, - }) - expect((res as { status?: number })?.status).toBe(401) - }) - - it('verifyAuth accepts a legacy webhook (no secret) from a Telegram source IP', () => { - requestUtilsMockFns.mockGetClientIp.mockReturnValue('149.154.167.197') - const res = telegramHandler.verifyAuth!({ - request: reqWithHeaders({}), - rawBody: '{}', - requestId: 't1b', - providerConfig: {}, - webhook: {}, - workflow: {}, - }) - expect(res).toBeNull() - }) - - it('verifyAuth rejects a legacy webhook (no secret) when the source IP is unknown', () => { - requestUtilsMockFns.mockGetClientIp.mockReturnValue('unknown') - const res = telegramHandler.verifyAuth!({ - request: reqWithHeaders({}), - rawBody: '{}', - requestId: 't1c', - providerConfig: {}, - webhook: {}, - workflow: {}, - }) - expect((res as { status?: number })?.status).toBe(401) - }) - - it('verifyAuth rejects when the secret token header is missing', () => { - const res = telegramHandler.verifyAuth!({ - request: reqWithHeaders({}), - rawBody: '{}', - requestId: 't2', - providerConfig: { secretToken: 'super-secret' }, - webhook: {}, - workflow: {}, - }) - expect((res as { status?: number })?.status).toBe(401) - }) - - it('verifyAuth rejects when the secret token does not match', () => { - const res = telegramHandler.verifyAuth!({ - request: reqWithHeaders({ 'x-telegram-bot-api-secret-token': 'wrong' }), - rawBody: '{}', - requestId: 't3', - providerConfig: { secretToken: 'super-secret' }, - webhook: {}, - workflow: {}, - }) - expect((res as { status?: number })?.status).toBe(401) - }) - - it('verifyAuth accepts a matching secret token', () => { - const res = telegramHandler.verifyAuth!({ - request: reqWithHeaders({ 'x-telegram-bot-api-secret-token': 'super-secret' }), - rawBody: '{}', - requestId: 't4', - providerConfig: { secretToken: 'super-secret' }, - webhook: {}, - workflow: {}, - }) - expect(res).toBeNull() - }) - - it('extractIdempotencyId keys on update_id', () => { - expect(telegramHandler.extractIdempotencyId!({ update_id: 42 })).toBe('telegram:42') - expect(telegramHandler.extractIdempotencyId!({})).toBeNull() - }) -}) diff --git a/apps/sim/lib/webhooks/providers/telegram.ts b/apps/sim/lib/webhooks/providers/telegram.ts index 3cf3f93829..9720bccd60 100644 --- a/apps/sim/lib/webhooks/providers/telegram.ts +++ b/apps/sim/lib/webhooks/providers/telegram.ts @@ -1,11 +1,6 @@ import { db, webhook, workflowDeploymentVersion } from '@sim/db' import { createLogger } from '@sim/logger' -import { safeCompare } from '@sim/security/compare' -import { generateId } from '@sim/utils/id' import { and, eq, isNull, ne } from 'drizzle-orm' -import * as ipaddr from 'ipaddr.js' -import { NextResponse } from 'next/server' -import { getClientIp } from '@/lib/core/utils/request' import { getNotificationUrl, getProviderConfig } from '@/lib/webhooks/provider-subscription-utils' import type { AuthContext, @@ -19,63 +14,14 @@ import type { const logger = createLogger('WebhookProvider:Telegram') -/** - * Telegram's published source ranges for webhook delivery. - * @see https://core.telegram.org/bots/webhooks - */ -const TELEGRAM_WEBHOOK_CIDRS: ReadonlyArray = [ - ['149.154.160.0', 20], - ['91.108.4.0', 22], -] as const - -/** Whether a client IP falls inside Telegram's documented webhook source ranges. */ -function isTelegramWebhookIp(ip: string): boolean { - if (!ip || ip === 'unknown' || !ipaddr.isValid(ip)) { - return false - } - try { - const addr = ipaddr.process(ip) - if (addr.kind() !== 'ipv4') { - return false - } - return TELEGRAM_WEBHOOK_CIDRS.some(([network, prefix]) => - addr.match([ipaddr.parse(network), prefix]) - ) - } catch { - return false - } -} - export const telegramHandler: WebhookProviderHandler = { - verifyAuth({ request, requestId, providerConfig }: AuthContext): NextResponse | null { - const secretToken = (providerConfig.secretToken as string | undefined)?.trim() - - if (secretToken) { - const providedToken = request.headers.get('x-telegram-bot-api-secret-token') - if (!providedToken) { - logger.warn( - `[${requestId}] Telegram webhook missing secret token header — rejecting request` - ) - return new NextResponse('Unauthorized - Missing Telegram secret token', { status: 401 }) - } - - if (!safeCompare(providedToken, secretToken)) { - logger.warn(`[${requestId}] Telegram secret token verification failed`) - return new NextResponse('Unauthorized - Invalid Telegram secret token', { status: 401 }) - } - - return null - } - - const clientIp = getClientIp(request) - if (!isTelegramWebhookIp(clientIp)) { + verifyAuth({ request, requestId }: AuthContext) { + const userAgent = request.headers.get('user-agent') + if (!userAgent) { logger.warn( - `[${requestId}] Telegram webhook without a registered secret token rejected — source IP is not in Telegram's published ranges. Re-save the trigger to enable secret-token verification.`, - { clientIp } + `[${requestId}] Telegram webhook request has empty User-Agent header. This may be blocked by middleware.` ) - return new NextResponse('Unauthorized - Untrusted Telegram webhook source', { status: 401 }) } - return null }, @@ -179,9 +125,6 @@ export const telegramHandler: WebhookProviderHandler = { const notificationUrl = getNotificationUrl(ctx.webhook) const telegramApiUrl = `https://api.telegram.org/bot${botToken}/setWebhook` - const existingSecretToken = (config.secretToken as string | undefined)?.trim() - const secretToken = existingSecretToken || generateId() - try { const telegramResponse = await fetch(telegramApiUrl, { method: 'POST', @@ -189,7 +132,7 @@ export const telegramHandler: WebhookProviderHandler = { 'Content-Type': 'application/json', 'User-Agent': 'TelegramBot/1.0', }, - body: JSON.stringify({ url: notificationUrl, secret_token: secretToken }), + body: JSON.stringify({ url: notificationUrl }), }) const responseBody = await telegramResponse.json() @@ -213,7 +156,7 @@ export const telegramHandler: WebhookProviderHandler = { logger.info( `[${ctx.requestId}] Successfully created Telegram webhook for webhook ${ctx.webhook.id}` ) - return { providerConfigUpdates: { secretToken } } + return {} } catch (error: unknown) { if ( error instanceof Error && diff --git a/apps/sim/lib/webhooks/providers/utils.ts b/apps/sim/lib/webhooks/providers/utils.ts index 0e13c9b000..dff3db7ce1 100644 --- a/apps/sim/lib/webhooks/providers/utils.ts +++ b/apps/sim/lib/webhooks/providers/utils.ts @@ -16,10 +16,6 @@ interface HmacVerifierOptions { /** * Factory that creates a `verifyAuth` implementation for HMAC-signature-based providers. * Covers the common pattern: get secret → check header → validate signature → return 401 or null. - * - * Fails closed: when no signing secret is configured the request is rejected (401), matching - * Stripe/WhatsApp/Vercel. A signed-provider webhook with no secret would otherwise accept any - * unauthenticated body that knows the URL, downgrading the provider's mandatory signature check. */ export function createHmacVerifier({ configKey, @@ -35,12 +31,7 @@ export function createHmacVerifier({ }: AuthContext): Promise => { const secret = providerConfig[configKey] as string | undefined if (!secret) { - logger.warn( - `[${requestId}] ${providerLabel} webhook missing signing secret in providerConfig — rejecting request` - ) - return new NextResponse(`Unauthorized - ${providerLabel} signing secret not configured`, { - status: 401, - }) + return null } const signature = request.headers.get(headerName) diff --git a/apps/sim/triggers/circleback/webhook.ts b/apps/sim/triggers/circleback/webhook.ts index 1553a855ca..8fc8fe1af3 100644 --- a/apps/sim/triggers/circleback/webhook.ts +++ b/apps/sim/triggers/circleback/webhook.ts @@ -38,9 +38,8 @@ export const circlebackWebhookTrigger: TriggerConfig = { id: 'webhookSecret', title: 'Signing Secret', type: 'short-input', - placeholder: 'Paste signing secret from Circleback', - description: - 'Validates that webhook deliveries originate from Circleback using HMAC-SHA256. Required: deliveries are rejected until this is set.', + placeholder: 'Paste signing secret from Circleback (optional)', + description: 'Validates that webhook deliveries originate from Circleback using HMAC-SHA256.', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/confluence/utils.ts b/apps/sim/triggers/confluence/utils.ts index 54f298239b..cc722457a9 100644 --- a/apps/sim/triggers/confluence/utils.ts +++ b/apps/sim/triggers/confluence/utils.ts @@ -54,7 +54,7 @@ export function buildConfluenceExtraFields(triggerId: string): SubBlockConfig[] type: 'short-input', placeholder: 'Enter a strong secret', description: - 'Secret to validate webhook deliveries from Confluence using HMAC signature. Required: deliveries are rejected until this is set.', + 'Optional secret to validate webhook deliveries from Confluence using HMAC signature', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/fireflies/transcription_complete.ts b/apps/sim/triggers/fireflies/transcription_complete.ts index ea55f92991..49d3e1fe52 100644 --- a/apps/sim/triggers/fireflies/transcription_complete.ts +++ b/apps/sim/triggers/fireflies/transcription_complete.ts @@ -25,8 +25,7 @@ export const firefliesTranscriptionCompleteTrigger: TriggerConfig = { title: 'Webhook Secret', type: 'short-input', placeholder: 'Enter your 16-32 character secret', - description: - 'Secret key for HMAC signature verification (set in Fireflies dashboard). Required: deliveries are rejected until this is set.', + description: 'Secret key for HMAC signature verification (set in Fireflies dashboard)', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/github/webhook.ts b/apps/sim/triggers/github/webhook.ts index 9c1aeacd9b..5b1b4c630e 100644 --- a/apps/sim/triggers/github/webhook.ts +++ b/apps/sim/triggers/github/webhook.ts @@ -46,8 +46,7 @@ export const githubWebhookTrigger: TriggerConfig = { title: 'Webhook Secret', type: 'short-input', placeholder: 'Generate or enter a strong secret', - description: - 'Validates that webhook deliveries originate from GitHub. Required: deliveries are rejected until this is set.', + description: 'Validates that webhook deliveries originate from GitHub.', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/greenhouse/utils.ts b/apps/sim/triggers/greenhouse/utils.ts index 93204ba3c0..8210761dc8 100644 --- a/apps/sim/triggers/greenhouse/utils.ts +++ b/apps/sim/triggers/greenhouse/utils.ts @@ -62,17 +62,17 @@ export function isGreenhouseEventMatch(triggerId: string, action: string): boole /** * Builds extra fields for Greenhouse triggers. - * Includes the secret key used for HMAC signature verification. + * Includes an optional secret key for HMAC signature verification. */ export function buildGreenhouseExtraFields(triggerId: string): SubBlockConfig[] { return [ { id: 'secretKey', - title: 'Secret Key', + title: 'Secret Key (Optional)', type: 'short-input', placeholder: 'Enter the same secret key configured in Greenhouse', description: - 'Used to verify the HMAC-SHA256 Signature header. Required: deliveries are rejected until a secret key is configured here and in Greenhouse.', + 'When set, requests must include a valid Signature header (HMAC-SHA256). If left empty, the endpoint does not verify signatures—only use on a private URL you fully control.', password: true, mode: 'trigger', condition: { field: 'selectedTriggerId', value: triggerId }, @@ -101,7 +101,7 @@ export function greenhouseSetupInstructions(eventType: string): string { 'In Greenhouse, go to Configure > Dev Center > Webhooks.', 'Click Create New Webhook.', 'Paste the Webhook URL into the Endpoint URL field.', - 'Enter a Secret Key for HMAC signature verification. This is required — deliveries without a valid signature are rejected.', + 'Enter a Secret Key for HMAC signature verification (recommended). Leave empty only if you accept unauthenticated POSTs to this URL.', `Under When, select the appropriate ${eventType}.`, 'Click Create Webhook to save.', 'Click "Save" above to activate your trigger.', diff --git a/apps/sim/triggers/jira/webhook.ts b/apps/sim/triggers/jira/webhook.ts index b1ac9dc531..57c9475193 100644 --- a/apps/sim/triggers/jira/webhook.ts +++ b/apps/sim/triggers/jira/webhook.ts @@ -34,8 +34,7 @@ export const jiraWebhookTrigger: TriggerConfig = { title: 'Webhook Secret', type: 'short-input', placeholder: 'Enter a strong secret', - description: - 'Secret to validate webhook deliveries from Jira using HMAC signature. Required: deliveries are rejected until this is set.', + description: 'Optional secret to validate webhook deliveries from Jira using HMAC signature', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/jsm/utils.ts b/apps/sim/triggers/jsm/utils.ts index 24e037206d..984784bf9c 100644 --- a/apps/sim/triggers/jsm/utils.ts +++ b/apps/sim/triggers/jsm/utils.ts @@ -49,8 +49,7 @@ function jsmWebhookSecretField(triggerId: string): SubBlockConfig { title: 'Webhook Secret', type: 'short-input', placeholder: 'Enter a strong secret', - description: - 'Secret to validate webhook deliveries from Jira using HMAC signature. Required: deliveries are rejected until this is set.', + description: 'Optional secret to validate webhook deliveries from Jira using HMAC signature', password: true, required: false, mode: 'trigger', diff --git a/apps/sim/triggers/typeform/webhook.ts b/apps/sim/triggers/typeform/webhook.ts index 29bcdd97ad..53bec744e3 100644 --- a/apps/sim/triggers/typeform/webhook.ts +++ b/apps/sim/triggers/typeform/webhook.ts @@ -45,9 +45,9 @@ export const typeformWebhookTrigger: TriggerConfig = { id: 'secret', title: 'Webhook Secret', type: 'short-input', - placeholder: 'Enter a secret for webhook signature verification', + placeholder: 'Enter a secret for webhook signature verification (optional)', description: - 'A secret string used to verify webhook authenticity. Required: deliveries are rejected until this is set. Generate a secure random string (min 20 characters recommended).', + 'A secret string used to verify webhook authenticity. Highly recommended for security. Generate a secure random string (min 20 characters recommended).', password: true, required: false, mode: 'trigger', From 8e3984cecd2091a2d4a45f4fe093eea40c1b8cd0 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 09:42:39 -0700 Subject: [PATCH 26/36] test(chat): make RateLimiter mock a constructable class The arrow-function mockImplementation form was not reliably constructable in the full suite run (`new RateLimiter()` threw "is not a constructor"), though it passed in isolation. Switch to the class-based mock used by the sibling OTP/speech route tests. --- apps/sim/app/api/chat/utils.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/chat/utils.test.ts b/apps/sim/app/api/chat/utils.test.ts index d12867a1d3..337310d630 100644 --- a/apps/sim/app/api/chat/utils.test.ts +++ b/apps/sim/app/api/chat/utils.test.ts @@ -31,9 +31,9 @@ const { })) vi.mock('@/lib/core/rate-limiter', () => ({ - RateLimiter: vi.fn().mockImplementation(() => ({ - checkRateLimitDirect: mockCheckRateLimitDirect, - })), + RateLimiter: class { + checkRateLimitDirect = mockCheckRateLimitDirect + }, })) vi.mock('@/lib/auth', () => ({ From b9d004a3fd28f5e075137051aae62694c0a54d1f Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 09:58:22 -0700 Subject: [PATCH 27/36] fix(billing): release admission slot on pre-execution aborts; cluster-safe release Addresses PR review on the usage-cap admission reservation: - Slot leak: the reservation taken at the end of preprocessing was only released when the LoggingSession finalized. The execute route's pre-execution exits (client cancel, workspace/API-key guards) returned without finalizing a session, leaking the slot until its TTL and wrongly throttling later runs. Release explicitly on those paths; executions that start are still released via session finalization. - Release is now cluster-safe: replaced the Lua script that rebuilt the in-flight key from the pointer value (a key not declared in KEYS, which silently breaks Redis Cluster slot routing) with discrete single-key GETDEL + ZREM commands. --- .../app/api/workflows/[id]/execute/route.ts | 9 ++++++ .../calculations/usage-reservation.test.ts | 29 ++++++++++++++----- .../billing/calculations/usage-reservation.ts | 24 +++++++-------- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index 800d4bd687..c9820a4c88 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 { @@ -703,7 +704,11 @@ async function handleExecutePost( ) } + // Preprocessing reserved an admission slot (released when the LoggingSession + // finalizes). Any path that exits before execution starts must release it + // here, or the slot leaks until its TTL and wrongly throttles later runs. if (req.signal.aborted) { + await releaseExecutionSlot(executionId) return clientCancelledResponse() } @@ -712,12 +717,14 @@ async function handleExecutePost( if (!workflow.workspaceId) { reqLogger.error('Workflow has no workspaceId') + await releaseExecutionSlot(executionId) return NextResponse.json({ error: 'Workflow has no associated workspace' }, { status: 500 }) } const workspaceId = workflow.workspaceId reqLogger = reqLogger.withMetadata({ workspaceId, userId: actorUserId }) if (auth.apiKeyType === 'workspace' && auth.workspaceId !== workspaceId) { + await releaseExecutionSlot(executionId) return NextResponse.json( { error: 'API key is not authorized for this workspace' }, { status: 403 } @@ -751,6 +758,7 @@ async function handleExecutePost( let processedInput = input try { if (req.signal.aborted) { + await releaseExecutionSlot(executionId) return clientCancelledResponse() } const workflowData = shouldUseDraftState @@ -758,6 +766,7 @@ async function handleExecutePost( : await loadDeployedWorkflowState(workflowId, workspaceId) if (req.signal.aborted) { + await releaseExecutionSlot(executionId) return clientCancelledResponse() } diff --git a/apps/sim/lib/billing/calculations/usage-reservation.test.ts b/apps/sim/lib/billing/calculations/usage-reservation.test.ts index f610d930ad..a6435aff69 100644 --- a/apps/sim/lib/billing/calculations/usage-reservation.test.ts +++ b/apps/sim/lib/billing/calculations/usage-reservation.test.ts @@ -24,7 +24,9 @@ import { } from '@/lib/billing/calculations/usage-reservation' const evalMock = vi.fn() -const fakeRedis = { eval: evalMock } +const getdelMock = vi.fn() +const zremMock = vi.fn() +const fakeRedis = { eval: evalMock, getdel: getdelMock, zrem: zremMock } const baseParams = { userId: 'user-1', @@ -117,22 +119,33 @@ describe('usage-reservation', () => { }) describe('releaseExecutionSlot', () => { - it('runs the release script for the execution pointer', async () => { - evalMock.mockResolvedValueOnce(1) + it('reads the pointer then removes the slot from that entity set', async () => { + getdelMock.mockResolvedValueOnce('org:org-9') await releaseExecutionSlot('exec-1') - const args = evalMock.mock.calls[0] - expect(args[2]).toBe('usage:reservation:exec-1') - expect(args[3]).toBe('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(evalMock).not.toHaveBeenCalled() + expect(getdelMock).not.toHaveBeenCalled() }) it('swallows release errors', async () => { - evalMock.mockRejectedValueOnce(new Error('boom')) + 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 index 5ef1f5d798..c448c88776 100644 --- a/apps/sim/lib/billing/calculations/usage-reservation.ts +++ b/apps/sim/lib/billing/calculations/usage-reservation.ts @@ -77,19 +77,6 @@ redis.call('SET', KEYS[2], ARGV[6], 'PX', pttl) return 1 ` -/** - * Release a previously reserved slot. Idempotent: a no-op when the pointer is - * absent (already released or expired). The in-flight set key is rebuilt from - * the pointer value with a fixed prefix (single-instance Redis). - */ -const RELEASE_SCRIPT = ` -local entity = redis.call('GET', KEYS[1]) -if not entity then return 0 end -redis.call('DEL', KEYS[1]) -redis.call('ZREM', '${INFLIGHT_KEY_PREFIX}' .. entity, ARGV[1]) -return 1 -` - /** * Stable per-entity reservation key. Org-scoped subscriptions reserve against * the organization's pooled cap; everyone else against their personal cap — @@ -192,6 +179,11 @@ export async function reserveExecutionSlot( * 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) { @@ -204,7 +196,11 @@ export async function releaseExecutionSlot(executionId: string): Promise { } try { - await redis.eval(RELEASE_SCRIPT, 1, `${POINTER_KEY_PREFIX}${executionId}`, executionId) + 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, From b2322294b4f090bec62e308d720aa60e4f1ffffe Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 09:58:37 -0700 Subject: [PATCH 28/36] improvement(files): log missing owner metadata distinctly on profile-picture delete deny MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per PR review: when a profile-picture delete is denied, distinguish a missing owner record (no userId metadata) from a genuine ownership mismatch so the fail-closed denial is diagnosable. Behavior unchanged — both still deny. --- apps/sim/app/api/files/authorization.ts | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/apps/sim/app/api/files/authorization.ts b/apps/sim/app/api/files/authorization.ts index 0d4a756162..7b96031baf 100644 --- a/apps/sim/app/api/files/authorization.ts +++ b/apps/sim/app/api/files/authorization.ts @@ -315,11 +315,24 @@ async function verifyPublicAssetWriteAccess( if (metadata.userId && metadata.userId === userId) { return true } - logger.warn('profile-pictures delete denied: caller does not own the file', { - userId, - fileUserId: metadata.userId, - cloudKey, - }) + // 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 } From cbfe114afa25b80917842858c983e58586432356 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 10:08:11 -0700 Subject: [PATCH 29/36] fix(billing): release admission slot when async enqueue fails If queueing the background workflow job throws, no job runs and no LoggingSession finalizes, so the admission slot reserved during preprocessing would leak until its TTL. Release it before returning 500. --- apps/sim/app/api/workflows/[id]/execute/route.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index c9820a4c88..23f5c204bf 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.ts @@ -329,6 +329,9 @@ async function handleAsyncExecution(params: AsyncExecutionParams): Promise Date: Wed, 10 Jun 2026 10:14:59 -0700 Subject: [PATCH 30/36] fix(api): make body-size caps NaN-safe and raise chat input/attachment limits - DEFAULT_MAX_JSON_BODY_BYTES and CHAT_MAX_REQUEST_BYTES now fall back to hardcoded defaults (50 MB / 220 MB) when the env value is missing or non-numeric, so a misconfig can't silently produce a NaN cap that never rejects. - Raise CHAT_MAX_REQUEST_BYTES default to 220 MB to cover 15 base64 file attachments, and MAX_CHAT_INPUT_CHARS to 1,000,000. - Minor: tidy use-inline-rename onSave type; drop two redundant test comments. --- apps/sim/app/api/chat/[identifier]/route.ts | 2 +- apps/sim/app/api/files/authorization.test.ts | 1 - apps/sim/app/api/files/delete/route.test.ts | 1 - apps/sim/hooks/use-inline-rename.ts | 2 +- apps/sim/lib/api/contracts/chats.ts | 2 +- apps/sim/lib/api/server/validation.ts | 5 ++++- apps/sim/lib/core/config/env.ts | 2 +- 7 files changed, 8 insertions(+), 7 deletions(-) diff --git a/apps/sim/app/api/chat/[identifier]/route.ts b/apps/sim/app/api/chat/[identifier]/route.ts index 9f69975510..aca5479999 100644 --- a/apps/sim/app/api/chat/[identifier]/route.ts +++ b/apps/sim/app/api/chat/[identifier]/route.ts @@ -42,7 +42,7 @@ 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) +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 }> }) => { diff --git a/apps/sim/app/api/files/authorization.test.ts b/apps/sim/app/api/files/authorization.test.ts index ce92c16191..4409108eac 100644 --- a/apps/sim/app/api/files/authorization.test.ts +++ b/apps/sim/app/api/files/authorization.test.ts @@ -177,7 +177,6 @@ describe('public-context access (profile-pictures / og-images / workspace-logos) }) it('denies a cross-tenant delete that names a workspace key under a public context', async () => { - // Reported attack: a workspace key passed under a public context to dodge the ownership check. await expect(write('workspace/victim-ws/123-report.pdf', 'og-images')).resolves.toBe(false) expect(mockGetUserEntityPermissions).not.toHaveBeenCalled() }) diff --git a/apps/sim/app/api/files/delete/route.test.ts b/apps/sim/app/api/files/delete/route.test.ts index 5b2f88dcff..d843b57365 100644 --- a/apps/sim/app/api/files/delete/route.test.ts +++ b/apps/sim/app/api/files/delete/route.test.ts @@ -200,7 +200,6 @@ describe('File Delete API Route', () => { }) it('rejects a client context that disagrees with the key prefix', async () => { - // Reported attack: a workspace key passed under a public context to dodge the ownership check. const req = createMockRequest('POST', { filePath: '/api/files/serve/s3/workspace/victim-ws/1234-report.pdf', context: 'og-images', diff --git a/apps/sim/hooks/use-inline-rename.ts b/apps/sim/hooks/use-inline-rename.ts index 87c819e1a3..5f0cba4fab 100644 --- a/apps/sim/hooks/use-inline-rename.ts +++ b/apps/sim/hooks/use-inline-rename.ts @@ -9,7 +9,7 @@ interface UseInlineRenameProps { * `mutateAsync(...)`) — NOT a fire-and-forget `mutate(...)` — so `isSaving` * spans the in-flight request and a rejection can revive the edit session. */ - onSave: (id: string, newName: string) => void | Promise + onSave: (id: string, newName: string) => undefined | Promise } /** diff --git a/apps/sim/lib/api/contracts/chats.ts b/apps/sim/lib/api/contracts/chats.ts index 94777002f2..d0840bca8a 100644 --- a/apps/sim/lib/api/contracts/chats.ts +++ b/apps/sim/lib/api/contracts/chats.ts @@ -110,7 +110,7 @@ export const deployedChatAuthBodySchema = z.object({ }) export type DeployedChatAuthBody = z.input -const MAX_CHAT_INPUT_CHARS = 100_000 +const MAX_CHAT_INPUT_CHARS = 1_000_000 const MAX_CHAT_FILE_DATA_CHARS = 14 * 1024 * 1024 const MAX_CHAT_FILES = 15 diff --git a/apps/sim/lib/api/server/validation.ts b/apps/sim/lib/api/server/validation.ts index a312fbadbf..75cd5cbce8 100644 --- a/apps/sim/lib/api/server/validation.ts +++ b/apps/sim/lib/api/server/validation.ts @@ -20,8 +20,11 @@ import { * 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) +export const DEFAULT_MAX_JSON_BODY_BYTES = + Number.parseInt(env.API_MAX_JSON_BODY_BYTES, 10) || 50 * 1024 * 1024 export interface ValidationErrorBody { error: string diff --git a/apps/sim/lib/core/config/env.ts b/apps/sim/lib/core/config/env.ts index 0d93a8dfb6..87af1a75f3 100644 --- a/apps/sim/lib/core/config/env.ts +++ b/apps/sim/lib/core/config/env.ts @@ -244,7 +244,7 @@ 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('20971520'),// Max request body size for the public deployed-chat endpoint (20 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) From 7e012af0853881ab1c704985b534ddd616855605 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 10:28:25 -0700 Subject: [PATCH 31/36] fix(hooks): restore void return in useInlineRename onSave type A prior commit changed onSave's return type from `void | Promise` to `undefined | Promise`, which broke the build: callbacks that return nothing (table-grid column rename, table header rename) infer a `void` return, which is not assignable to `undefined`. Restore the `void` union so both fire-and-forget and Promise-returning callbacks type-check. --- apps/sim/hooks/use-inline-rename.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/sim/hooks/use-inline-rename.ts b/apps/sim/hooks/use-inline-rename.ts index 5f0cba4fab..87c819e1a3 100644 --- a/apps/sim/hooks/use-inline-rename.ts +++ b/apps/sim/hooks/use-inline-rename.ts @@ -9,7 +9,7 @@ interface UseInlineRenameProps { * `mutateAsync(...)`) — NOT a fire-and-forget `mutate(...)` — so `isSaving` * spans the in-flight request and a rejection can revive the edit session. */ - onSave: (id: string, newName: string) => undefined | Promise + onSave: (id: string, newName: string) => void | Promise } /** From 74a39b35e0557ce120998dc508adc1b35a0c2a86 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 10:56:16 -0700 Subject: [PATCH 32/36] fix(billing,api): release chat reservation slot on early exit; preserve 413 on oversized import - Chat route: preprocessExecution reserves a billing concurrency slot, but the post-preprocess early exits (missing workspaceId, execution-setup failure) returned without releasing it, leaking the slot until TTL and wrongly throttling later runs. Release explicitly on those paths (idempotent), mirroring the workflows execute route. - Admin import route: an oversized JSON body now returns the real 413 from parseJsonBody instead of being remapped to a 400; invalid JSON still 400s. --- apps/sim/app/api/chat/[identifier]/route.ts | 7 +++++++ apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/api/chat/[identifier]/route.ts b/apps/sim/app/api/chat/[identifier]/route.ts index aca5479999..330ece6885 100644 --- a/apps/sim/app/api/chat/[identifier]/route.ts +++ b/apps/sim/app/api/chat/[identifier]/route.ts @@ -6,6 +6,7 @@ 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' @@ -194,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) } @@ -300,6 +304,9 @@ 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) { 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 ad7780ef14..7bbf9a5ea0 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 @@ -97,7 +97,10 @@ export const POST = withRouteHandler( 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) From 79273365f8904c90693fee25ee0740aa9b136719 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 11:10:57 -0700 Subject: [PATCH 33/36] fix(icons): make Infisical icon black for contrast; regenerate docs The Infisical mark rendered near-white on its yellow block background and was barely visible; switch its fill from currentColor to #000000 (matching the hardcoded-fill pattern of sibling brand icons). Sync the docs icon copy and pick up a stale servicenow doc regeneration. --- apps/docs/components/icons.tsx | 2 +- apps/docs/content/docs/en/tools/servicenow.mdx | 1 - apps/sim/components/icons.tsx | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/docs/components/icons.tsx b/apps/docs/components/icons.tsx index 41732af3b2..a158ae1bb8 100644 --- a/apps/docs/components/icons.tsx +++ b/apps/docs/components/icons.tsx @@ -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 398e3fd6db..7337cbf636 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/components/icons.tsx b/apps/sim/components/icons.tsx index 41732af3b2..a158ae1bb8 100644 --- a/apps/sim/components/icons.tsx +++ b/apps/sim/components/icons.tsx @@ -4967,7 +4967,7 @@ export function InfisicalIcon(props: SVGProps) { ) From eaf73928f8f7ce361889fec9e76eaff70eaca846 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 11:17:43 -0700 Subject: [PATCH 34/36] fix(billing): release reserved slot on execute-route 503 and setup throw After preprocessExecution reserves a billing concurrency slot, the streaming path could exit without releasing it: the 503 return when initializeExecutionStreamMeta fails, and any throw during stream setup (caught by the outer handler, which only returned 500). Both left the slot held until TTL, wrongly throttling unrelated runs. Release on the 503 path and in the outer catch (executionId hoisted so the catch can see it; release is idempotent and a no-op when no slot was reserved). --- apps/sim/app/api/workflows/[id]/execute/route.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index 23f5c204bf..295517525e 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.ts @@ -378,6 +378,10 @@ async function handleExecutePost( } const callChain = buildNextCallChain(incomingCallChain, workflowId) + // Hoisted so the outer catch can release a reserved billing slot when a throw + // after preprocessExecution exits before the stream takes over its release. + let executionId = '' + try { const auth = await checkHybridAuth(req, { requireWorkflowId: false }) const isMcpBridgeRequest = @@ -637,8 +641,7 @@ async function handleExecutePost( ) } - const executionId = - isClientSession && requestedExecutionId ? requestedExecutionId : generateId() + executionId = isClientSession && requestedExecutionId ? requestedExecutionId : generateId() reqLogger = reqLogger.withMetadata({ userId, executionId }) reqLogger.info('Starting server-side execution', { @@ -1166,6 +1169,7 @@ async function handleExecutePost( }) if (!metaInitialized) { timeoutController.cleanup() + await releaseExecutionSlot(executionId) return NextResponse.json( { error: 'Run buffer temporarily unavailable' }, { status: 503, headers: { 'X-Execution-Id': executionId } } @@ -1724,6 +1728,9 @@ async function handleExecutePost( }) } catch (error: any) { reqLogger.error('Failed to start workflow execution:', error) + // Release a reserved billing slot if a throw exited before the stream took + // over its release (idempotent; no-op when never reserved). + if (executionId) await releaseExecutionSlot(executionId) return NextResponse.json( { error: error.message || 'Failed to start workflow execution' }, { status: 500 } From 897569897ee13521eeb597e9d5d4b895c28d5267 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 11:19:21 -0700 Subject: [PATCH 35/36] fix(icons): make Linkup icon black for contrast The Linkup mark rendered with currentColor (near-white on its block background); switch its fill to #000000 for legibility, matching the Infisical fix. Docs icon copy synced via generate-docs. --- apps/docs/components/icons.tsx | 2 +- apps/sim/components/icons.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/docs/components/icons.tsx b/apps/docs/components/icons.tsx index a158ae1bb8..bfd2ade63a 100644 --- a/apps/docs/components/icons.tsx +++ b/apps/docs/components/icons.tsx @@ -2492,7 +2492,7 @@ export function LinkupIcon(props: SVGProps) { ) diff --git a/apps/sim/components/icons.tsx b/apps/sim/components/icons.tsx index a158ae1bb8..bfd2ade63a 100644 --- a/apps/sim/components/icons.tsx +++ b/apps/sim/components/icons.tsx @@ -2492,7 +2492,7 @@ export function LinkupIcon(props: SVGProps) { ) From 95d724df47712d23a166afebc52097f57167c012 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 10 Jun 2026 11:31:27 -0700 Subject: [PATCH 36/36] fix(billing): release reserved slot if inline async job never starts In the inline (single-process) async path, if jobQueue.startJob threw before executeWorkflowJob ran, no LoggingSession finalized and the reserved billing slot was held until TTL. Release it in the fire-and-forget catch (idempotent; a no-op when the job already finalized and released). The queued-worker path and all in-job outcomes already release via the job's LoggingSession finalize. --- apps/sim/app/api/workflows/[id]/execute/route.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index 295517525e..c8e89c259b 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.ts @@ -304,6 +304,11 @@ async function handleAsyncExecution(params: AsyncExecutionParams): Promise