Skip to content

Commit 1b56990

Browse files
Merge origin/staging into dev
Integrates staging's security hardening into dev while preserving dev's newer work (191 commits ahead). Security changes brought in: SSRF pinning across connectors/MCP OAuth/knowledge document fetch, Twilio webhook signature auth, copilot credential-leak fix, and audit-log tenant scoping. Conflict resolutions (22 files): - home/chat UI, copilot lib, settings (admin/mothership/byok/billing/usage), skills, blocks registry, integrations types, logs route: kept dev's newer implementations (member credits, vfs aliases, server-side built-in skills, refactored listLogs, FileV5 block, BYOK key manager). - DB migration journal: kept dev's superset (adds 0228_smart_infant_terrible); staging introduced no new migrations. - Dropped staging's client-side sample-skills fixtures, superseded by dev's server-side built-in skills (isBuiltinSkillId + BUILTIN_SKILLS). - Restored byok-skeleton (removed by staging's "drop loading skeletons") since dev's byok-key-manager depends on it; removed the unused BYOKSkeleton export. Verified: monorepo type-check passes (16/16 workspaces). Co-authored-by: Cursor <cursoragent@cursor.com>
2 parents 991db6f + 20a00a1 commit 1b56990

36 files changed

Lines changed: 1746 additions & 282 deletions

File tree

apps/sim/app/api/audit-logs/route.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { formatAuditLogEntry } from '@/app/api/v1/audit-logs/format'
1010
import {
1111
buildFilterConditions,
1212
buildOrgScopeCondition,
13+
getOrgWorkspaceIds,
1314
queryAuditLogs,
1415
} from '@/app/api/v1/audit-logs/query'
1516

@@ -29,7 +30,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
2930
return authResult.response
3031
}
3132

32-
const { orgMemberIds } = authResult.context
33+
const { organizationId, orgMemberIds } = authResult.context
3334

3435
const parsed = await parseRequest(
3536
listAuditLogsContract,
@@ -57,7 +58,13 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
5758
cursor,
5859
} = parsed.data.query
5960

