fix(webhooks): validate and pin EmailBison apiBaseUrl before outbound requests (#5415)

* fix(webhooks): validate and pin EmailBison apiBaseUrl before outbound requests Route Email Bison webhook create/delete requests through the shared DNS-validated, IP-pinned fetch used by the Teams and Slack webhook providers instead of a raw fetch to the user-configured instance URL. * fix(webhooks): restore NEXT_PUBLIC_APP_URL in emailbison tests, dedupe strict-delete warning log Test env var mutation was never restored, risking cross-file leakage in single-threaded vitest runs. Strict-mode deleteSubscription failures were logged twice (once at the throw site with context, once generically by the outer catch); the outer catch now skips its own log for errors already logged at the throw site.
2026-09-24 15:45:35 +08:00 · 2026-07-04 15:37:09 -07:00
parent a1fbb5743d
commit 41fb86355d
2 changed files with 229 additions and 13 deletions
@@ -0,0 +1,179 @@
+/**
+ * @vitest-environment node
+ */
+import { inputValidationMock, inputValidationMockFns } from '@sim/testing'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
+
+import { emailBisonHandler } from '@/lib/webhooks/providers/emailbison'
+
+const WEBHOOK_ID = 'webhook-uuid-1234'
+const PUBLIC_BASE_URL = 'https://my-instance.emailbison.com'
+
+function makeWebhook(providerConfig: Record<string, unknown>) {
+  return {
+    id: WEBHOOK_ID,
+    path: 'abc',
+    providerConfig,
+  } as unknown as Parameters<typeof emailBisonHandler.deleteSubscription>[0]['webhook']
+}
+
+function jsonSecureResponse(status: number, body: Record<string, unknown>) {
+  return {
+    ok: status >= 200 && status < 300,
+    status,
+    statusText: '',
+    headers: { get: () => null },
+    body: { cancel: vi.fn() },
+    json: vi.fn().mockResolvedValue(body),
+    text: vi.fn().mockResolvedValue(JSON.stringify(body)),
+    arrayBuffer: vi.fn(),
+  }
+}
+
+describe('emailBisonHandler createSubscription', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    process.env.NEXT_PUBLIC_APP_URL = 'https://app.example.com'
+  })
+
+  afterEach(() => {
+    process.env.NEXT_PUBLIC_APP_URL = undefined
+  })
+
+  it('rejects an apiBaseUrl that resolves to a blocked address before making a request', async () => {
+    inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
+      isValid: false,
+      error: 'URL resolves to a blocked address',
+    })
+
+    const webhook = makeWebhook({
+      apiKey: 'test-key',
+      apiBaseUrl: 'https://169.254.169.254',
+      triggerId: 'emailbison_email_sent',
+    })
+
+    await expect(
+      emailBisonHandler.createSubscription({
+        webhook,
+        workflow: {} as never,
+        userId: 'user-1',
+        requestId: 'req-1',
+      } as never)
+    ).rejects.toThrow('Email Bison Instance URL could not be validated.')
+
+    expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  it('creates the webhook subscription for a valid public instance URL', async () => {
+    inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
+      isValid: true,
+      resolvedIP: '203.0.113.10',
+    })
+    inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValue(
+      jsonSecureResponse(200, { data: { id: 42 } })
+    )
+
+    const webhook = makeWebhook({
+      apiKey: 'test-key',
+      apiBaseUrl: PUBLIC_BASE_URL,
+      triggerId: 'emailbison_email_sent',
+    })
+
+    const result = await emailBisonHandler.createSubscription({
+      webhook,
+      workflow: {} as never,
+      userId: 'user-1',
+      requestId: 'req-1',
+    } as never)
+
+    expect(result).toEqual({ providerConfigUpdates: { externalId: '42' } })
+    expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).toHaveBeenCalledWith(
+      expect.stringContaining(PUBLIC_BASE_URL),
+      '203.0.113.10',
+      expect.objectContaining({ method: 'POST' })
+    )
+  })
+})
+
+describe('emailBisonHandler deleteSubscription', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('rejects an apiBaseUrl that resolves to a blocked address before making a request', async () => {
+    inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
+      isValid: false,
+      error: 'URL resolves to a blocked address',
+    })
+
+    const webhook = makeWebhook({
+      apiKey: 'test-key',
+      apiBaseUrl: 'https://127.0.0.1',
+      externalId: '42',
+    })
+
+    await emailBisonHandler.deleteSubscription({
+      webhook,
+      workflow: {} as never,
+      requestId: 'req-1',
+      strict: false,
+    } as never)
+
+    expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  it('throws when strict and the apiBaseUrl resolves to a blocked address', async () => {
+    inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
+      isValid: false,
+      error: 'URL resolves to a blocked address',
+    })
+
+    const webhook = makeWebhook({
+      apiKey: 'test-key',
+      apiBaseUrl: 'https://127.0.0.1',
+      externalId: '42',
+    })
+
+    await expect(
+      emailBisonHandler.deleteSubscription({
+        webhook,
+        workflow: {} as never,
+        requestId: 'req-1',
+        strict: true,
+      } as never)
+    ).rejects.toThrow('Email Bison Instance URL could not be validated.')
+
+    expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  it('deletes the webhook subscription for a valid public instance URL', async () => {
+    inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
+      isValid: true,
+      resolvedIP: '203.0.113.10',
+    })
+    inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValue(
+      jsonSecureResponse(200, {})
+    )
+
+    const webhook = makeWebhook({
+      apiKey: 'test-key',
+      apiBaseUrl: PUBLIC_BASE_URL,
+      externalId: '42',
+    })
+
+    await emailBisonHandler.deleteSubscription({
+      webhook,
+      workflow: {} as never,
+      requestId: 'req-1',
+      strict: false,
+    } as never)
+
+    expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).toHaveBeenCalledWith(
+      expect.stringContaining(PUBLIC_BASE_URL),
+      '203.0.113.10',
+      expect.objectContaining({ method: 'DELETE' })
+    )
+  })
+})
@@ -1,6 +1,11 @@
 import { createLogger } from '@sim/logger'
 import { toError } from '@sim/utils/errors'
 import { isRecordLike } from '@sim/utils/object'
