fix(security): SSRF pinning, Twilio webhook auth, copilot token leak, audit-log tenant scoping (#4899)

* fix(clickhouse): pin outbound HTTP connection to validated IP (DNS rebinding)

clickhouseRequest() validated config.host via validateDatabaseHost() but
discarded the resolved IP and called fetch() with the original hostname,
triggering a second DNS lookup. A workflow author controlling the host
parameter could use DNS rebinding to pass validation against a public IP
and then connect to an internal/private address (SSRF).

Replace fetch() with secureFetchWithPinnedIP(), connecting to the
validated resolvedIP while preserving the hostname for Host/TLS SNI — the
same DNS-pinning pattern used by the other DB tools. Set Content-Length
explicitly so request framing is identical to the previous fetch.

Add tests locking the contract: connection targets the validated IP not
the hostname, no request is issued on validation failure, http/https and
allowHttp are selected from secure, and body/headers propagate.

* fix(mcp): pin auth-type probe to validated IP to close SSRF/DNS-rebinding window

The MCP auth-type probe (detectMcpAuthType) issued raw, unpinned fetch()
calls against the user-supplied server URL, re-resolving DNS independently
of validateMcpServerSsrf. This re-opened the exact DNS-rebinding (TOCTOU)
window the pinned McpClient path was built to close: a hostname that
resolves to a public IP during validation could resolve to an internal IP
during the probe.

The probe now pins to the IP already validated by the caller via
createMcpPinnedFetch(resolvedIP); when no pre-validated IP is available it
falls back to createSsrfGuardedMcpFetch(), which validates and pins each
request. The best-effort session-close DELETE reuses the same pinned fetch.
Both call sites (test-connection route and performCreateMcpServer) thread
the resolved IP into the probe.

* fix(security): stop returning plaintext OAuth access tokens from copilot credentials

GET /api/copilot/credentials returned each connected account's live,
post-refresh OAuth access token in plaintext to any session for that
user. The endpoint is only used for credential display/masking and no
client reads the token, so drop accessToken from the get_credentials
tool output and the copilot credentials response contract. Also removes
the incidental refreshTokenIfNeeded side-effect on this read path.

Adds regression tests:
- get-credentials: asserts the response exposes only masked metadata and
  never leaks the access/refresh token.
- revoke: locks in that revokeMcpOauthTokens routes OAuth discovery and
  RFC 7009 revocation through the SSRF-guarded fetch (no raw fetch to an
  attacker-controlled revocation_endpoint).

* fix(webhooks): verify X-Twilio-Signature on Twilio SMS webhooks

The twilio (SMS) provider handler implemented no verifyAuth, so the webhook
dispatcher queued workflow executions for any request to a known SMS trigger
path without validating the Twilio signature — allowing forged inbound SMS
events. Only the twilio-voice handler performed signature verification.

Extract the shared HMAC-SHA1 signature validation into twilio-signature.ts
and wire it into both the SMS and Voice handlers. Verification is enforced
when an auth token is configured (parity with Voice); requests without a
configured token pass through per the provider-wide optional-secret
convention. Add regression tests for both handlers.

* fix(connectors): route user-controlled connector hosts through DNS-validated, IP-pinned fetch

Knowledge connectors that accept a custom service host/endpoint (S3-compatible
endpoints, self-managed GitLab/Sentry hosts, Obsidian vault URLs) performed
server-side fetches without the repository's SSRF guard, letting an authenticated
user with KB write access probe internal/loopback hosts from the backend.

Add secureFetchWithRetry (validateUrlWithDNS + secureFetchWithPinnedIP + the same
retry/backoff as fetchWithRetry) and route every request in the s3, gitlab, sentry,
and obsidian connectors through it - including pagination and hydration. Gate the
S3 plain-http loopback exception to self-hosted deployments.

* fix(audit-logs): scope enterprise audit log access to organization boundary

Actor membership was used as a standalone tenant predicate, letting org
admins read members' audit activity from personal workspaces and other
tenants. Scope queries to org-attached workspaces plus org-level events,
with actor membership only narrowing the scope; validate workspaceId
filters against the caller's organization.

* fix(webhooks): warn when Twilio webhook has no auth token configured

Addresses PR review: when no auth token is set, verifyTwilioAuth skips
signature verification (optional-secret convention). Log a warning so
operators can detect a webhook running unauthenticated.

* fix(audit-logs): include system events (null actor) in default org audit scope

SQL IN never matches NULL, so system/automated events inside org
workspaces were hidden unless includeDeparted=true. The default scope now
matches current members OR null-actor rows, still inside the org boundary.

* fix(auth): type-safe access to OAuth2Tokens raw payload

Installed better-auth's OAuth2Tokens no longer declares the raw property;
access it through an intersection cast (no behavior change) so type-check
passes.

* fix(build): keep connector SSRF fetch out of the client bundle

The connectors SSRF fix routed s3/gitlab/sentry/obsidian through
secureFetchWithRetry, which transitively imports input-validation.server
(and its Node-only `dns/promises`). connectors/registry.ts is imported by
client components for connector metadata, so the connector sync code —
which only ever runs in server API routes — gets pulled into the client
bundle, and Turbopack fails to resolve `dns/promises` (no browser shim).

- Move secureFetchWithRetry into a dedicated `secure-fetch.server` module so
  the shared documents/utils stays client-safe; connectors import from there.
- Add a browser-only `turbopack.resolveAlias` stub for `dns`/`dns/promises`
  (the documented Next 16 remedy). Server bundles keep the real module, so
  SSRF validation is unaffected — only the never-executed client copy is stubbed.

Verified with a full `next build` (compiles successfully, no module errors).
This commit is contained in:
Waleed
2026-06-06 15:13:21 -07:00
committed by GitHub
parent 0075ab9cf6
commit 20a00a181d
33 changed files with 1740 additions and 259 deletions
+9 -2
View File
@@ -10,6 +10,7 @@ import { formatAuditLogEntry } from '@/app/api/v1/audit-logs/format'
import {
buildFilterConditions,
buildOrgScopeCondition,
getOrgWorkspaceIds,
queryAuditLogs,
} from '@/app/api/v1/audit-logs/query'
@@ -29,7 +30,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
return authResult.response
}
const { orgMemberIds } = authResult.context
const { organizationId, orgMemberIds } = authResult.context
const parsed = await parseRequest(
listAuditLogsContract,
@@ -57,7 +58,13 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
cursor,
} = parsed.data.query
const scopeCondition = await buildOrgScopeCondition(orgMemberIds, includeDeparted)
const orgWorkspaceIds = await getOrgWorkspaceIds(organizationId)
const scopeCondition = buildOrgScopeCondition({
organizationId,
orgWorkspaceIds,
orgMemberIds,
includeDeparted,
})
const filterConditions = buildFilterConditions({
action,
resourceType,
@@ -173,7 +173,7 @@ export const POST = withRouteHandler(
// Skip unauth connect when the server returns an RFC 9728 OAuth challenge.
if (testConfig.url) {
const detectedAuthType = await detectMcpAuthType(testConfig.url)
const detectedAuthType = await detectMcpAuthType(testConfig.url, resolvedIP)
if (detectedAuthType === 'oauth') {
result.authRequired = true
result.authType = 'oauth'
@@ -0,0 +1,126 @@
/**
* @vitest-environment node
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { ClickHouseConnectionConfig } from '@/tools/clickhouse/types'
const { mockValidateDatabaseHost, mockSecureFetchWithPinnedIP, mockValidateSqlWhereClause } =
vi.hoisted(() => ({
mockValidateDatabaseHost: vi.fn(),
mockSecureFetchWithPinnedIP: vi.fn(),
mockValidateSqlWhereClause: vi.fn(),
}))
vi.mock('@/lib/core/security/input-validation.server', () => ({
validateDatabaseHost: mockValidateDatabaseHost,
secureFetchWithPinnedIP: mockSecureFetchWithPinnedIP,
validateSqlWhereClause: mockValidateSqlWhereClause,
}))
import { executeClickHouseInsert, executeClickHouseQuery } from '@/app/api/tools/clickhouse/utils'
function makeConfig(
overrides: Partial<ClickHouseConnectionConfig> = {}
): ClickHouseConnectionConfig {
return {
host: 'clickhouse.example.com',
port: 8123,
database: 'default',
username: 'default',
password: 'secret',
secure: false,
...overrides,
}
}
function okResponse(body: string, summary?: string) {
return {
ok: true,
status: 200,
statusText: 'OK',
text: async () => body,
headers: {
get: (name: string) =>
name.toLowerCase() === 'x-clickhouse-summary' ? (summary ?? null) : null,
},
}
}
describe('clickhouseRequest DNS pinning', () => {
beforeEach(() => {
vi.clearAllMocks()
mockValidateDatabaseHost.mockResolvedValue({
isValid: true,
resolvedIP: '93.184.216.34',
originalHostname: 'clickhouse.example.com',
})
mockValidateSqlWhereClause.mockReturnValue({ isValid: true })
mockSecureFetchWithPinnedIP.mockResolvedValue(okResponse('{"data":[{"x":1}],"rows":1}'))
})
it('pins the connection to the validated IP, not the attacker-controlled hostname', async () => {
await executeClickHouseQuery(makeConfig({ host: 'rebind.attacker.example' }), 'SELECT 1')
expect(mockValidateDatabaseHost).toHaveBeenCalledWith('rebind.attacker.example', 'host')
expect(mockSecureFetchWithPinnedIP).toHaveBeenCalledTimes(1)
const [url, pinnedIP, options] = mockSecureFetchWithPinnedIP.mock.calls[0]
// The actual TCP target is the validated IP — re-resolution of the hostname can never happen.
expect(pinnedIP).toBe('93.184.216.34')
// The hostname is preserved only in the URL (for Host header / TLS SNI), never used to connect.
expect(url).toContain('rebind.attacker.example')
expect(options.method).toBe('POST')
})
it('never issues the request when host validation fails (no SSRF window)', async () => {
mockValidateDatabaseHost.mockResolvedValue({
isValid: false,
error: 'host resolves to a blocked IP address',
})
await expect(executeClickHouseQuery(makeConfig(), 'SELECT 1')).rejects.toThrow(
'host resolves to a blocked IP address'
)
expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
})
it('uses https and disallows http redirects when secure is true', async () => {
await executeClickHouseQuery(makeConfig({ secure: true, port: 8443 }), 'SELECT 1')
const [url, , options] = mockSecureFetchWithPinnedIP.mock.calls[0]
expect(url).toMatch(/^https:\/\//)
expect(options.allowHttp).toBe(false)
})
it('allows http for the initial request when secure is false', async () => {
await executeClickHouseQuery(makeConfig({ secure: false }), 'SELECT 1')
const [url, , options] = mockSecureFetchWithPinnedIP.mock.calls[0]
expect(url).toMatch(/^http:\/\//)
expect(options.allowHttp).toBe(true)
})
it('sends the statement as the body with a matching Content-Length and auth headers', async () => {
await executeClickHouseInsert(makeConfig(), 'events', { id: 1 })
const [, , options] = mockSecureFetchWithPinnedIP.mock.calls[0]
expect(options.body).toContain('INSERT INTO `events` FORMAT JSONEachRow')
expect(options.headers['Content-Length']).toBe(String(Buffer.byteLength(options.body, 'utf-8')))
expect(options.headers['X-ClickHouse-User']).toBe('default')
expect(options.headers['X-ClickHouse-Key']).toBe('secret')
})
it('propagates non-ok responses as errors with the body text', async () => {
mockSecureFetchWithPinnedIP.mockResolvedValue({
ok: false,
status: 400,
statusText: 'Bad Request',
text: async () => 'Code: 62. DB::Exception: Syntax error',
headers: { get: () => null },
})
await expect(executeClickHouseQuery(makeConfig(), 'SELECT 1')).rejects.toThrow(
'Code: 62. DB::Exception: Syntax error'
)
})
})
+16 -18
View File
@@ -1,4 +1,5 @@
import {
secureFetchWithPinnedIP,
validateDatabaseHost,
validateSqlWhereClause,
} from '@/lib/core/security/input-validation.server'
@@ -81,24 +82,21 @@ async function clickhouseRequest(
url.searchParams.set('readonly', '1')
}
const controller = new AbortController()
const timeout = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS)
let response: Response
try {
response = await fetch(url.toString(), {
method: 'POST',
headers: {
'X-ClickHouse-User': config.username,
'X-ClickHouse-Key': config.password,
'Content-Type': 'text/plain; charset=utf-8',
},
body: statement,
signal: controller.signal,
})
} finally {
clearTimeout(timeout)
}
// Pin the connection to the IP that passed validation. Without this, fetch()
// would re-resolve `config.host` and a DNS-rebinding hostname could point the
// actual request at an internal/private address after validation succeeded.
const response = await secureFetchWithPinnedIP(url.toString(), hostValidation.resolvedIP!, {
method: 'POST',
headers: {
'X-ClickHouse-User': config.username,
'X-ClickHouse-Key': config.password,
'Content-Type': 'text/plain; charset=utf-8',
'Content-Length': String(Buffer.byteLength(statement, 'utf-8')),
},
body: statement,
timeout: REQUEST_TIMEOUT_MS,
allowHttp: !config.secure,
})
const text = await response.text()
@@ -0,0 +1,129 @@
/**
* @vitest-environment node
*
* Tests for GET /api/v1/audit-logs/[id] — verifies the lookup is constrained
* by the organization scope and 404s for rows outside it.
*/
import { createMockRequest, dbChainMock, dbChainMockFns } from '@sim/testing'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
mockCheckRateLimit,
mockValidateEnterpriseAuditAccess,
mockBuildOrgScopeCondition,
mockGetOrgWorkspaceIds,
} = vi.hoisted(() => ({
mockCheckRateLimit: vi.fn(),
mockValidateEnterpriseAuditAccess: vi.fn(),
mockBuildOrgScopeCondition: vi.fn(),
mockGetOrgWorkspaceIds: vi.fn(),
}))
vi.mock('@sim/db', () => dbChainMock)
vi.mock('@/app/api/v1/middleware', () => ({
checkRateLimit: mockCheckRateLimit,
createRateLimitResponse: vi.fn(),
}))
vi.mock('@/app/api/v1/audit-logs/auth', () => ({
validateEnterpriseAuditAccess: mockValidateEnterpriseAuditAccess,
}))
vi.mock('@/app/api/v1/audit-logs/query', () => ({
buildOrgScopeCondition: mockBuildOrgScopeCondition,
getOrgWorkspaceIds: mockGetOrgWorkspaceIds,
}))
vi.mock('@/app/api/v1/logs/meta', () => ({
getUserLimits: vi.fn().mockResolvedValue({}),
createApiResponse: vi.fn((body: unknown) => ({ body, headers: {} })),
}))
import { GET } from '@/app/api/v1/audit-logs/[id]/route'
const ORG_ID = 'org-1'
const MEMBER_IDS = ['admin-1', 'member-1']
const ORG_WORKSPACE_IDS = ['ws-org-1']
const SCOPE_SENTINEL = { type: 'org-scope-sentinel' }
const AUDIT_ROW = {
id: 'log-1',
workspaceId: 'ws-org-1',
actorId: 'member-1',
actorName: 'Member',
actorEmail: 'member@example.com',
action: 'workflow.created',
resourceType: 'workflow',
resourceId: 'wf-1',
resourceName: 'My Workflow',
description: 'Created workflow',
metadata: {},
ipAddress: '127.0.0.1',
userAgent: 'test',
createdAt: new Date('2026-01-01T00:00:00Z'),
}
function callRoute(id: string) {
const request = createMockRequest(
'GET',
undefined,
{},
`http://localhost:3000/api/v1/audit-logs/${id}`
)
return GET(request, { params: Promise.resolve({ id }) })
}
describe('GET /api/v1/audit-logs/[id]', () => {
beforeEach(() => {
vi.clearAllMocks()
mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'admin-1' })
mockValidateEnterpriseAuditAccess.mockResolvedValue({
success: true,
context: { organizationId: ORG_ID, orgMemberIds: MEMBER_IDS },
})
mockGetOrgWorkspaceIds.mockResolvedValue(ORG_WORKSPACE_IDS)
mockBuildOrgScopeCondition.mockReturnValue(SCOPE_SENTINEL)
})
it('constrains the lookup with the org scope condition (includeDeparted)', async () => {
dbChainMockFns.limit.mockResolvedValueOnce([AUDIT_ROW])
const response = await callRoute('log-1')
expect(response.status).toBe(200)
expect(mockBuildOrgScopeCondition).toHaveBeenCalledWith({
organizationId: ORG_ID,
orgWorkspaceIds: ORG_WORKSPACE_IDS,
orgMemberIds: MEMBER_IDS,
includeDeparted: true,
})
expect(dbChainMockFns.where).toHaveBeenCalledWith(
expect.objectContaining({
type: 'and',
conditions: expect.arrayContaining([SCOPE_SENTINEL]),
})
)
})
it('returns 404 when the row is outside the organization scope', async () => {
dbChainMockFns.limit.mockResolvedValueOnce([])
const response = await callRoute('log-outside-org')
expect(response.status).toBe(404)
const body = await response.json()
expect(body.error).toBe('Audit log not found')
})
it('excludes ipAddress and userAgent from the response', async () => {
dbChainMockFns.limit.mockResolvedValueOnce([AUDIT_ROW])
const response = await callRoute('log-1')
const body = await response.json()
expect(body.data.id).toBe('log-1')
expect(body.data.ipAddress).toBeUndefined()
expect(body.data.userAgent).toBeUndefined()
})
})
+15 -18
View File
@@ -4,24 +4,26 @@
* Get a single audit log entry by ID, scoped to the authenticated user's organization.
* Requires enterprise subscription and org admin/owner role.
*
* Scope includes logs from current org members AND logs within org workspaces
* (including those from departed members or system actions with null actorId).
* Scope is the organization boundary: logs within org-attached workspaces and
* org-level events (including those from departed members or system actions
* with null actorId).
*
* Response: { data: AuditLogEntry, limits: UserLimits }
*/
import { db } from '@sim/db'
import { auditLog, workspace } from '@sim/db/schema'
import { auditLog } from '@sim/db/schema'
import { createLogger } from '@sim/logger'
import { getErrorMessage } from '@sim/utils/errors'
import { generateId } from '@sim/utils/id'
import { and, eq, inArray, or } from 'drizzle-orm'
import { and, eq } from 'drizzle-orm'
import { type NextRequest, NextResponse } from 'next/server'
import { v1GetAuditLogContract } from '@/lib/api/contracts/v1/audit-logs'
import { parseRequest } from '@/lib/api/server'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { validateEnterpriseAuditAccess } from '@/app/api/v1/audit-logs/auth'
import { formatAuditLogEntry } from '@/app/api/v1/audit-logs/format'
import { buildOrgScopeCondition, getOrgWorkspaceIds } from '@/app/api/v1/audit-logs/query'
import { createApiResponse, getUserLimits } from '@/app/api/v1/logs/meta'
import { checkRateLimit, createRateLimitResponse } from '@/app/api/v1/middleware'
@@ -53,25 +55,20 @@ export const GET = withRouteHandler(
return authResult.response
}
const { orgMemberIds } = authResult.context
const { organizationId, orgMemberIds } = authResult.context
const orgWorkspaceIds = db
.select({ id: workspace.id })
.from(workspace)
.where(inArray(workspace.ownerId, orgMemberIds))
const orgWorkspaceIds = await getOrgWorkspaceIds(organizationId)
const scopeCondition = buildOrgScopeCondition({
organizationId,
orgWorkspaceIds,
orgMemberIds,
includeDeparted: true,
})
const [log] = await db
.select()
.from(auditLog)
.where(
and(
eq(auditLog.id, id),
or(
inArray(auditLog.actorId, orgMemberIds),
inArray(auditLog.workspaceId, orgWorkspaceIds)
)
)
)
.where(and(eq(auditLog.id, id), scopeCondition))
.limit(1)
if (!log) {
@@ -0,0 +1,172 @@
/**
* @vitest-environment node
*
* Tests for the enterprise audit-log tenant boundary. The global drizzle-orm
* mock returns structured operator objects, so these tests assert directly on
* the predicate tree.
*/
import { dbChainMock, dbChainMockFns } from '@sim/testing'
import { beforeEach, describe, expect, it, vi } from 'vitest'
vi.mock('@sim/db', () => dbChainMock)
import { buildOrgScopeCondition, getOrgWorkspaceIds } from '@/app/api/v1/audit-logs/query'
const ORG_ID = 'org-1'
const MEMBER_IDS = ['user-1', 'user-2']
const WORKSPACE_IDS = ['ws-1', 'ws-2']
interface MockCondition {
type?: string
conditions?: MockCondition[]
column?: string
values?: string[]
left?: string
right?: string
strings?: string[]
}
function asCondition(value: unknown): MockCondition {
return value as MockCondition
}
/**
* Asserts the condition matches null-workspace rows tied to the organization
* via metadata or the organization resource itself.
*/
function expectOrgLevelCondition(condition: MockCondition, organizationId: string): void {
expect(condition.type).toBe('and')
const [nullCheck, orgLink] = condition.conditions!
expect(nullCheck).toMatchObject({ type: 'isNull', column: 'workspaceId' })
expect(orgLink.type).toBe('or')
const [metadataMatch, orgResourceMatch] = orgLink.conditions!
expect(metadataMatch.strings?.join('?')).toContain("->>'organizationId' =")
expect(metadataMatch.values).toContain(organizationId)
expect(orgResourceMatch.type).toBe('and')
expect(orgResourceMatch.conditions).toEqual([
expect.objectContaining({ type: 'eq', left: 'resourceType', right: 'organization' }),
expect.objectContaining({ type: 'eq', left: 'resourceId', right: organizationId }),
])
}
describe('buildOrgScopeCondition', () => {
it('never uses actor membership as a standalone boundary (default scope)', () => {
const condition = asCondition(
buildOrgScopeCondition({
organizationId: ORG_ID,
orgWorkspaceIds: WORKSPACE_IDS,
orgMemberIds: MEMBER_IDS,
includeDeparted: false,
})
)
expect(condition.type).toBe('and')
const [orgScope, actorFilter] = condition.conditions!
expect(orgScope.type).toBe('or')
const [workspaceScope, orgLevel] = orgScope.conditions!
expect(workspaceScope).toMatchObject({
type: 'inArray',
column: 'workspaceId',
values: WORKSPACE_IDS,
})
expectOrgLevelCondition(orgLevel, ORG_ID)
expect(actorFilter).toMatchObject({
type: 'or',
conditions: [
expect.objectContaining({ type: 'inArray', column: 'actorId', values: MEMBER_IDS }),
expect.objectContaining({ type: 'isNull', column: 'actorId' }),
],
})
})
it('omits the actor filter entirely when includeDeparted is true', () => {
const condition = asCondition(
buildOrgScopeCondition({
organizationId: ORG_ID,
orgWorkspaceIds: WORKSPACE_IDS,
orgMemberIds: MEMBER_IDS,
includeDeparted: true,
})
)
expect(condition.type).toBe('or')
const [workspaceScope, orgLevel] = condition.conditions!
expect(workspaceScope).toMatchObject({
type: 'inArray',
column: 'workspaceId',
values: WORKSPACE_IDS,
})
expectOrgLevelCondition(orgLevel, ORG_ID)
expect(JSON.stringify(condition)).not.toContain('actorId')
})
it('falls back to the org-level branch alone when the org has no workspaces', () => {
const condition = asCondition(
buildOrgScopeCondition({
organizationId: ORG_ID,
orgWorkspaceIds: [],
orgMemberIds: MEMBER_IDS,
includeDeparted: true,
})
)
expectOrgLevelCondition(condition, ORG_ID)
})
it('still applies the actor filter on top of the org scope with no workspaces', () => {
const condition = asCondition(
buildOrgScopeCondition({
organizationId: ORG_ID,
orgWorkspaceIds: [],
orgMemberIds: MEMBER_IDS,
includeDeparted: false,
})
)
expect(condition.type).toBe('and')
const [orgLevel, actorFilter] = condition.conditions!
expectOrgLevelCondition(orgLevel, ORG_ID)
expect(actorFilter).toMatchObject({
type: 'or',
conditions: [
expect.objectContaining({ type: 'inArray', column: 'actorId', values: MEMBER_IDS }),
expect.objectContaining({ type: 'isNull', column: 'actorId' }),
],
})
})
it('only matches system events when the org has no current members', () => {
const condition = asCondition(
buildOrgScopeCondition({
organizationId: ORG_ID,
orgWorkspaceIds: WORKSPACE_IDS,
orgMemberIds: [],
includeDeparted: false,
})
)
expect(condition.type).toBe('and')
const [, actorFilter] = condition.conditions!
expect(actorFilter).toMatchObject({ type: 'isNull', column: 'actorId' })
})
})
describe('getOrgWorkspaceIds', () => {
beforeEach(() => {
vi.clearAllMocks()
})
it('selects workspaces by organization ownership, not member ownership', async () => {
const ids = await getOrgWorkspaceIds(ORG_ID)
expect(ids).toEqual([])
expect(dbChainMockFns.where).toHaveBeenCalledWith(
expect.objectContaining({ type: 'eq', left: 'organizationId', right: ORG_ID })
)
})
})
+51 -22
View File
@@ -1,7 +1,8 @@
import { AuditResourceType } from '@sim/audit'
import { db } from '@sim/db'
import { auditLog, workspace } from '@sim/db/schema'
import type { InferSelectModel } from 'drizzle-orm'
import { and, desc, eq, gte, ilike, inArray, lt, lte, or, type SQL, sql } from 'drizzle-orm'
import { and, desc, eq, gte, ilike, inArray, isNull, lt, lte, or, type SQL, sql } from 'drizzle-orm'
type DbAuditLog = InferSelectModel<typeof auditLog>
@@ -68,33 +69,61 @@ export function buildFilterConditions(params: AuditLogFilterParams): SQL<unknown
return conditions
}
export async function buildOrgScopeCondition(
orgMemberIds: string[],
includeDeparted: boolean
): Promise<SQL<unknown>> {
if (orgMemberIds.length === 0) {
return sql`1 = 0`
}
if (!includeDeparted) {
return inArray(auditLog.actorId, orgMemberIds)
}
const orgWorkspaces = await db
/**
* Returns the IDs of all workspaces attached to the organization.
*/
export async function getOrgWorkspaceIds(organizationId: string): Promise<string[]> {
const rows = await db
.select({ id: workspace.id })
.from(workspace)
.where(inArray(workspace.ownerId, orgMemberIds))
.where(eq(workspace.organizationId, organizationId))
return rows.map((row) => row.id)
}
const orgWorkspaceIds = orgWorkspaces.map((w) => w.id)
export interface OrgScopeParams {
organizationId: string
orgWorkspaceIds: string[]
orgMemberIds: string[]
includeDeparted: boolean
}
if (orgWorkspaceIds.length > 0) {
return or(
inArray(auditLog.actorId, orgMemberIds),
inArray(auditLog.workspaceId, orgWorkspaceIds)
)!
/**
* Builds the tenant-boundary predicate for organization audit log access:
* rows in org-attached workspaces, plus org-level rows (`workspace_id IS
* NULL`) tied to the org via `metadata.organizationId` or the organization
* resource itself. Actor membership is never a standalone boundary — when
* `includeDeparted` is false it only narrows the org scope to current members
* and system events (null actor).
*/
export function buildOrgScopeCondition(params: OrgScopeParams): SQL<unknown> {
const { organizationId, orgWorkspaceIds, orgMemberIds, includeDeparted } = params
const orgLevelCondition = and(
isNull(auditLog.workspaceId),
or(
sql`${auditLog.metadata}->>'organizationId' = ${organizationId}`,
and(
eq(auditLog.resourceType, AuditResourceType.ORGANIZATION),
eq(auditLog.resourceId, organizationId)
)
)
)!
const orgScope =
orgWorkspaceIds.length > 0
? or(inArray(auditLog.workspaceId, orgWorkspaceIds), orgLevelCondition)!
: orgLevelCondition
if (includeDeparted) {
return orgScope
}
return inArray(auditLog.actorId, orgMemberIds)
const currentActorCondition =
orgMemberIds.length > 0
? or(inArray(auditLog.actorId, orgMemberIds), isNull(auditLog.actorId))!
: isNull(auditLog.actorId)
return and(orgScope, currentActorCondition)!
}
function buildCursorCondition(cursor: string): SQL<unknown> | null {
@@ -0,0 +1,130 @@
/**
* @vitest-environment node
*
* Tests for GET /api/v1/audit-logs — verifies filters are validated against
* the caller's organization and the scope is built from the org context.
*/
import { createMockRequest } from '@sim/testing'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
mockCheckRateLimit,
mockValidateEnterpriseAuditAccess,
mockBuildOrgScopeCondition,
mockGetOrgWorkspaceIds,
mockQueryAuditLogs,
mockBuildFilterConditions,
} = vi.hoisted(() => ({
mockCheckRateLimit: vi.fn(),
mockValidateEnterpriseAuditAccess: vi.fn(),
mockBuildOrgScopeCondition: vi.fn(),
mockGetOrgWorkspaceIds: vi.fn(),
mockQueryAuditLogs: vi.fn(),
mockBuildFilterConditions: vi.fn(),
}))
vi.mock('@/app/api/v1/middleware', () => ({
checkRateLimit: mockCheckRateLimit,
createRateLimitResponse: vi.fn(),
}))
vi.mock('@/app/api/v1/audit-logs/auth', () => ({
validateEnterpriseAuditAccess: mockValidateEnterpriseAuditAccess,
}))
vi.mock('@/app/api/v1/audit-logs/query', () => ({
buildFilterConditions: mockBuildFilterConditions,
buildOrgScopeCondition: mockBuildOrgScopeCondition,
getOrgWorkspaceIds: mockGetOrgWorkspaceIds,
queryAuditLogs: mockQueryAuditLogs,
}))
vi.mock('@/app/api/v1/logs/meta', () => ({
getUserLimits: vi.fn().mockResolvedValue({}),
createApiResponse: vi.fn((body: unknown) => ({ body, headers: {} })),
}))
import { GET } from '@/app/api/v1/audit-logs/route'
const ORG_ID = 'org-1'
const MEMBER_IDS = ['admin-1', 'member-1']
const ORG_WORKSPACE_IDS = ['ws-org-1', 'ws-org-2']
const SCOPE_SENTINEL = { type: 'org-scope-sentinel' }
function makeRequest(query: string) {
return createMockRequest('GET', undefined, {}, `http://localhost:3000/api/v1/audit-logs${query}`)
}
describe('GET /api/v1/audit-logs', () => {
beforeEach(() => {
vi.clearAllMocks()
mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'admin-1' })
mockValidateEnterpriseAuditAccess.mockResolvedValue({
success: true,
context: { organizationId: ORG_ID, orgMemberIds: MEMBER_IDS },
})
mockGetOrgWorkspaceIds.mockResolvedValue(ORG_WORKSPACE_IDS)
mockBuildOrgScopeCondition.mockReturnValue(SCOPE_SENTINEL)
mockBuildFilterConditions.mockReturnValue([])
mockQueryAuditLogs.mockResolvedValue({ data: [], nextCursor: undefined })
})
it('rejects an actorId that is not a current org member', async () => {
const response = await GET(makeRequest('?actorId=outsider-1'))
expect(response.status).toBe(400)
const body = await response.json()
expect(body.error).toBe('actorId is not a member of your organization')
expect(mockQueryAuditLogs).not.toHaveBeenCalled()
})
it('rejects a workspaceId that does not belong to the organization', async () => {
const response = await GET(makeRequest('?workspaceId=ws-other-org'))
expect(response.status).toBe(400)
const body = await response.json()
expect(body.error).toBe('workspaceId does not belong to your organization')
expect(mockQueryAuditLogs).not.toHaveBeenCalled()
})
it('accepts a workspaceId that belongs to the organization', async () => {
const response = await GET(makeRequest('?workspaceId=ws-org-1'))
expect(response.status).toBe(200)
expect(mockQueryAuditLogs).toHaveBeenCalled()
})
it('builds the scope from the organization context, never from actors alone', async () => {
const response = await GET(makeRequest('?actorId=member-1'))
expect(response.status).toBe(200)
expect(mockBuildOrgScopeCondition).toHaveBeenCalledWith({
organizationId: ORG_ID,
orgWorkspaceIds: ORG_WORKSPACE_IDS,
orgMemberIds: MEMBER_IDS,
includeDeparted: false,
})
const [conditions] = mockQueryAuditLogs.mock.calls[0]
expect(conditions[0]).toBe(SCOPE_SENTINEL)
})
it('passes includeDeparted through to the scope builder', async () => {
const response = await GET(makeRequest('?includeDeparted=true'))
expect(response.status).toBe(200)
expect(mockBuildOrgScopeCondition).toHaveBeenCalledWith(
expect.objectContaining({ includeDeparted: true })
)
})
it('returns the auth failure response when enterprise access is denied', async () => {
const denied = new Response(JSON.stringify({ error: 'nope' }), { status: 403 })
mockValidateEnterpriseAuditAccess.mockResolvedValue({ success: false, response: denied })
const response = await GET(makeRequest(''))
expect(response.status).toBe(403)
expect(mockQueryAuditLogs).not.toHaveBeenCalled()
})
})
+17 -2
View File
@@ -31,6 +31,7 @@ import { formatAuditLogEntry } from '@/app/api/v1/audit-logs/format'
import {
buildFilterConditions,
buildOrgScopeCondition,
getOrgWorkspaceIds,
queryAuditLogs,
} from '@/app/api/v1/audit-logs/query'
import { createApiResponse, getUserLimits } from '@/app/api/v1/logs/meta'
@@ -57,7 +58,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
return authResult.response
}
const { orgMemberIds } = authResult.context
const { organizationId, orgMemberIds } = authResult.context
const parsed = await parseRequest(
v1ListAuditLogsContract,
@@ -85,7 +86,21 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
)
}
const scopeCondition = await buildOrgScopeCondition(orgMemberIds, params.includeDeparted)
const orgWorkspaceIds = await getOrgWorkspaceIds(organizationId)
if (params.workspaceId && !orgWorkspaceIds.includes(params.workspaceId)) {
return NextResponse.json(
{ error: 'workspaceId does not belong to your organization' },
{ status: 400 }
)
}
const scopeCondition = buildOrgScopeCondition({
organizationId,
orgWorkspaceIds,
orgMemberIds,
includeDeparted: params.includeDeparted,
})
const filterConditions = buildFilterConditions({
action: params.action,
resourceType: params.resourceType,
+12 -10
View File
@@ -1,8 +1,10 @@
import { createLogger } from '@sim/logger'
import { getErrorMessage, toError } from '@sim/utils/errors'
import { GitLabIcon } from '@/components/icons'
import type { SecureFetchResponse } from '@/lib/core/security/input-validation.server'
import { isSameOrigin } from '@/lib/core/utils/validation'
import { fetchWithRetry, VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
import { secureFetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server'
import { VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
import { computeContentHash, joinTagArray, parseTagDate } from '@/connectors/utils'
@@ -461,8 +463,8 @@ async function fetchProject(
encodedProject: string,
accessToken: string,
retryOptions?: typeof VALIDATE_RETRY_OPTIONS
): Promise<Response> {
return fetchWithRetry(
): Promise<SecureFetchResponse> {
return secureFetchWithRetry(
`${apiBase}/projects/${encodedProject}`,
{ method: 'GET', headers: authHeaders(accessToken) },
retryOptions
@@ -760,7 +762,7 @@ export const gitlabConnector: ConnectorConfig = {
continued: Boolean(state.fileNextUrl),
})
const response = await fetchWithRetry(url, {
const response = await secureFetchWithRetry(url, {
method: 'GET',
headers: authHeaders(accessToken),
})
@@ -816,7 +818,7 @@ export const gitlabConnector: ConnectorConfig = {
const url = `${apiBase}/projects/${encodedProject}/wikis?with_content=1`
logger.info('Listing GitLab wiki pages', { host, project: encodedProject })
const response = await fetchWithRetry(url, {
const response = await secureFetchWithRetry(url, {
method: 'GET',
headers: authHeaders(accessToken),
})
@@ -891,7 +893,7 @@ export const gitlabConnector: ConnectorConfig = {
incremental: Boolean(lastSyncAt),
})
const response = await fetchWithRetry(url, {
const response = await secureFetchWithRetry(url, {
method: 'GET',
headers: authHeaders(accessToken),
})
@@ -954,7 +956,7 @@ export const gitlabConnector: ConnectorConfig = {
if (!slug) return null
const url = `${apiBase}/projects/${encodedProject}/wikis/${encodeURIComponent(slug)}?render_html=false`
const response = await fetchWithRetry(url, {
const response = await secureFetchWithRetry(url, {
method: 'GET',
headers: authHeaders(accessToken),
})
@@ -975,7 +977,7 @@ export const gitlabConnector: ConnectorConfig = {
if (!iidStr || Number.isNaN(iid)) return null
const url = `${apiBase}/projects/${encodedProject}/issues/${iid}`
const response = await fetchWithRetry(url, {
const response = await secureFetchWithRetry(url, {
method: 'GET',
headers: authHeaders(accessToken),
})
@@ -1002,7 +1004,7 @@ export const gitlabConnector: ConnectorConfig = {
accessToken
)
const url = `${apiBase}/projects/${encodedProject}/repository/files/${encodeURIComponent(path)}?ref=${encodeURIComponent(ref)}`
const response = await fetchWithRetry(url, {
const response = await secureFetchWithRetry(url, {
method: 'GET',
headers: authHeaders(accessToken),
})
@@ -1078,7 +1080,7 @@ export const gitlabConnector: ConnectorConfig = {
const userRef = typeof sourceConfig.ref === 'string' ? sourceConfig.ref.trim() : ''
if (userRef && activePhases(choice).includes('repo')) {
const refResponse = await fetchWithRetry(
const refResponse = await secureFetchWithRetry(
`${apiBase}/projects/${encodedProject}/repository/commits/${encodeURIComponent(userRef)}`,
{ method: 'GET', headers: authHeaders(accessToken) },
VALIDATE_RETRY_OPTIONS
+16 -18
View File
@@ -2,7 +2,8 @@ import { createLogger } from '@sim/logger'
import { toError } from '@sim/utils/errors'
import { ObsidianIcon } from '@/components/icons'
import { validateExternalUrl } from '@/lib/core/security/input-validation'
import { fetchWithRetry, VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
import { secureFetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server'
import { VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
import { joinTagArray, parseTagDate } from '@/connectors/utils'
@@ -24,18 +25,15 @@ interface NoteJson {
}
/**
* Normalizes the vault URL and validates it against SSRF protections.
* Normalizes the vault URL and runs an early structural SSRF check via the
* shared `validateExternalUrl` policy (hosted Sim blocks localhost/private/HTTP;
* self-hosted allows http://localhost only).
*
* The Obsidian Local REST API plugin runs on the user's own machine, so there
* is no SaaS domain to allowlist — the vault URL is fully user-controlled. We
* defer to the shared `validateExternalUrl` policy:
* - hosted Sim: blocks localhost, private IPs, HTTP (forces HTTPS)
* - self-hosted Sim: allows http://localhost (built-in carve-out), still
* blocks non-loopback private IPs and dangerous ports (22, 25, 3306,
* 5432, 6379, 27017, 9200)
*
* This does not defend against DNS rebinding; for hosted deployments the user
* must expose the plugin through a public URL (tunnel, port-forward).
* The authoritative SSRF boundary is enforced at request time: every vault
* request goes through {@link secureFetchWithRetry}, which resolves DNS,
* re-checks the resolved IP, and pins the connection to it — closing the
* DNS-rebinding gap a synchronous string check cannot. On hosted Sim the plugin
* must be exposed through a public URL.
*/
function resolveVaultEndpoint(rawUrl: string | undefined): string {
let url = (rawUrl || DEFAULT_VAULT_URL).trim().replace(/\/+$/, '')
@@ -57,12 +55,12 @@ async function listDirectory(
baseUrl: string,
accessToken: string,
dirPath: string,
retryOptions?: Parameters<typeof fetchWithRetry>[2]
retryOptions?: Parameters<typeof secureFetchWithRetry>[2]
): Promise<string[]> {
const encodedDir = dirPath ? dirPath.split('/').map(encodeURIComponent).join('/') : ''
const endpoint = encodedDir ? `${baseUrl}/vault/${encodedDir}/` : `${baseUrl}/vault/`
const response = await fetchWithRetry(
const response = await secureFetchWithRetry(
endpoint,
{
method: 'GET',
@@ -88,7 +86,7 @@ async function listVaultFiles(
baseUrl: string,
accessToken: string,
folderPath?: string,
retryOptions?: Parameters<typeof fetchWithRetry>[2],
retryOptions?: Parameters<typeof secureFetchWithRetry>[2],
depth = 0
): Promise<string[]> {
if (depth > MAX_RECURSION_DEPTH) {
@@ -134,9 +132,9 @@ async function fetchNote(
baseUrl: string,
accessToken: string,
filePath: string,
retryOptions?: Parameters<typeof fetchWithRetry>[2]
retryOptions?: Parameters<typeof secureFetchWithRetry>[2]
): Promise<NoteJson> {
const response = await fetchWithRetry(
const response = await secureFetchWithRetry(
`${baseUrl}/vault/${filePath.split('/').map(encodeURIComponent).join('/')}`,
{
method: 'GET',
@@ -304,7 +302,7 @@ export const obsidianConnector: ConnectorConfig = {
}
try {
const response = await fetchWithRetry(
const response = await secureFetchWithRetry(
`${baseUrl}/`,
{
method: 'GET',
+14 -10
View File
@@ -2,7 +2,9 @@ import crypto from 'crypto'
import { createLogger } from '@sim/logger'
import { getErrorMessage, toError } from '@sim/utils/errors'
import { S3Icon } from '@/components/icons'
import { fetchWithRetry, VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
import { isHosted } from '@/lib/core/config/feature-flags'
import { secureFetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server'
import { VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
import { parseTagDate, readBodyWithLimit } from '@/connectors/utils'
import { encodeS3PathComponent, getSignatureKey } from '@/tools/s3/utils'
@@ -112,9 +114,11 @@ function isSupportedKey(key: string, allowedExtensions: Set<string>): boolean {
}
/**
* Returns true when the host is a loopback address for which plain `http://`
* is tolerated (local MinIO development). Any other host must use `https://` so
* that credentials are never transmitted over cleartext.
* Returns true when the host is a loopback address for which plain `http://` is
* tolerated (local MinIO development on a self-hosted deployment). Any other
* host must use `https://`. This is only an early check — the SSRF boundary is
* enforced at request time by {@link secureFetchWithRetry}, which blocks
* loopback/private targets on hosted Sim regardless of what this parser accepts.
*/
function isLoopbackHost(host: string): boolean {
const bare = host.replace(/^\[|\]$/g, '')
@@ -160,9 +164,9 @@ function parseEndpoint(raw: string): S3Endpoint {
const host = url.hostname
if (!host) throw new Error('Endpoint is missing a host')
if (scheme === 'http' && !isLoopbackHost(host)) {
if (scheme === 'http' && !(isLoopbackHost(host) && !isHosted)) {
throw new Error(
'Plain http:// endpoints are only allowed for localhost — use https:// otherwise'
'Plain http:// endpoints are only allowed for localhost on self-hosted deployments — use https:// otherwise'
)
}
@@ -261,7 +265,7 @@ function buildUrl(ctx: S3Context, encodedPath: string, canonicalQueryString: str
* Reuses {@link getSignatureKey} from the s3 tool utilities.
*
* The signed headers embed `x-amz-date` and are reused verbatim across
* `fetchWithRetry` attempts. S3 allows a 15-minute clock-skew window; the
* `secureFetchWithRetry` attempts. S3 allows a 15-minute clock-skew window; the
* retry helper's worst-case total backoff (~31s default, ~10s in validate) is
* far inside that window, so a stale timestamp never triggers
* RequestTimeTooSkewed.
@@ -449,7 +453,7 @@ async function listObjectsPage(
ctx: S3Context,
prefix: string,
continuationToken: string | undefined,
retryOptions?: Parameters<typeof fetchWithRetry>[2],
retryOptions?: Parameters<typeof secureFetchWithRetry>[2],
maxKeys: number = LIST_MAX_KEYS
): Promise<{ objects: S3ObjectEntry[]; isTruncated: boolean; nextContinuationToken?: string }> {
const queryParams: Record<string, string> = {
@@ -466,7 +470,7 @@ async function listObjectsPage(
const url = buildUrl(ctx, bucketPath, canonicalQueryString)
const response = await fetchWithRetry(url, { method: 'GET', headers }, retryOptions)
const response = await secureFetchWithRetry(url, { method: 'GET', headers }, retryOptions)
if (!response.ok) {
const errorText = await response.text()
@@ -616,7 +620,7 @@ export const s3Connector: ConnectorConfig = {
const headers = buildSignedHeaders(ctx, 'GET', encodedPath, '')
const url = buildUrl(ctx, encodedPath, '')
const response = await fetchWithRetry(url, { method: 'GET', headers })
const response = await secureFetchWithRetry(url, { method: 'GET', headers })
if (response.status === 404) return null
if (!response.ok) {
+7 -6
View File
@@ -1,7 +1,8 @@
import { createLogger } from '@sim/logger'
import { getErrorMessage, toError } from '@sim/utils/errors'
import { SentryIcon } from '@/components/icons'
import { fetchWithRetry, VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
import { secureFetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server'
import { VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
import { parseTagDate } from '@/connectors/utils'
@@ -373,7 +374,7 @@ async function fetchLatestEvent(
): Promise<SentryEvent | null> {
const url = `${apiBase}/organizations/${encodeURIComponent(organization)}/issues/${encodeURIComponent(issueId)}/events/latest/`
const response = await fetchWithRetry(url, {
const response = await secureFetchWithRetry(url, {
method: 'GET',
headers: authHeaders(accessToken),
})
@@ -507,7 +508,7 @@ export const sentryConnector: ConnectorConfig = {
maxIssues,
})
const response = await fetchWithRetry(url.toString(), {
const response = await secureFetchWithRetry(url.toString(), {
method: 'GET',
headers: authHeaders(accessToken),
})
@@ -564,7 +565,7 @@ export const sentryConnector: ConnectorConfig = {
const url = `${apiBase}/organizations/${encodeURIComponent(organization)}/issues/${encodeURIComponent(externalId)}/`
const response = await fetchWithRetry(url, {
const response = await secureFetchWithRetry(url, {
method: 'GET',
headers: authHeaders(accessToken),
})
@@ -629,7 +630,7 @@ export const sentryConnector: ConnectorConfig = {
* scope and the project-scoped path style, and gives a precise "not found"
* message when the org or project slug is wrong.
*/
const projectResponse = await fetchWithRetry(
const projectResponse = await secureFetchWithRetry(
`${apiBase}/projects/${encodeURIComponent(organization)}/${encodeURIComponent(project)}/`,
{
method: 'GET',
@@ -670,7 +671,7 @@ export const sentryConnector: ConnectorConfig = {
issuesProbeUrl.searchParams.set('query', DEFAULT_QUERY)
issuesProbeUrl.searchParams.set('limit', '1')
const issuesResponse = await fetchWithRetry(
const issuesResponse = await secureFetchWithRetry(
issuesProbeUrl.toString(),
{
method: 'GET',
+3 -1
View File
@@ -1,3 +1,5 @@
import type { SecureFetchResponse } from '@/lib/core/security/input-validation.server'
/**
* Strips HTML tags from content and decodes common HTML entities.
*/
@@ -88,7 +90,7 @@ export function parseMultiValue(value: unknown): string[] {
* Returns null when the cap is exceeded.
*/
export async function readBodyWithLimit(
response: Response,
response: Response | SecureFetchResponse,
maxBytes: number
): Promise<Buffer | null> {
if (!response.body) {
-1
View File
@@ -430,7 +430,6 @@ const copilotConnectedCredentialSchema = z.object({
serviceName: z.string(),
lastUsed: z.string(),
isDefault: z.boolean(),
accessToken: z.string().nullable(),
})
const copilotNotConnectedServiceSchema = z.object({
+2 -1
View File
@@ -2543,7 +2543,8 @@ export const auth = betterAuth({
* marker disambiguates it from a legacy bot id (same `U.../B...` shape);
* absent it, we keep the legacy format and today's behavior.
*/
const authedUser = tokens.raw?.authed_user as { id?: string } | undefined
const rawTokens = (tokens as typeof tokens & { raw?: Record<string, unknown> }).raw
const authedUser = rawTokens?.authed_user as { id?: string } | undefined
const installerUserId = authedUser?.id
const userSegment = installerUserId
? `usr_${installerUserId}`
@@ -0,0 +1,134 @@
/**
* @vitest-environment node
*
* Regression test: the credentials response must expose only display metadata,
* never the connected account's OAuth access/refresh token.
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
const SECRET_ACCESS_TOKEN = 'ya29.a0SECRET_GOOGLE_BEARER_TOKEN_DO_NOT_LEAK'
const { selectMock, getAllOAuthServicesMock, getPersonalAndWorkspaceEnvMock, jwtDecodeMock } =
vi.hoisted(() => ({
selectMock: vi.fn(),
getAllOAuthServicesMock: vi.fn(),
getPersonalAndWorkspaceEnvMock: vi.fn(),
jwtDecodeMock: vi.fn(),
}))
vi.mock('@sim/db', () => ({
db: { select: selectMock },
}))
vi.mock('@/lib/oauth', () => ({
getAllOAuthServices: getAllOAuthServicesMock,
}))
vi.mock('@/lib/environment/utils', () => ({
getPersonalAndWorkspaceEnv: getPersonalAndWorkspaceEnvMock,
}))
vi.mock('jwt-decode', () => ({
jwtDecode: jwtDecodeMock,
}))
import { getCredentialsServerTool } from './get-credentials'
/**
* Wires the two sequential `db.select()` reads the tool performs:
* 1. `select().from(account).where()` → account rows (awaited directly)
* 2. `select({...}).from(user).where().limit(1)` → user row
*/
function wireDb(accountRows: unknown[], userRows: Array<{ email: string }>) {
const whereThenable = {
then: (resolve: (rows: unknown[]) => unknown) => resolve(accountRows),
limit: () => Promise.resolve(userRows),
}
const builder = { from: () => builder, where: () => whereThenable }
selectMock.mockReturnValue(builder)
}
describe('getCredentialsServerTool', () => {
beforeEach(() => {
vi.clearAllMocks()
wireDb(
[
{
id: 'acct-google-1',
providerId: 'google-default',
accountId: '1234567890',
idToken: 'jwt-token',
accessToken: SECRET_ACCESS_TOKEN,
refreshToken: 'refresh-secret',
updatedAt: new Date('2026-04-17T02:26:05.546Z'),
},
],
[{ email: 'brent@cellular.so' }]
)
getAllOAuthServicesMock.mockReturnValue([
{
providerId: 'google-default',
name: 'Google',
description: 'Google account',
baseProvider: 'google',
},
{
providerId: 'slack',
name: 'Slack',
description: 'Slack workspace',
baseProvider: 'slack',
},
])
getPersonalAndWorkspaceEnvMock.mockResolvedValue({
personalEncrypted: {},
workspaceEncrypted: {},
conflicts: [],
})
jwtDecodeMock.mockReturnValue({ email: 'brent@cellular.so' })
})
it('never returns access tokens for connected OAuth credentials', async () => {
const result = await getCredentialsServerTool.execute({}, { userId: 'user-1' })
const credentials = result.oauth.connected.credentials
expect(credentials).toHaveLength(1)
for (const credential of credentials) {
expect(credential).not.toHaveProperty('accessToken')
expect(credential).not.toHaveProperty('refreshToken')
expect(credential).not.toHaveProperty('idToken')
}
})
it('returns only masked display metadata for each credential', async () => {
const result = await getCredentialsServerTool.execute({}, { userId: 'user-1' })
expect(result.oauth.connected.credentials[0]).toEqual({
id: 'acct-google-1',
name: 'brent@cellular.so',
provider: 'google-default',
serviceName: 'Google',
lastUsed: '2026-04-17T02:26:05.546Z',
isDefault: true,
})
})
it('does not leak the token value anywhere in the serialized response', async () => {
const result = await getCredentialsServerTool.execute({}, { userId: 'user-1' })
expect(JSON.stringify(result)).not.toContain(SECRET_ACCESS_TOKEN)
expect(JSON.stringify(result)).not.toContain('refresh-secret')
})
it('rejects unauthenticated callers without touching the database', async () => {
await expect(getCredentialsServerTool.execute({}, undefined)).rejects.toThrow(
'Authentication required'
)
expect(selectMock).not.toHaveBeenCalled()
})
})
@@ -6,10 +6,8 @@ import { eq } from 'drizzle-orm'
import { jwtDecode } from 'jwt-decode'
import { createPermissionError, verifyWorkflowAccess } from '@/lib/copilot/auth/permissions'
import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool'
import { generateRequestId } from '@/lib/core/utils/request'
import { getPersonalAndWorkspaceEnv } from '@/lib/environment/utils'
import { getAllOAuthServices } from '@/lib/oauth'
import { refreshTokenIfNeeded } from '@/app/api/auth/oauth/utils'
interface GetCredentialsParams {
workflowId?: string
@@ -76,9 +74,7 @@ export const getCredentialsServerTool: BaseServerTool<GetCredentialsParams, any>
serviceName: string
lastUsed: string
isDefault: boolean
accessToken: string | null
}> = []
const requestId = generateRequestId()
for (const acc of accounts) {
const providerId = acc.providerId
@@ -104,19 +100,6 @@ export const getCredentialsServerTool: BaseServerTool<GetCredentialsParams, any>
const service = allOAuthServices.find((s) => s.providerId === providerId)
const serviceName = service?.name ?? providerId
let accessToken: string | null = acc.accessToken ?? null
try {
const { accessToken: refreshedToken } = await refreshTokenIfNeeded(
requestId,
acc as any,
acc.id
)
accessToken = refreshedToken || accessToken
} catch (error) {
logger.warn('Failed to refresh OAuth access token', {
error: toError(error).message,
})
}
connectedCredentials.push({
id: acc.id,
name: displayName,
@@ -124,7 +107,6 @@ export const getCredentialsServerTool: BaseServerTool<GetCredentialsParams, any>
serviceName,
lastUsed: acc.updatedAt.toISOString(),
isDefault: featureType === 'default',
accessToken,
})
}
@@ -0,0 +1,13 @@
/**
* Browser fallback for Node-only builtins (e.g. `dns/promises`) that get pulled
* into the client bundle by server-only code which never executes in the
* browser — notably the connector registry, whose `ConnectorConfig` objects are
* imported by client UI for metadata while their `listDocuments`/`getDocument`
* fetch logic (which transitively imports `input-validation.server`) only ever
* runs in server API routes.
*
* Wired in via `turbopack.resolveAlias` with the `browser` condition only, so
* the real Node module is still resolved on the server and SSRF validation
* remains fully intact. See `next.config.ts`.
*/
export default {}
@@ -0,0 +1,71 @@
import {
type SecureFetchOptions,
type SecureFetchResponse,
secureFetchWithValidation,
} from '@/lib/core/security/input-validation.server'
import {
type HTTPError,
isRetryableError,
type RetryOptions,
retryWithExponentialBackoff,
} from '@/lib/knowledge/documents/utils'
export interface SecureFetchRetryOptions extends RetryOptions {
allowHttp?: boolean
timeout?: number
maxResponseBytes?: number
}
/**
* SSRF-safe counterpart to {@link fetchWithRetry} for connector requests to
* user-controlled hosts. Every attempt re-runs {@link secureFetchWithValidation}
* (DNS resolution, private/loopback/reserved-IP rejection, IP-pinned connection,
* redirect re-validation); retry/backoff semantics mirror {@link fetchWithRetry}.
*
* Lives in a `.server.ts` module because it pulls in Node-only `dns/promises`
* via {@link secureFetchWithValidation}; importing it from the shared
* `documents/utils` barrel would drag that into client bundles.
*/
export async function secureFetchWithRetry(
url: string,
options: SecureFetchOptions = {},
retryOptions: SecureFetchRetryOptions = {}
): Promise<SecureFetchResponse> {
const { allowHttp, timeout, maxResponseBytes, ...retry } = retryOptions
return retryWithExponentialBackoff(async () => {
const response = await secureFetchWithValidation(
url,
{
...options,
...(allowHttp !== undefined ? { allowHttp } : {}),
...(timeout !== undefined ? { timeout } : {}),
...(maxResponseBytes !== undefined ? { maxResponseBytes } : {}),
},
'url'
)
if (!response.ok && isRetryableError({ status: response.status })) {
const errorText = await response.text()
const error: HTTPError = new Error(
`HTTP ${response.status}: ${response.statusText} - ${errorText}`
)
error.status = response.status
error.statusText = response.statusText
const retryAfter = response.headers.get('retry-after')
if (retryAfter) {
const waitMs = Number.isNaN(Number(retryAfter))
? Math.max(0, new Date(retryAfter).getTime() - Date.now())
: Number(retryAfter) * 1000
if (waitMs > 0) {
error.retryAfterMs = waitMs
}
}
throw error
}
return response
}, retry)
}
+121 -1
View File
@@ -1,9 +1,39 @@
/**
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { mockSecureFetchWithValidation } = vi.hoisted(() => ({
mockSecureFetchWithValidation: vi.fn(),
}))
vi.mock('@/lib/core/security/input-validation.server', () => ({
secureFetchWithValidation: mockSecureFetchWithValidation,
}))
import { secureFetchWithRetry } from './secure-fetch.server'
import { isRetryableError } from './utils'
/** Builds a minimal SecureFetchResponse-shaped object for tests. */
function fakeResponse(
status: number,
options: { headers?: Record<string, string>; body?: string } = {}
) {
const headers = options.headers ?? {}
return {
ok: status >= 200 && status < 300,
status,
statusText: `status-${status}`,
headers: { get: (name: string) => headers[name.toLowerCase()] ?? null },
body: null,
text: async () => options.body ?? '',
json: async () => JSON.parse(options.body ?? '{}'),
arrayBuffer: async () => new ArrayBuffer(0),
}
}
const FAST_RETRY = { initialDelayMs: 1, maxDelayMs: 2, maxRetries: 3 }
describe('isRetryableError', () => {
describe('retryable status codes', () => {
it.concurrent('returns true for 429 on Error with status', () => {
@@ -150,3 +180,93 @@ describe('isRetryableError', () => {
})
})
})
describe('secureFetchWithRetry', () => {
beforeEach(() => {
mockSecureFetchWithValidation.mockReset()
})
it('routes the request through secureFetchWithValidation and returns the response', async () => {
mockSecureFetchWithValidation.mockResolvedValue(fakeResponse(200, { body: 'ok' }))
const response = await secureFetchWithRetry('https://example.com/api', {
method: 'GET',
headers: { Accept: 'application/json' },
})
expect(response.status).toBe(200)
expect(mockSecureFetchWithValidation).toHaveBeenCalledTimes(1)
const [url, options, paramName] = mockSecureFetchWithValidation.mock.calls[0]
expect(url).toBe('https://example.com/api')
expect(options).toMatchObject({ method: 'GET', headers: { Accept: 'application/json' } })
expect(paramName).toBe('url')
})
it('propagates SSRF validation failures without retrying', async () => {
mockSecureFetchWithValidation.mockRejectedValue(
new Error('url resolves to a blocked IP address')
)
await expect(
secureFetchWithRetry('https://attacker.test', { method: 'GET' }, FAST_RETRY)
).rejects.toThrow('blocked IP address')
expect(mockSecureFetchWithValidation).toHaveBeenCalledTimes(1)
})
it('retries on a retryable status (503) and succeeds', async () => {
mockSecureFetchWithValidation
.mockResolvedValueOnce(fakeResponse(503, { body: 'try later' }))
.mockResolvedValueOnce(fakeResponse(200, { body: 'ok' }))
const response = await secureFetchWithRetry(
'https://example.com/api',
{ method: 'GET' },
FAST_RETRY
)
expect(response.status).toBe(200)
expect(mockSecureFetchWithValidation).toHaveBeenCalledTimes(2)
})
it('does not retry a non-retryable status (404) and returns it to the caller', async () => {
mockSecureFetchWithValidation.mockResolvedValue(fakeResponse(404, { body: 'missing' }))
const response = await secureFetchWithRetry(
'https://example.com/api',
{ method: 'GET' },
FAST_RETRY
)
expect(response.status).toBe(404)
expect(mockSecureFetchWithValidation).toHaveBeenCalledTimes(1)
})
it('forwards allowHttp / timeout / maxResponseBytes to the pinned fetch', async () => {
mockSecureFetchWithValidation.mockResolvedValue(fakeResponse(200))
await secureFetchWithRetry(
'http://localhost:9000',
{ method: 'GET' },
{ allowHttp: true, timeout: 5000, maxResponseBytes: 1024, ...FAST_RETRY }
)
const [, options] = mockSecureFetchWithValidation.mock.calls[0]
expect(options).toMatchObject({ allowHttp: true, timeout: 5000, maxResponseBytes: 1024 })
})
it('honors Retry-After (seconds) on a 429 before retrying', async () => {
mockSecureFetchWithValidation
.mockResolvedValueOnce(fakeResponse(429, { headers: { 'retry-after': '0' } }))
.mockResolvedValueOnce(fakeResponse(200))
const response = await secureFetchWithRetry(
'https://example.com/api',
{ method: 'GET' },
FAST_RETRY
)
expect(response.status).toBe(200)
expect(mockSecureFetchWithValidation).toHaveBeenCalledTimes(2)
})
})
+1 -1
View File
@@ -5,7 +5,7 @@ import { randomFloat } from '@sim/utils/random'
const logger = createLogger('RetryUtils')
interface HTTPError extends Error {
export interface HTTPError extends Error {
status?: number
statusText?: string
retryAfterMs?: number
+112
View File
@@ -0,0 +1,112 @@
/**
* @vitest-environment node
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
const {
mockCreateMcpPinnedFetch,
mockCreateSsrfGuardedMcpFetch,
mockPinnedFetch,
mockGuardedFetch,
} = vi.hoisted(() => {
const mockPinnedFetch = vi.fn()
const mockGuardedFetch = vi.fn()
return {
mockPinnedFetch,
mockGuardedFetch,
mockCreateMcpPinnedFetch: vi.fn(() => mockPinnedFetch),
mockCreateSsrfGuardedMcpFetch: vi.fn(() => mockGuardedFetch),
}
})
vi.mock('@/lib/mcp/pinned-fetch', () => ({
createMcpPinnedFetch: mockCreateMcpPinnedFetch,
createSsrfGuardedMcpFetch: mockCreateSsrfGuardedMcpFetch,
}))
import { detectMcpAuthType } from '@/lib/mcp/oauth/probe'
function makeResponse(init: { status?: number; headers?: Record<string, string> }): Response {
const status = init.status ?? 200
return {
status,
ok: status >= 200 && status < 300,
headers: new Headers(init.headers ?? {}),
} as unknown as Response
}
describe('detectMcpAuthType — connection pinning (SSRF / DNS-rebinding)', () => {
let globalFetchSpy: ReturnType<typeof vi.fn>
beforeEach(() => {
vi.clearAllMocks()
globalFetchSpy = vi.fn()
vi.stubGlobal('fetch', globalFetchSpy)
})
it('pins the probe to the pre-validated IP when resolvedIP is supplied', async () => {
mockPinnedFetch.mockResolvedValue(makeResponse({ status: 200 }))
const authType = await detectMcpAuthType('https://rebind.example.com/mcp', '203.0.113.10')
expect(authType).toBe('none')
expect(mockCreateMcpPinnedFetch).toHaveBeenCalledWith('203.0.113.10')
expect(mockCreateSsrfGuardedMcpFetch).not.toHaveBeenCalled()
expect(mockPinnedFetch).toHaveBeenCalledTimes(1)
// The unpinned global fetch must never be used — that was the SSRF sink.
expect(globalFetchSpy).not.toHaveBeenCalled()
})
it('falls back to the SSRF-guarded fetch when no resolvedIP is supplied', async () => {
mockGuardedFetch.mockResolvedValue(makeResponse({ status: 200 }))
const authType = await detectMcpAuthType('https://example.com/mcp')
expect(authType).toBe('none')
expect(mockCreateSsrfGuardedMcpFetch).toHaveBeenCalledTimes(1)
expect(mockCreateMcpPinnedFetch).not.toHaveBeenCalled()
expect(mockGuardedFetch).toHaveBeenCalledTimes(1)
expect(globalFetchSpy).not.toHaveBeenCalled()
})
it('classifies an RFC 9728 OAuth challenge as oauth via the pinned fetch', async () => {
mockPinnedFetch.mockResolvedValue(
makeResponse({
status: 401,
headers: {
'www-authenticate':
'Bearer resource_metadata="https://example.com/.well-known/oauth-protected-resource"',
},
})
)
const authType = await detectMcpAuthType('https://example.com/mcp', '203.0.113.10')
expect(authType).toBe('oauth')
expect(globalFetchSpy).not.toHaveBeenCalled()
})
it('does not probe (no network call) for non-https, non-loopback URLs', async () => {
const authType = await detectMcpAuthType('http://example.com/mcp', '203.0.113.10')
expect(authType).toBe('headers')
expect(mockCreateMcpPinnedFetch).not.toHaveBeenCalled()
expect(mockCreateSsrfGuardedMcpFetch).not.toHaveBeenCalled()
expect(globalFetchSpy).not.toHaveBeenCalled()
})
it('reuses the pinned fetch for best-effort session cleanup (DELETE)', async () => {
mockPinnedFetch
.mockResolvedValueOnce(makeResponse({ status: 200, headers: { 'mcp-session-id': 'sess-1' } }))
.mockResolvedValueOnce(makeResponse({ status: 200 }))
const authType = await detectMcpAuthType('https://example.com/mcp', '203.0.113.10')
expect(authType).toBe('none')
// POST probe + DELETE cleanup, both through the pinned fetch.
await vi.waitFor(() => expect(mockPinnedFetch).toHaveBeenCalledTimes(2))
const deleteCall = mockPinnedFetch.mock.calls[1]
expect(deleteCall[1]).toMatchObject({ method: 'DELETE' })
expect(globalFetchSpy).not.toHaveBeenCalled()
})
})
+29 -6
View File
@@ -1,13 +1,26 @@
import { extractWWWAuthenticateParams } from '@modelcontextprotocol/sdk/client/auth.js'
import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'
import { createLogger } from '@sim/logger'
import { isLoopbackHostname } from '@/lib/core/utils/urls'
import { createMcpPinnedFetch, createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
import type { McpAuthType } from '@/lib/mcp/types'
const logger = createLogger('McpOauthProbe')
const PROBE_TIMEOUT_MS = 5000
export async function detectMcpAuthType(url: string): Promise<McpAuthType> {
/**
* Probes an MCP server URL to classify its auth requirement.
*
* The probe must never re-resolve DNS independently of the caller's SSRF
* validation, or it re-opens the DNS-rebinding window. When the caller passes a
* pre-validated `resolvedIP` the connection is pinned to it; otherwise an
* SSRF-guarded fetch validates and pins each request itself.
*/
export async function detectMcpAuthType(
url: string,
resolvedIP?: string | null
): Promise<McpAuthType> {
let parsed: URL
try {
parsed = new URL(url)
@@ -18,11 +31,16 @@ export async function detectMcpAuthType(url: string): Promise<McpAuthType> {
if (parsed.protocol !== 'https:' && !isLoopbackHttp) {
return 'headers'
}
const probeFetch: FetchLike = resolvedIP
? createMcpPinnedFetch(resolvedIP)
: createSsrfGuardedMcpFetch()
const controller = new AbortController()
const timer = setTimeout(() => controller.abort(), PROBE_TIMEOUT_MS)
try {
const res = await fetch(url, {
const res = await probeFetch(url, {
method: 'POST',
redirect: 'manual',
headers: {
@@ -44,7 +62,7 @@ export async function detectMcpAuthType(url: string): Promise<McpAuthType> {
const sessionId = res.headers.get('mcp-session-id')
if (sessionId) {
void closeMcpSession(url, sessionId)
void closeMcpSession(url, sessionId, probeFetch)
}
if (res.status === 401) {
@@ -71,14 +89,19 @@ export async function detectMcpAuthType(url: string): Promise<McpAuthType> {
/**
* Best-effort DELETE to release the streamable-HTTP session the probe just
* allocated. Failures are ignored — the session will expire on the server side.
* allocated. Reuses the probe's pinned fetch so this cleanup hop stays pinned.
* Failures are ignored — the session will expire on the server side.
*/
async function closeMcpSession(url: string, sessionId: string): Promise<void> {
async function closeMcpSession(
url: string,
sessionId: string,
probeFetch: FetchLike
): Promise<void> {
try {
const controller = new AbortController()
const timer = setTimeout(() => controller.abort(), PROBE_TIMEOUT_MS)
try {
await fetch(url, {
await probeFetch(url, {
method: 'DELETE',
headers: { 'Mcp-Session-Id': sessionId },
signal: controller.signal,
+161
View File
@@ -0,0 +1,161 @@
/**
* @vitest-environment node
*
* Regression test: `revokeMcpOauthTokens` must route both metadata discovery
* and the RFC 7009 revocation POST through the SSRF-guarded fetch, since
* `revocation_endpoint` comes from attacker-controlled server metadata. Uses
* the real `createSsrfGuardedMcpFetch` so it fails if revoke.ts regresses to a
* raw `fetch`.
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
const BLOCKED_ENDPOINT = 'http://169.254.170.2/v2/credentials/'
const PUBLIC_SERVER_URL = 'https://mcp.attacker.com'
const PUBLIC_SERVER_IP = '203.0.113.10'
const {
MockAgent,
mockUndiciFetch,
mockValidateMcpServerSsrf,
mockDiscoverOAuthServerInfo,
mockLoadOauthRow,
mockDecryptSecret,
mockDbSelect,
} = vi.hoisted(() => {
class MockAgent {
close() {
return Promise.resolve()
}
}
return {
MockAgent,
mockUndiciFetch: vi.fn(),
mockValidateMcpServerSsrf: vi.fn(),
mockDiscoverOAuthServerInfo: vi.fn(),
mockLoadOauthRow: vi.fn(),
mockDecryptSecret: vi.fn(),
mockDbSelect: vi.fn(),
}
})
vi.mock('undici', () => ({ Agent: MockAgent, fetch: mockUndiciFetch }))
vi.mock('@/lib/core/security/input-validation.server', () => ({
createPinnedLookup: vi.fn(() => 'pinned-lookup-fn'),
}))
vi.mock('@/lib/mcp/domain-check', () => ({
validateMcpServerSsrf: mockValidateMcpServerSsrf,
}))
vi.mock('@modelcontextprotocol/sdk/client/auth.js', () => ({
discoverOAuthServerInfo: mockDiscoverOAuthServerInfo,
}))
vi.mock('@/lib/mcp/oauth/storage', () => ({
loadOauthRow: mockLoadOauthRow,
}))
vi.mock('@/lib/core/security/encryption', () => ({
decryptSecret: mockDecryptSecret,
}))
vi.mock('@sim/db', () => ({
db: { select: mockDbSelect },
}))
import { __resetPinnedAgentsForTests } from '@/lib/mcp/pinned-fetch'
import { revokeMcpOauthTokens } from './revoke'
function wireServerRow(row: Record<string, unknown>) {
const builder = {
from: () => builder,
where: () => builder,
limit: () => Promise.resolve([row]),
}
mockDbSelect.mockReturnValue(builder)
}
describe('revokeMcpOauthTokens — SSRF guard', () => {
beforeEach(() => {
vi.clearAllMocks()
__resetPinnedAgentsForTests()
mockLoadOauthRow.mockResolvedValue({
tokens: { access_token: 'access-secret', refresh_token: 'refresh-secret' },
clientInformation: { client_id: 'client-123' },
})
wireServerRow({
url: PUBLIC_SERVER_URL,
oauthClientId: 'client-123',
oauthClientSecret: null,
})
mockDiscoverOAuthServerInfo.mockResolvedValue({
authorizationServerMetadata: {
issuer: PUBLIC_SERVER_URL,
revocation_endpoint: BLOCKED_ENDPOINT,
},
})
mockUndiciFetch.mockResolvedValue(new Response('ok'))
// Catches a regression to raw globalThis.fetch without hitting the network.
vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('ok'))
// Public server host resolves; the revocation endpoint is blocked.
mockValidateMcpServerSsrf.mockImplementation(async (target: string) => {
if (target.startsWith(BLOCKED_ENDPOINT) || target.includes('169.254.')) {
throw new Error('MCP server URL resolves to a blocked IP address')
}
return PUBLIC_SERVER_IP
})
})
it('routes metadata discovery through the SSRF-guarded fetch', async () => {
await revokeMcpOauthTokens('server-1')
expect(mockDiscoverOAuthServerInfo).toHaveBeenCalledTimes(1)
const [, options] = mockDiscoverOAuthServerInfo.mock.calls[0]
expect(typeof options?.fetchFn).toBe('function')
})
it('validates the attacker-controlled revocation_endpoint before issuing the request', async () => {
await revokeMcpOauthTokens('server-1')
expect(mockValidateMcpServerSsrf).toHaveBeenCalledWith(BLOCKED_ENDPOINT)
})
it('never issues an outbound request to the blocked revocation endpoint', async () => {
await revokeMcpOauthTokens('server-1')
const allCalls = [
...mockUndiciFetch.mock.calls,
...(globalThis.fetch as ReturnType<typeof vi.fn>).mock.calls,
]
for (const call of allCalls) {
const target = typeof call[0] === 'string' ? call[0] : String(call[0])
expect(target).not.toContain('169.254.170.2')
}
})
it('swallows the SSRF rejection — revocation is best-effort and never throws', async () => {
await expect(revokeMcpOauthTokens('server-1')).resolves.toBeUndefined()
})
it('still issues the revocation POST when the endpoint resolves to a public IP', async () => {
const publicEndpoint = 'https://mcp.attacker.com/oauth/revoke'
mockDiscoverOAuthServerInfo.mockResolvedValue({
authorizationServerMetadata: {
issuer: PUBLIC_SERVER_URL,
revocation_endpoint: publicEndpoint,
},
})
await revokeMcpOauthTokens('server-1')
expect(mockValidateMcpServerSsrf).toHaveBeenCalledWith(publicEndpoint)
const revokeCalls = mockUndiciFetch.mock.calls.filter((call) => {
const target = typeof call[0] === 'string' ? call[0] : String(call[0])
return target === publicEndpoint
})
expect(revokeCalls.length).toBeGreaterThan(0)
expect(revokeCalls[0][1]).toMatchObject({ method: 'POST' })
})
})
@@ -86,17 +86,24 @@ export interface PerformMcpServerResult {
authType?: McpAuthType
}
async function validateMcpServerUrl(url: string): Promise<PerformMcpServerResult | null> {
type ValidateMcpServerUrlResult =
| { ok: true; resolvedIP: string | null }
| { ok: false; result: PerformMcpServerResult }
async function validateMcpServerUrl(url: string): Promise<ValidateMcpServerUrlResult> {
try {
validateMcpDomain(url)
await validateMcpServerSsrf(url)
return null
const resolvedIP = await validateMcpServerSsrf(url)
return { ok: true, resolvedIP }
} catch (error) {
if (error instanceof McpDomainNotAllowedError || error instanceof McpSsrfError) {
return { success: false, error: error.message, errorCode: 'forbidden' }
return { ok: false, result: { success: false, error: error.message, errorCode: 'forbidden' } }
}
if (error instanceof McpDnsResolutionError) {
return { success: false, error: error.message, errorCode: 'bad_gateway' }
return {
ok: false,
result: { success: false, error: error.message, errorCode: 'bad_gateway' },
}
}
throw error
}
@@ -106,7 +113,8 @@ export async function performCreateMcpServer(
params: PerformCreateMcpServerParams
): Promise<PerformMcpServerResult> {
const validation = await validateMcpServerUrl(params.url)
if (validation) return validation
if (!validation.ok) return validation.result
const validatedIP = validation.resolvedIP
const transport = params.transport || 'streamable-http'
const timeout = params.timeout || 30000
@@ -142,7 +150,7 @@ export async function performCreateMcpServer(
resolvedAuthType = (existingServer.authType ?? 'headers') as McpAuthType
} else if (params.url && !hasHeaders) {
try {
resolvedAuthType = await detectMcpAuthType(params.url)
resolvedAuthType = await detectMcpAuthType(params.url, validatedIP)
} catch (e) {
logger.warn('Probe failed, defaulting to headers', { url: params.url, error: e })
resolvedAuthType = 'headers'
@@ -281,7 +289,7 @@ export async function performUpdateMcpServer(
): Promise<PerformMcpServerResult> {
if (params.url) {
const validation = await validateMcpServerUrl(params.url)
if (validation) return validation
if (!validation.ok) return validation.result
}
const oauthClientSecretEncrypted =
@@ -0,0 +1,121 @@
import crypto from 'crypto'
import { createLogger } from '@sim/logger'
import { safeCompare } from '@sim/security/compare'
import { NextResponse } from 'next/server'
import type { AuthContext } from '@/lib/webhooks/providers/types'
const logger = createLogger('WebhookProvider:TwilioSignature')
/**
* Validate `X-Twilio-Signature`: HMAC-SHA1 over the callback URL plus each POST
* param key/value sorted alphabetically.
* @see https://www.twilio.com/docs/usage/security#validating-requests
*/
async function validateTwilioSignature(
authToken: string,
signature: string,
url: string,
params: Record<string, unknown>
): Promise<boolean> {
try {
if (!authToken || !signature || !url) {
logger.warn('Twilio signature validation missing required fields', {
hasAuthToken: !!authToken,
hasSignature: !!signature,
hasUrl: !!url,
})
return false
}
const sortedKeys = Object.keys(params).sort()
let data = url
for (const key of sortedKeys) {
data += key + params[key]
}
const encoder = new TextEncoder()
const key = await crypto.subtle.importKey(
'raw',
encoder.encode(authToken),
{ name: 'HMAC', hash: 'SHA-1' },
false,
['sign']
)
const signatureBytes = await crypto.subtle.sign('HMAC', key, encoder.encode(data))
const signatureArray = Array.from(new Uint8Array(signatureBytes))
const signatureBase64 = btoa(String.fromCharCode(...signatureArray))
return safeCompare(signatureBase64, signature)
} catch (error) {
logger.error('Error validating Twilio signature:', error)
return false
}
}
/**
* Reconstruct the public callback URL Twilio signed, recovering the original
* host/proto from forwarding headers when Sim runs behind a proxy. Forged headers
* don't help an attacker: without the auth token they can't match the signature.
*/
function getExternalUrl(request: Request): string {
const proto = request.headers.get('x-forwarded-proto') || 'https'
const host = request.headers.get('x-forwarded-host') || request.headers.get('host')
if (host) {
const url = new URL(request.url)
return `${proto}://${host}${url.pathname}${url.search}`
}
return request.url
}
/**
* Shared `verifyAuth` for Twilio webhook providers (SMS and Voice). Enforces a
* valid `X-Twilio-Signature` when an auth token is configured; skips verification
* when none is set (the provider-wide "optional secret" convention).
*/
export async function verifyTwilioAuth(
{ request, rawBody, requestId, providerConfig }: AuthContext,
providerLabel: string
): Promise<NextResponse | null> {
const authToken = providerConfig.authToken as string | undefined
if (!authToken) {
logger.warn(
`[${requestId}] ${providerLabel} webhook has no auth token configured — accepting request without signature verification. Configure an auth token to require signed requests.`
)
return null
}
const signature = request.headers.get('x-twilio-signature')
if (!signature) {
logger.warn(`[${requestId}] ${providerLabel} webhook missing signature header`)
return new NextResponse('Unauthorized - Missing Twilio signature', { status: 401 })
}
let params: Record<string, string> = {}
try {
if (typeof rawBody === 'string') {
const urlParams = new URLSearchParams(rawBody)
params = Object.fromEntries(urlParams.entries())
}
} catch (error) {
logger.error(
`[${requestId}] Error parsing ${providerLabel} webhook body for signature validation:`,
error
)
return new NextResponse('Bad Request - Invalid body format', { status: 400 })
}
const fullUrl = getExternalUrl(request)
const isValidSignature = await validateTwilioSignature(authToken, signature, fullUrl, params)
if (!isValidSignature) {
logger.warn(`[${requestId}] ${providerLabel} signature verification failed`, {
url: fullUrl,
signatureLength: signature.length,
paramsCount: Object.keys(params).length,
authTokenLength: authToken.length,
})
return new NextResponse('Unauthorized - Invalid Twilio signature', { status: 401 })
}
return null
}
@@ -0,0 +1,85 @@
/**
* @vitest-environment node
*/
import crypto from 'crypto'
import { createMockRequest } from '@sim/testing'
import { describe, expect, it } from 'vitest'
import { twilioVoiceHandler } from '@/lib/webhooks/providers/twilio-voice'
/** Twilio canonical signature: HMAC-SHA1(authToken, url + sorted(key+value)) base64. */
function signTwilio(authToken: string, url: string, params: Record<string, string>): string {
const data = Object.keys(params)
.sort()
.reduce((acc, key) => acc + key + params[key], url)
return crypto.createHmac('sha1', authToken).update(Buffer.from(data, 'utf8')).digest('base64')
}
describe('twilioVoiceHandler', () => {
describe('verifyAuth', () => {
const authToken = 'voice-auth-token'
const url = 'http://localhost:3000/api/test'
const params = { CallSid: 'CA123', From: '+15551234567', To: '+15557654321' }
const rawBody = new URLSearchParams(params).toString()
const signature = signTwilio(authToken, url, params)
it('skips verification when no auth token is configured', async () => {
const request = createMockRequest('POST', undefined, {})
const res = await twilioVoiceHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: {},
webhook: {},
workflow: {},
})
expect(res).toBeNull()
})
it('returns 401 when the signature header is missing', async () => {
const request = createMockRequest('POST', undefined, {})
const res = await twilioVoiceHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: { authToken },
webhook: {},
workflow: {},
})
expect(res?.status).toBe(401)
})
it('returns 401 when the signature is invalid', async () => {
const request = createMockRequest('POST', undefined, { 'x-twilio-signature': 'bad' })
const res = await twilioVoiceHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: { authToken },
webhook: {},
workflow: {},
})
expect(res?.status).toBe(401)
})
it('returns null when the signature is valid', async () => {
const request = createMockRequest('POST', undefined, { 'x-twilio-signature': signature })
const res = await twilioVoiceHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: { authToken },
webhook: {},
workflow: {},
})
expect(res).toBeNull()
})
})
describe('extractIdempotencyId', () => {
it('prefers MessageSid, falls back to CallSid', () => {
expect(twilioVoiceHandler.extractIdempotencyId!({ MessageSid: 'SM1' })).toBe('SM1')
expect(twilioVoiceHandler.extractIdempotencyId!({ CallSid: 'CA1' })).toBe('CA1')
expect(twilioVoiceHandler.extractIdempotencyId!({})).toBeNull()
})
})
})
+3 -114
View File
@@ -1,7 +1,5 @@
import crypto from 'crypto'
import { createLogger } from '@sim/logger'
import { safeCompare } from '@sim/security/compare'
import { NextResponse } from 'next/server'
import { verifyTwilioAuth } from '@/lib/webhooks/providers/twilio-signature'
import type {
AuthContext,
FormatInputContext,
@@ -10,118 +8,9 @@ import type {
} from '@/lib/webhooks/providers/types'
import { convertSquareBracketsToTwiML } from '@/lib/webhooks/utils'
const logger = createLogger('WebhookProvider:TwilioVoice')
async function validateTwilioSignature(
authToken: string,
signature: string,
url: string,
params: Record<string, unknown>
): Promise<boolean> {
try {
if (!authToken || !signature || !url) {
logger.warn('Twilio signature validation missing required fields', {
hasAuthToken: !!authToken,
hasSignature: !!signature,
hasUrl: !!url,
})
return false
}
const sortedKeys = Object.keys(params).sort()
let data = url
for (const key of sortedKeys) {
data += key + params[key]
}
logger.debug('Twilio signature validation string built', {
url,
sortedKeys,
dataLength: data.length,
})
const encoder = new TextEncoder()
const key = await crypto.subtle.importKey(
'raw',
encoder.encode(authToken),
{ name: 'HMAC', hash: 'SHA-1' },
false,
['sign']
)
const signatureBytes = await crypto.subtle.sign('HMAC', key, encoder.encode(data))
const signatureArray = Array.from(new Uint8Array(signatureBytes))
const signatureBase64 = btoa(String.fromCharCode(...signatureArray))
logger.debug('Twilio signature comparison', {
computedSignature: `${signatureBase64.substring(0, 10)}...`,
providedSignature: `${signature.substring(0, 10)}...`,
computedLength: signatureBase64.length,
providedLength: signature.length,
match: signatureBase64 === signature,
})
return safeCompare(signatureBase64, signature)
} catch (error) {
logger.error('Error validating Twilio signature:', error)
return false
}
}
function getExternalUrl(request: Request): string {
const proto = request.headers.get('x-forwarded-proto') || 'https'
const host = request.headers.get('x-forwarded-host') || request.headers.get('host')
if (host) {
const url = new URL(request.url)
const reconstructed = `${proto}://${host}${url.pathname}${url.search}`
return reconstructed
}
return request.url
}
export const twilioVoiceHandler: WebhookProviderHandler = {
async verifyAuth({ request, rawBody, requestId, providerConfig }: AuthContext) {
const authToken = providerConfig.authToken as string | undefined
if (authToken) {
const signature = request.headers.get('x-twilio-signature')
if (!signature) {
logger.warn(`[${requestId}] Twilio Voice webhook missing signature header`)
return new NextResponse('Unauthorized - Missing Twilio signature', {
status: 401,
})
}
let params: Record<string, string> = {}
try {
if (typeof rawBody === 'string') {
const urlParams = new URLSearchParams(rawBody)
params = Object.fromEntries(urlParams.entries())
}
} catch (error) {
logger.error(
`[${requestId}] Error parsing Twilio webhook body for signature validation:`,
error
)
return new NextResponse('Bad Request - Invalid body format', {
status: 400,
})
}
const fullUrl = getExternalUrl(request)
const isValidSignature = await validateTwilioSignature(authToken, signature, fullUrl, params)
if (!isValidSignature) {
logger.warn(`[${requestId}] Twilio Voice signature verification failed`, {
url: fullUrl,
signatureLength: signature.length,
paramsCount: Object.keys(params).length,
authTokenLength: authToken.length,
})
return new NextResponse('Unauthorized - Invalid Twilio signature', {
status: 401,
})
}
}
return null
verifyAuth(ctx: AuthContext) {
return verifyTwilioAuth(ctx, 'Twilio Voice')
},
extractIdempotencyId(body: unknown) {
@@ -0,0 +1,136 @@
/**
* @vitest-environment node
*/
import crypto from 'crypto'
import { createMockRequest } from '@sim/testing'
import { describe, expect, it } from 'vitest'
import { twilioHandler } from '@/lib/webhooks/providers/twilio'
/** Twilio canonical signature: HMAC-SHA1(authToken, url + sorted(key+value)) base64. */
function signTwilio(authToken: string, url: string, params: Record<string, string>): string {
const data = Object.keys(params)
.sort()
.reduce((acc, key) => acc + key + params[key], url)
return crypto.createHmac('sha1', authToken).update(Buffer.from(data, 'utf8')).digest('base64')
}
describe('twilioHandler', () => {
describe('verifyAuth', () => {
const authToken = 'test-auth-token'
const url = 'http://localhost:3000/api/test'
const params = { From: '+15551234567', To: '+15557654321', Body: 'hello', MessageSid: 'SM123' }
const rawBody = new URLSearchParams(params).toString()
const signature = signTwilio(authToken, url, params)
it('rejects a forged request with no signature header', async () => {
const request = createMockRequest('POST', undefined, {
'content-type': 'application/x-www-form-urlencoded',
})
const res = await twilioHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: { authToken },
webhook: {},
workflow: {},
})
expect(res?.status).toBe(401)
})
it('rejects a request with an invalid signature', async () => {
const request = createMockRequest('POST', undefined, {
'x-twilio-signature': 'not-the-real-signature',
})
const res = await twilioHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: { authToken },
webhook: {},
workflow: {},
})
expect(res?.status).toBe(401)
})
it('accepts a request with a valid signature', async () => {
const request = createMockRequest('POST', undefined, {
'x-twilio-signature': signature,
})
const res = await twilioHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: { authToken },
webhook: {},
workflow: {},
})
expect(res).toBeNull()
})
it('skips verification when no auth token is configured (optional-secret convention)', async () => {
const request = createMockRequest('POST', undefined, {})
const res = await twilioHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: {},
webhook: {},
workflow: {},
})
expect(res).toBeNull()
})
it('reconstructs the public URL from forwarding headers when validating', async () => {
const publicUrl = 'https://sim.ai/api/webhooks/trigger/twilio-sms-abc123'
const fwdSignature = signTwilio(authToken, publicUrl, params)
const request = createMockRequest(
'POST',
undefined,
{
'x-twilio-signature': fwdSignature,
'x-forwarded-proto': 'https',
'x-forwarded-host': 'sim.ai',
},
'http://internal-host:3000/api/webhooks/trigger/twilio-sms-abc123'
)
const res = await twilioHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: { authToken },
webhook: {},
workflow: {},
})
expect(res).toBeNull()
})
it('rejects a forged body even with a forwarded host (no valid token)', async () => {
const request = createMockRequest(
'POST',
undefined,
{
'x-twilio-signature': signTwilio('attacker-guess', url, params),
'x-forwarded-host': 'sim.ai',
},
url
)
const res = await twilioHandler.verifyAuth!({
request: request as any,
rawBody,
requestId: 'r1',
providerConfig: { authToken },
webhook: {},
workflow: {},
})
expect(res?.status).toBe(401)
})
})
describe('extractIdempotencyId', () => {
it('prefers MessageSid, falls back to CallSid', () => {
expect(twilioHandler.extractIdempotencyId!({ MessageSid: 'SM1' })).toBe('SM1')
expect(twilioHandler.extractIdempotencyId!({ CallSid: 'CA1' })).toBe('CA1')
expect(twilioHandler.extractIdempotencyId!({})).toBeNull()
})
})
})
+6 -1
View File
@@ -1,6 +1,11 @@
import type { WebhookProviderHandler } from '@/lib/webhooks/providers/types'
import { verifyTwilioAuth } from '@/lib/webhooks/providers/twilio-signature'
import type { AuthContext, WebhookProviderHandler } from '@/lib/webhooks/providers/types'
export const twilioHandler: WebhookProviderHandler = {
verifyAuth(ctx: AuthContext) {
return verifyTwilioAuth(ctx, 'Twilio SMS')
},
extractIdempotencyId(body: unknown) {
const obj = body as Record<string, unknown>
return (obj.MessageSid as string) || (obj.CallSid as string) || null
+11
View File
@@ -91,6 +91,17 @@ const nextConfig: NextConfig = {
'./lib/execution/sandbox/bundles/*.cjs',
],
},
turbopack: {
resolveAlias: {
// `dns/promises` has no browser shim. Server-only connector fetch logic
// (which imports `input-validation.server`) is statically reachable from
// the client bundle via the connector registry, but never runs there.
// Stub it for the browser only; the server keeps the real module so SSRF
// validation is unaffected.
'dns/promises': { browser: './lib/core/security/empty-node-fallback.browser.ts' },
dns: { browser: './lib/core/security/empty-node-fallback.browser.ts' },
},
},
experimental: {
optimizeCss: true,
preloadEntriesOnStart: false,