From 7a86adf6e37f0d68e85c547573be5171c2a83209 Mon Sep 17 00:00:00 2001 From: Bill Leoutsakos Date: Wed, 26 Aug 2026 12:18:47 -0700 Subject: [PATCH] fix(imap): resolve mailbox selector contexts server-side --- .../sim/app/api/tools/imap/mailboxes/route.ts | 65 +++++-- .../server-resolved-selector.test.ts | 182 ++++++++++++++++++ .../selectors/providers/imap/selectors.ts | 27 +-- .../imap/server-resolved-context.test.ts | 74 +++++++ .../tools/imap.server-resolved.test.ts | 53 +++++ apps/sim/lib/api/contracts/tools/imap.ts | 17 +- .../core/security/input-validation.server.ts | 27 +-- 7 files changed, 393 insertions(+), 52 deletions(-) create mode 100644 apps/sim/app/api/tools/imap/mailboxes/server-resolved-selector.test.ts create mode 100644 apps/sim/hooks/selectors/providers/imap/server-resolved-context.test.ts create mode 100644 apps/sim/lib/api/contracts/tools/imap.server-resolved.test.ts diff --git a/apps/sim/app/api/tools/imap/mailboxes/route.ts b/apps/sim/app/api/tools/imap/mailboxes/route.ts index b66c9eb34d4..e5200481c87 100644 --- a/apps/sim/app/api/tools/imap/mailboxes/route.ts +++ b/apps/sim/app/api/tools/imap/mailboxes/route.ts @@ -2,20 +2,25 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { ImapFlow } from 'imapflow' import { type NextRequest, NextResponse } from 'next/server' -import { imapMailboxesContract } from '@/lib/api/contracts/tools/imap' +import { + imapMailboxesContract, + resolvedImapMailboxesBodySchema, +} from '@/lib/api/contracts/tools/imap' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' -import { getSession } from '@/lib/auth' import { validateDatabaseHost } from '@/lib/core/security/input-validation.server' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { + authenticateSelectorRequest, + resolveAuthorizedSelectorContext, +} from '@/lib/selectors/server/resolve-authorized-context' const logger = createLogger('ImapMailboxesAPI') export const POST = withRouteHandler(async (request: NextRequest) => { - const session = await getSession() - if (!session?.user?.id) { - return NextResponse.json({ success: false, message: 'Unauthorized' }, { status: 401 }) + const authentication = await authenticateSelectorRequest(request) + if (!authentication.ok) { + return NextResponse.json({ error: authentication.error }, { status: authentication.status }) } - const parsed = await parseRequest( imapMailboxesContract, request, @@ -40,10 +45,29 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } ) if (!parsed.success) return parsed.response - const { host, port, secure, username, password } = parsed.data.body + const { workflowId, ...context } = parsed.data.body + + const resolution = await resolveAuthorizedSelectorContext(authentication.principal, { + workflowId, + context, + }) + if (!resolution.ok) { + return NextResponse.json( + { success: false, message: resolution.error }, + { status: resolution.status } + ) + } + const validated = resolvedImapMailboxesBodySchema.safeParse(resolution.context) + if (!validated.success) { + return NextResponse.json( + { success: false, message: 'Invalid IMAP selector configuration' }, + { status: 400 } + ) + } + const { host, port, secure, username, password } = validated.data try { - const hostValidation = await validateDatabaseHost(host, 'host') + const hostValidation = await validateDatabaseHost(host, 'host', { logFailureDetails: false }) if (!hostValidation.isValid) { return NextResponse.json({ success: false, message: hostValidation.error }, { status: 400 }) } @@ -94,17 +118,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => { throw error } } catch (error) { - const errorMessage = getErrorMessage(error, 'Unknown error') - logger.error('Error fetching IMAP mailboxes:', errorMessage) - - let userMessage = 'Failed to connect to IMAP server. Please check your connection settings.' - if ( - errorMessage.includes('AUTHENTICATIONFAILED') || - errorMessage.includes('Invalid credentials') - ) { - userMessage = 'Invalid username or password. For Gmail, use an App Password.' - } - - return NextResponse.json({ success: false, message: userMessage }, { status: 500 }) + logger.error('Error fetching IMAP mailboxes') + const errorMessage = getErrorMessage(error) + const userMessage = + errorMessage.includes('AUTHENTICATIONFAILED') || errorMessage.includes('Invalid credentials') + ? 'Invalid username or password. For Gmail, use an App Password.' + : 'Failed to connect to IMAP server. Please check your connection settings.' + return NextResponse.json( + { + success: false, + message: userMessage, + }, + { status: 500 } + ) } }) diff --git a/apps/sim/app/api/tools/imap/mailboxes/server-resolved-selector.test.ts b/apps/sim/app/api/tools/imap/mailboxes/server-resolved-selector.test.ts new file mode 100644 index 00000000000..d21c4c5bddf --- /dev/null +++ b/apps/sim/app/api/tools/imap/mailboxes/server-resolved-selector.test.ts @@ -0,0 +1,182 @@ +/** + * @vitest-environment node + */ +import { createMockRequest } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + authenticate: vi.fn(), + resolveContext: vi.fn(), + validateDatabaseHost: vi.fn(), + imapConstruct: vi.fn(), + imapConnect: vi.fn(), + imapList: vi.fn(), + imapLogout: vi.fn(), +})) + +vi.mock('@/lib/selectors/server/resolve-authorized-context', () => ({ + authenticateSelectorRequest: mocks.authenticate, + resolveAuthorizedSelectorContext: mocks.resolveContext, +})) + +vi.mock('@/lib/core/security/input-validation.server', () => ({ + validateDatabaseHost: mocks.validateDatabaseHost, +})) + +vi.mock('imapflow', () => ({ + ImapFlow: class MockImapFlow { + constructor(config: unknown) { + mocks.imapConstruct(config) + } + + connect = mocks.imapConnect + list = mocks.imapList + logout = mocks.imapLogout + }, +})) + +import { POST as listMailboxes } from '@/app/api/tools/imap/mailboxes/route' + +function request(body: unknown) { + return createMockRequest('POST', body, {}, 'http://localhost:3000/api/tools/imap/mailboxes') +} + +const wireBody = { + workflowId: 'workflow-1', + host: '{{IMAP_HOST}}', + port: '{{IMAP_PORT}}', + secure: '{{IMAP_TLS}}', + username: '{{IMAP_USERNAME}}', + password: '{{IMAP_PASSWORD}}', +} + +describe('server-resolved IMAP mailbox selector', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.authenticate.mockResolvedValue({ + ok: true, + principal: { kind: 'session', userId: 'viewer-1', sessionId: 'session-1' }, + }) + mocks.resolveContext.mockResolvedValue({ + ok: true, + context: { + host: 'imap.example.com', + port: '993', + secure: 'true', + username: 'mailbox@example.com', + password: 'resolved-password', + }, + requesterUserId: 'viewer-1', + workspaceId: 'workspace-1', + }) + mocks.validateDatabaseHost.mockResolvedValue({ + isValid: true, + resolvedIP: '203.0.113.10', + }) + mocks.imapConnect.mockResolvedValue(undefined) + mocks.imapList.mockResolvedValue([ + { path: 'Archive', name: 'Archive', delimiter: '/' }, + { path: 'INBOX', name: 'INBOX', delimiter: '/' }, + ]) + mocks.imapLogout.mockResolvedValue(undefined) + }) + + it('authenticates before parsing malformed requests', async () => { + mocks.authenticate.mockResolvedValue({ ok: false, status: 401, error: 'Unauthorized' }) + + const response = await listMailboxes(request({})) + + expect(response.status).toBe(401) + expect(mocks.resolveContext).not.toHaveBeenCalled() + }) + + it('passes every raw connection field to the resolver and maps sorted mailboxes', async () => { + const response = await listMailboxes(request(wireBody)) + + expect(await response.json()).toEqual({ + success: true, + mailboxes: [ + { path: 'INBOX', name: 'INBOX', delimiter: '/' }, + { path: 'Archive', name: 'Archive', delimiter: '/' }, + ], + }) + expect(mocks.resolveContext).toHaveBeenCalledWith(expect.anything(), { + workflowId: 'workflow-1', + context: { + host: '{{IMAP_HOST}}', + port: '{{IMAP_PORT}}', + secure: '{{IMAP_TLS}}', + username: '{{IMAP_USERNAME}}', + password: '{{IMAP_PASSWORD}}', + }, + }) + expect(mocks.validateDatabaseHost).toHaveBeenCalledWith('imap.example.com', 'host', { + logFailureDetails: false, + }) + expect(mocks.imapConstruct).toHaveBeenCalledWith( + expect.objectContaining({ + host: '203.0.113.10', + servername: 'imap.example.com', + port: 993, + secure: true, + auth: { user: 'mailbox@example.com', pass: 'resolved-password' }, + }) + ) + expect(mocks.imapLogout).toHaveBeenCalledOnce() + }) + + it('short-circuits inaccessible references before DNS or provider access', async () => { + mocks.resolveContext.mockResolvedValue({ + ok: false, + status: 400, + error: 'Unable to resolve selector configuration', + }) + + const response = await listMailboxes(request(wireBody)) + + expect(response.status).toBe(400) + expect(await response.json()).toEqual({ + success: false, + message: 'Unable to resolve selector configuration', + }) + expect(mocks.validateDatabaseHost).not.toHaveBeenCalled() + expect(mocks.imapConstruct).not.toHaveBeenCalled() + }) + + it('rejects invalid resolved port/TLS values before DNS or provider access', async () => { + mocks.resolveContext.mockResolvedValue({ + ok: true, + context: { + host: 'imap.example.com', + port: 'invalid', + secure: 'invalid', + username: 'mailbox@example.com', + password: 'resolved-password', + }, + requesterUserId: 'viewer-1', + workspaceId: 'workspace-1', + }) + + const response = await listMailboxes(request(wireBody)) + + expect(response.status).toBe(400) + expect(mocks.validateDatabaseHost).not.toHaveBeenCalled() + expect(mocks.imapConstruct).not.toHaveBeenCalled() + }) + + it('sanitizes provider connection failures and logs out best-effort', async () => { + mocks.imapConnect.mockRejectedValue( + new Error('connection failed with resolved-password and imap.example.com') + ) + + const response = await listMailboxes(request(wireBody)) + const body = await response.json() + + expect(response.status).toBe(500) + expect(body).toEqual({ + success: false, + message: 'Failed to connect to IMAP server. Please check your connection settings.', + }) + expect(mocks.imapLogout).toHaveBeenCalledOnce() + }) +}) diff --git a/apps/sim/hooks/selectors/providers/imap/selectors.ts b/apps/sim/hooks/selectors/providers/imap/selectors.ts index 8d706121c48..a3798815727 100644 --- a/apps/sim/hooks/selectors/providers/imap/selectors.ts +++ b/apps/sim/hooks/selectors/providers/imap/selectors.ts @@ -9,34 +9,23 @@ export const imapSelectors = { * not a stored credential the server can resolve by id — the user types the connection in, * so the parameters travel on the context. * - * **The password is deliberately absent from the query key.** A query key identifies a - * resource; a credential authorizes access to it. `oauthCredential` is safe in a key because - * it is only an id, but a typed password is a secret, and React Query keys are held in cache - * and surfaced by devtools. Host, port, TLS and username already identify the mailbox list - * uniquely — the password only proves the caller may read it, and it rides the request body - * exactly as it did before. - * - * The consequence is intentional: correcting a wrong password re-runs the request (the - * previous attempt failed and cached nothing), while changing ONLY the password on an - * otherwise identical connection reuses the cached list, which is the same list. + * **Connection fields are deliberately absent from the query key.** React Query keys are + * retained in browser memory and surfaced by devtools, so server-resolved dependencies are + * represented by the shared opaque revision instead of their literal or referenced values. */ 'imap.mailboxes': { key: 'imap.mailboxes', contracts: [imapMailboxesContract], + serverResolvedContextFields: ['host', 'port', 'secure', 'username', 'password'], staleTime: SELECTOR_STALE, - getQueryKey: ({ context }: SelectorQueryArgs) => [ - 'selectors', - 'imap.mailboxes', - context.host ?? 'none', - context.port ?? 'default', - context.secure ?? 'default', - context.username ?? 'none', - ], - enabled: ({ context }) => Boolean(context.host && context.username && context.password), + getQueryKey: () => ['selectors', 'imap.mailboxes'], + enabled: ({ context }) => + Boolean(context.host && context.username && context.password && context.workflowId), fetchList: async ({ context, signal }: SelectorQueryArgs) => { if (!context.host || !context.username || !context.password) return [] const data = await requestJson(imapMailboxesContract, { body: { + workflowId: context.workflowId!, host: context.host, port: context.port, secure: context.secure, diff --git a/apps/sim/hooks/selectors/providers/imap/server-resolved-context.test.ts b/apps/sim/hooks/selectors/providers/imap/server-resolved-context.test.ts new file mode 100644 index 00000000000..f9b538f257e --- /dev/null +++ b/apps/sim/hooks/selectors/providers/imap/server-resolved-context.test.ts @@ -0,0 +1,74 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ requestJson: vi.fn() })) + +vi.mock('@/lib/api/client/request', () => ({ requestJson: mocks.requestJson })) + +import { imapSelectors } from '@/hooks/selectors/providers/imap/selectors' + +describe('IMAP server-resolved selector context', () => { + beforeEach(() => vi.clearAllMocks()) + + it('opts every connection field into server resolution', () => { + expect(imapSelectors['imap.mailboxes']?.serverResolvedContextFields).toEqual([ + 'host', + 'port', + 'secure', + 'username', + 'password', + ]) + }) + + it('forwards raw references and maps mailbox options', async () => { + const definition = imapSelectors['imap.mailboxes']! + const context = { + workflowId: 'workflow-1', + host: '{{IMAP_HOST}}', + port: '{{IMAP_PORT}}', + secure: '{{IMAP_TLS}}', + username: '{{IMAP_USERNAME}}', + password: '{{IMAP_PASSWORD}}', + } + mocks.requestJson.mockResolvedValue({ + success: true, + mailboxes: [{ path: 'INBOX', name: 'Inbox', delimiter: '/' }], + }) + + expect(await definition.fetchList!({ key: 'imap.mailboxes', context })).toEqual([ + { id: 'INBOX', label: 'Inbox' }, + ]) + expect(mocks.requestJson.mock.calls[0][1].body).toEqual(context) + }) + + it('requires a workflow and keeps connection plaintext out of base query keys', () => { + const definition = imapSelectors['imap.mailboxes']! + const context = { + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + host: 'private-imap.example.com', + port: '993', + secure: 'true', + username: 'private-user@example.com', + password: 'imap-literal-secret', + } + const key = definition.getQueryKey!({ key: 'imap.mailboxes', context }) + + expect(definition.enabled?.({ key: 'imap.mailboxes', context })).toBe(true) + expect( + definition.enabled?.({ + key: 'imap.mailboxes', + context: { ...context, workflowId: undefined }, + }) + ).toBe(false) + for (const secret of [ + 'private-imap.example.com', + 'private-user@example.com', + 'imap-literal-secret', + ]) { + expect(JSON.stringify(key)).not.toContain(secret) + } + }) +}) diff --git a/apps/sim/lib/api/contracts/tools/imap.server-resolved.test.ts b/apps/sim/lib/api/contracts/tools/imap.server-resolved.test.ts new file mode 100644 index 00000000000..f67a455a1a0 --- /dev/null +++ b/apps/sim/lib/api/contracts/tools/imap.server-resolved.test.ts @@ -0,0 +1,53 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + imapMailboxesBodySchema, + resolvedImapMailboxesBodySchema, +} from '@/lib/api/contracts/tools/imap' + +describe('IMAP selector contracts', () => { + it('accepts literal or exact-reference values for every connection field', () => { + for (const useReferences of [false, true]) { + expect( + imapMailboxesBodySchema.safeParse({ + workflowId: 'workflow-1', + host: useReferences ? '{{IMAP_HOST}}' : 'imap.example.com', + port: useReferences ? '{{IMAP_PORT}}' : 993, + secure: useReferences ? '{{IMAP_TLS}}' : true, + username: useReferences ? '{{IMAP_USERNAME}}' : 'mailbox@example.com', + password: useReferences ? '{{IMAP_PASSWORD}}' : 'literal-password', + }).success + ).toBe(true) + } + }) + + it.each([ + ['993', 'true', 993, true], + ['143', 'false', 143, false], + [undefined, undefined, 993, true], + ])('coerces resolved port %s and TLS %s', (port, secure, expectedPort, expectedSecure) => { + expect( + resolvedImapMailboxesBodySchema.parse({ + host: 'imap.example.com', + port, + secure, + username: 'mailbox@example.com', + password: 'resolved-password', + }) + ).toMatchObject({ port: expectedPort, secure: expectedSecure }) + }) + + it('rejects invalid resolved port and TLS values', () => { + expect( + resolvedImapMailboxesBodySchema.safeParse({ + host: 'imap.example.com', + port: 'invalid', + secure: 'invalid', + username: 'mailbox@example.com', + password: 'resolved-password', + }).success + ).toBe(false) + }) +}) diff --git a/apps/sim/lib/api/contracts/tools/imap.ts b/apps/sim/lib/api/contracts/tools/imap.ts index ace87bd4ca1..b4c6152be12 100644 --- a/apps/sim/lib/api/contracts/tools/imap.ts +++ b/apps/sim/lib/api/contracts/tools/imap.ts @@ -21,13 +21,26 @@ export const imapMailboxesResponseSchema = z.object({ }) export const imapMailboxesBodySchema = z.object({ + workflowId: z.string().min(1, 'Workflow ID is required'), host: z.string().min(1), - port: z.preprocess((value) => value || 993, z.coerce.number().int().positive()), - secure: z.preprocess((value) => value ?? true, z.boolean()), + port: z.union([z.string(), z.number()]).optional(), + secure: z.union([z.string(), z.boolean()]).optional(), username: z.string().min(1), password: z.string().min(1), }) +export const resolvedImapMailboxesBodySchema = imapMailboxesBodySchema + .omit({ workflowId: true }) + .extend({ + port: z.preprocess((value) => value || 993, z.coerce.number().int().positive()), + secure: z.preprocess((value) => { + if (value === undefined || value === null || value === '') return true + if (value === 'true') return true + if (value === 'false') return false + return value + }, z.boolean()), + }) + export const imapMailboxesContract = defineRouteContract({ method: 'POST', path: '/api/tools/imap/mailboxes', diff --git a/apps/sim/lib/core/security/input-validation.server.ts b/apps/sim/lib/core/security/input-validation.server.ts index 08cd4ad311e..2e37c52fde2 100644 --- a/apps/sim/lib/core/security/input-validation.server.ts +++ b/apps/sim/lib/core/security/input-validation.server.ts @@ -194,7 +194,8 @@ export async function validateAndPinProxyUrl( */ export async function validateDatabaseHost( host: string | null | undefined, - paramName = 'host' + paramName = 'host', + options: { logFailureDetails?: boolean } = {} ): Promise { if (!host) { return { isValid: false, error: `${paramName} is required` } @@ -217,11 +218,13 @@ export async function validateDatabaseHost( : addresses.find((candidate) => isPrivateIp(candidate)) if (blockedAddress !== undefined) { - logger.warn('Database host resolves to blocked IP address', { - paramName, - hostname: host, - resolvedIP: blockedAddress, - }) + if (options.logFailureDetails !== false) { + logger.warn('Database host resolves to blocked IP address', { + paramName, + hostname: host, + resolvedIP: blockedAddress, + }) + } return { isValid: false, error: `${paramName} resolves to a blocked IP address`, @@ -234,11 +237,13 @@ export async function validateDatabaseHost( originalHostname: host, } } catch (error) { - logger.warn('DNS lookup failed for database host', { - paramName, - hostname: host, - error: toError(error).message, - }) + if (options.logFailureDetails !== false) { + logger.warn('DNS lookup failed for database host', { + paramName, + hostname: host, + error: toError(error).message, + }) + } return { isValid: false, error: `${paramName} hostname could not be resolved`,