+import {
+  type SecureFetchResponse,
+  secureFetchWithPinnedIP,
+  validateUrlWithDNS,
+} from '@/lib/core/security/input-validation.server'
 import { getNotificationUrl, getProviderConfig } from '@/lib/webhooks/provider-subscription-utils'
 import type {
  DeleteSubscriptionContext,
@@ -147,7 +152,14 @@ export const emailBisonHandler: WebhookProviderHandler = {
      webhookId: webhook.id,
    })

-    const response = await fetch(emailBisonUrl('/api/webhook-url', {}, apiBaseUrl), {
+    const targetUrl = emailBisonUrl('/api/webhook-url', {}, apiBaseUrl)
+    const urlValidation = await validateUrlWithDNS(targetUrl, 'apiBaseUrl')
+    if (!urlValidation.isValid) {
+      logger.warn(`[${requestId}] Invalid Email Bison Instance URL: ${urlValidation.error}`)
+      throw new Error('Email Bison Instance URL could not be validated.')
+    }
+
+    const response = await secureFetchWithPinnedIP(targetUrl, urlValidation.resolvedIP!, {
      method: 'POST',
      headers: emailBisonHeaders({ apiKey, apiBaseUrl }),
      body: JSON.stringify({
@@ -207,17 +219,30 @@ export const emailBisonHandler: WebhookProviderHandler = {
          hasApiBaseUrl: Boolean(apiBaseUrl),
          hasExternalId: Boolean(externalId),
        })
-        if (ctx.strict) throw new Error('Missing Email Bison webhook cleanup configuration')
+        if (ctx.strict)
+          throw new AlreadyLoggedError('Missing Email Bison webhook cleanup configuration')
        return
      }

-      const response = await fetch(
-        emailBisonUrl(`/api/webhook-url/${encodeURIComponent(externalId)}`, {}, apiBaseUrl),
-        {
-          method: 'DELETE',
-          headers: emailBisonHeaders({ apiKey, apiBaseUrl }),
-        }
+      const targetUrl = emailBisonUrl(
+        `/api/webhook-url/${encodeURIComponent(externalId)}`,
+        {},
+        apiBaseUrl
      )
+      const urlValidation = await validateUrlWithDNS(targetUrl, 'apiBaseUrl')
+      if (!urlValidation.isValid) {
+        logger.warn(`[${requestId}] Invalid Email Bison Instance URL: ${urlValidation.error}`, {
+          webhookId: webhook.id,
+        })
+        if (ctx.strict)
+          throw new AlreadyLoggedError('Email Bison Instance URL could not be validated.')
+        return
+      }
+
+      const response = await secureFetchWithPinnedIP(targetUrl, urlValidation.resolvedIP!, {
+        method: 'DELETE',
+        headers: emailBisonHeaders({ apiKey, apiBaseUrl }),
+      })

      if (!response.ok && response.status !== 404) {
        const responseBody = await parseJsonResponse(response)
@@ -225,7 +250,8 @@ export const emailBisonHandler: WebhookProviderHandler = {
          status: response.status,
          response: responseBody,
        })
-        if (ctx.strict) throw new Error(`Failed to delete Email Bison webhook: ${response.status}`)
+        if (ctx.strict)
+          throw new AlreadyLoggedError(`Failed to delete Email Bison webhook: ${response.status}`)
        return
      }

@@ -235,15 +261,26 @@ export const emailBisonHandler: WebhookProviderHandler = {
        webhookId: webhook.id,
      })
    } catch (error) {
-      logger.warn(`[${requestId}] Error deleting Email Bison webhook`, {
-        message: toError(error).message,
-      })
+      if (!(error instanceof AlreadyLoggedError)) {
+        logger.warn(`[${requestId}] Error deleting Email Bison webhook`, {
+          message: toError(error).message,
+        })
+      }
      if (ctx.strict) throw error
    }
  },
 }

-async function parseJsonResponse(response: Response): Promise<Record<string, unknown> | null> {
+/**
+ * Marks an error whose failure reason has already been logged with full context
+ * at the throw site, so the outer catch in `deleteSubscription` does not emit
+ * a second, redundant warning for the same failure.
+ */
+class AlreadyLoggedError extends Error {}
+
+async function parseJsonResponse(
+  response: SecureFetchResponse
+): Promise<Record<string, unknown> | null> {
  try {
    const body: unknown = await response.json()
    return isRecordLike(body) ? body : null