60-
const scopeCondition = await buildOrgScopeCondition(orgMemberIds, includeDeparted)
61+
const orgWorkspaceIds = await getOrgWorkspaceIds(organizationId)
62+
const scopeCondition = buildOrgScopeCondition({
63+
organizationId,
64+
orgWorkspaceIds,
65+
orgMemberIds,
66+
includeDeparted,
67+
})
6168
const filterConditions = buildFilterConditions({
6269
action,
6370
resourceType,

apps/sim/app/api/mcp/servers/test-connection/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ export const POST = withRouteHandler(
173173

174174
// Skip unauth connect when the server returns an RFC 9728 OAuth challenge.
175175
if (testConfig.url) {
176-
const detectedAuthType = await detectMcpAuthType(testConfig.url)
176+
const detectedAuthType = await detectMcpAuthType(testConfig.url, resolvedIP)
177177
if (detectedAuthType === 'oauth') {
178178
result.authRequired = true
179179
result.authType = 'oauth'
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import type { ClickHouseConnectionConfig } from '@/tools/clickhouse/types'
6+
7+
const { mockValidateDatabaseHost, mockSecureFetchWithPinnedIP, mockValidateSqlWhereClause } =
8+
vi.hoisted(() => ({
9+
mockValidateDatabaseHost: vi.fn(),
10+
mockSecureFetchWithPinnedIP: vi.fn(),
11+
mockValidateSqlWhereClause: vi.fn(),
12+
}))
13+
14+
vi.mock('@/lib/core/security/input-validation.server', () => ({
15+
validateDatabaseHost: mockValidateDatabaseHost,
16+
secureFetchWithPinnedIP: mockSecureFetchWithPinnedIP,
17+
validateSqlWhereClause: mockValidateSqlWhereClause,
18+
}))
19+
20+
import { executeClickHouseInsert, executeClickHouseQuery } from '@/app/api/tools/clickhouse/utils'
21+
22+
function makeConfig(
23+
overrides: Partial<ClickHouseConnectionConfig> = {}
24+
): ClickHouseConnectionConfig {
25+
return {
26+
host: 'clickhouse.example.com',
27+
port: 8123,
28+
database: 'default',
29+
username: 'default',
30+
password: 'secret',
31+
secure: false,
32+
...overrides,
33+
}
34+
}
35+
36+
function okResponse(body: string, summary?: string) {
37+
return {
38+
ok: true,
39+
status: 200,
40+
statusText: 'OK',
41+
text: async () => body,
42+
headers: {
43+
get: (name: string) =>
44+
name.toLowerCase() === 'x-clickhouse-summary' ? (summary ?? null) : null,
45+
},
46+
}
47+
}
48+
49+
describe('clickhouseRequest DNS pinning', () => {
50+
beforeEach(() => {
51+
vi.clearAllMocks()
52+
mockValidateDatabaseHost.mockResolvedValue({
53+
isValid: true,
54+
resolvedIP: '93.184.216.34',
55+
originalHostname: 'clickhouse.example.com',
56+
})
57+
mockValidateSqlWhereClause.mockReturnValue({ isValid: true })
58+
mockSecureFetchWithPinnedIP.mockResolvedValue(okResponse('{"data":[{"x":1}],"rows":1}'))
59+
})
60+
61+
it('pins the connection to the validated IP, not the attacker-controlled hostname', async () => {
62+
await executeClickHouseQuery(makeConfig({ host: 'rebind.attacker.example' }), 'SELECT 1')
63+
64+
expect(mockValidateDatabaseHost).toHaveBeenCalledWith('rebind.attacker.example', 'host')
65+
expect(mockSecureFetchWithPinnedIP).toHaveBeenCalledTimes(1)
66+
67+
const [url, pinnedIP, options] = mockSecureFetchWithPinnedIP.mock.calls[0]
68+
// The actual TCP target is the validated IP — re-resolution of the hostname can never happen.
69+
expect(pinnedIP).toBe('93.184.216.34')
70+
// The hostname is preserved only in the URL (for Host header / TLS SNI), never used to connect.
71+
expect(url).toContain('rebind.attacker.example')
72+
expect(options.method).toBe('POST')
73+
})
74+
75+
it('never issues the request when host validation fails (no SSRF window)', async () => {
76+
mockValidateDatabaseHost.mockResolvedValue({
77+
isValid: false,
78+
error: 'host resolves to a blocked IP address',
79+
})
80+
81+
await expect(executeClickHouseQuery(makeConfig(), 'SELECT 1')).rejects.toThrow(
82+
'host resolves to a blocked IP address'
83+
)
84+
expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
85+
})
86+
87+
it('uses https and disallows http redirects when secure is true', async () => {
88+
await executeClickHouseQuery(makeConfig({ secure: true, port: 8443 }), 'SELECT 1')
89+
90+
const [url, , options] = mockSecureFetchWithPinnedIP.mock.calls[0]
91+
expect(url).toMatch(/^https:\/\//)
92+
expect(options.allowHttp).toBe(false)
93+
})
94+
95+
it('allows http for the initial request when secure is false', async () => {
96+
await executeClickHouseQuery(makeConfig({ secure: false }), 'SELECT 1')
97+
98+
const [url, , options] = mockSecureFetchWithPinnedIP.mock.calls[0]
99+
expect(url).toMatch(/^http:\/\//)
100+
expect(options.allowHttp).toBe(true)
101+
})
102+
103+
it('sends the statement as the body with a matching Content-Length and auth headers', async () => {
104+
await executeClickHouseInsert(makeConfig(), 'events', { id: 1 })
105+
106+
const [, , options] = mockSecureFetchWithPinnedIP.mock.calls[0]
107+
expect(options.body).toContain('INSERT INTO `events` FORMAT JSONEachRow')
108+
expect(options.headers['Content-Length']).toBe(String(Buffer.byteLength(options.body, 'utf-8')))
109+
expect(options.headers['X-ClickHouse-User']).toBe('default')
110+
expect(options.headers['X-ClickHouse-Key']).toBe('secret')
111+
})
112+
113+
it('propagates non-ok responses as errors with the body text', async () => {
114+
mockSecureFetchWithPinnedIP.mockResolvedValue({
115+
ok: false,
116+
status: 400,
117+
statusText: 'Bad Request',
118+
text: async () => 'Code: 62. DB::Exception: Syntax error',
119+
headers: { get: () => null },
120+
})
121+
122+
await expect(executeClickHouseQuery(makeConfig(), 'SELECT 1')).rejects.toThrow(
123+
'Code: 62. DB::Exception: Syntax error'
124+
)
125+
})
126+
})

apps/sim/app/api/tools/clickhouse/utils.ts

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
secureFetchWithPinnedIP,
23
validateDatabaseHost,
34
validateSqlWhereClause,
45
} from '@/lib/core/security/input-validation.server'
@@ -81,24 +82,21 @@ async function clickhouseRequest(
8182
url.searchParams.set('readonly', '1')
8283
}
8384

84-
const controller = new AbortController()
85-
const timeout = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS)
86-
87-
let response: Response
88-
try {
89-
response = await fetch(url.toString(), {
90-
method: 'POST',
91-
headers: {
92-
'X-ClickHouse-User': config.username,
93-
'X-ClickHouse-Key': config.password,
94-
'Content-Type': 'text/plain; charset=utf-8',
95-
},
96-
body: statement,
97-
signal: controller.signal,
98-
})
99-
} finally {
100-
clearTimeout(timeout)
101-
}
85+
// Pin the connection to the IP that passed validation. Without this, fetch()
86+
// would re-resolve `config.host` and a DNS-rebinding hostname could point the
87+
// actual request at an internal/private address after validation succeeded.
88+
const response = await secureFetchWithPinnedIP(url.toString(), hostValidation.resolvedIP!, {
89+
method: 'POST',
90+
headers: {
91+
'X-ClickHouse-User': config.username,
92+
'X-ClickHouse-Key': config.password,
93+
'Content-Type': 'text/plain; charset=utf-8',
94+
'Content-Length': String(Buffer.byteLength(statement, 'utf-8')),
95+
},
96+
body: statement,
97+
timeout: REQUEST_TIMEOUT_MS,
98+
allowHttp: !config.secure,
99+
})
102100

103101
const text = await response.text()
104102

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Tests for GET /api/v1/audit-logs/[id] — verifies the lookup is constrained
5+
* by the organization scope and 404s for rows outside it.
6+
*/
7+
import { createMockRequest, dbChainMock, dbChainMockFns } from '@sim/testing'
8+
import { beforeEach, describe, expect, it, vi } from 'vitest'
9+
10+
const {
11+
mockCheckRateLimit,
12+
mockValidateEnterpriseAuditAccess,
13+
mockBuildOrgScopeCondition,
14+
mockGetOrgWorkspaceIds,
15+
} = vi.hoisted(() => ({
16+
mockCheckRateLimit: vi.fn(),
17+
mockValidateEnterpriseAuditAccess: vi.fn(),
18+
mockBuildOrgScopeCondition: vi.fn(),
19+
mockGetOrgWorkspaceIds: vi.fn(),
20+
}))
21+
22+
vi.mock('@sim/db', () => dbChainMock)
23+
24+
vi.mock('@/app/api/v1/middleware', () => ({
25+
checkRateLimit: mockCheckRateLimit,
26+
createRateLimitResponse: vi.fn(),
27+
}))
28+
29+
vi.mock('@/app/api/v1/audit-logs/auth', () => ({
30+
validateEnterpriseAuditAccess: mockValidateEnterpriseAuditAccess,
31+
}))
32+
33+
vi.mock('@/app/api/v1/audit-logs/query', () => ({
34+
buildOrgScopeCondition: mockBuildOrgScopeCondition,
35+
getOrgWorkspaceIds: mockGetOrgWorkspaceIds,
36+
}))
37+
38+
vi.mock('@/app/api/v1/logs/meta', () => ({
39+
getUserLimits: vi.fn().mockResolvedValue({}),
40+
createApiResponse: vi.fn((body: unknown) => ({ body, headers: {} })),
41+
}))
42+
43+
import { GET } from '@/app/api/v1/audit-logs/[id]/route'
44+
45+
const ORG_ID = 'org-1'
46+
const MEMBER_IDS = ['admin-1', 'member-1']
47+
const ORG_WORKSPACE_IDS = ['ws-org-1']
48+
const SCOPE_SENTINEL = { type: 'org-scope-sentinel' }
49+
50+
const AUDIT_ROW = {
51+
id: 'log-1',
52+
workspaceId: 'ws-org-1',
53+
actorId: 'member-1',
54+
actorName: 'Member',
55+
actorEmail: 'member@example.com',
56+
action: 'workflow.created',
57+
resourceType: 'workflow',
58+
resourceId: 'wf-1',
59+
resourceName: 'My Workflow',
60+
description: 'Created workflow',
61+
metadata: {},
62+
ipAddress: '127.0.0.1',
63+
userAgent: 'test',
64+
createdAt: new Date('2026-01-01T00:00:00Z'),
65+
}
66+
67+
function callRoute(id: string) {
68+
const request = createMockRequest(
69+
'GET',
70+
undefined,
71+
{},
72+
`http://localhost:3000/api/v1/audit-logs/${id}`
73+
)
74+
return GET(request, { params: Promise.resolve({ id }) })
75+
}
76+
77+
describe('GET /api/v1/audit-logs/[id]', () => {
78+
beforeEach(() => {
79+
vi.clearAllMocks()
80+
mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'admin-1' })
81+
mockValidateEnterpriseAuditAccess.mockResolvedValue({
82+
success: true,
83+
context: { organizationId: ORG_ID, orgMemberIds: MEMBER_IDS },
84+
})
85+
mockGetOrgWorkspaceIds.mockResolvedValue(ORG_WORKSPACE_IDS)
86+
mockBuildOrgScopeCondition.mockReturnValue(SCOPE_SENTINEL)
87+
})
88+
89+
it('constrains the lookup with the org scope condition (includeDeparted)', async () => {
90+
dbChainMockFns.limit.mockResolvedValueOnce([AUDIT_ROW])
91+
92+
const response = await callRoute('log-1')
93+
94+
expect(response.status).toBe(200)
95+
expect(mockBuildOrgScopeCondition).toHaveBeenCalledWith({
96+
organizationId: ORG_ID,
97+
orgWorkspaceIds: ORG_WORKSPACE_IDS,
98+
orgMemberIds: MEMBER_IDS,
99+
includeDeparted: true,
100+
})
101+
expect(dbChainMockFns.where).toHaveBeenCalledWith(
102+
expect.objectContaining({
103+
type: 'and',
104+
conditions: expect.arrayContaining([SCOPE_SENTINEL]),
105+
})
106+
)
107+
})
108+
109+
it('returns 404 when the row is outside the organization scope', async () => {
110+
dbChainMockFns.limit.mockResolvedValueOnce([])
111+
112+
const response = await callRoute('log-outside-org')
113+
114+
expect(response.status).toBe(404)
115+
const body = await response.json()
116+
expect(body.error).toBe('Audit log not found')
117+
})
118+
119+
it('excludes ipAddress and userAgent from the response', async () => {
120+
dbChainMockFns.limit.mockResolvedValueOnce([AUDIT_ROW])
121+
122+
const response = await callRoute('log-1')
123+
const body = await response.json()
124+
125+
expect(body.data.id).toBe('log-1')
126+
expect(body.data.ipAddress).toBeUndefined()
127+
expect(body.data.userAgent).toBeUndefined()
128+
})
129+
})

0 commit comments

Comments
 (0)