Skip to content

Commit 67e3cab

Browse files
committed
fix(elasticsearch): register an error extractor so auth headers stay out of errors
A 401 dumped the whole error blob, including WWW-Authenticate, into the user-visible message. Replaces a tautological assertion on transformResponse.toString() with one that drives executeTool and proves the executor throws on a 404 before transformResponse runs.
1 parent 3d3c6dc commit 67e3cab

12 files changed

Lines changed: 401 additions & 17 deletions

File tree

apps/sim/app/api/tools/google_drive/export/path-safety.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ beforeEach(() => {
8484

8585
describe('POST /api/tools/google_drive/export traversal safety', () => {
8686
it.each(REJECTED)('rejects fileId %j with a clean 400 and no outbound request', async (fileId) => {
87+
mockSecureFetchWithPinnedIP
88+
.mockResolvedValueOnce(metadataResponse('doc-1'))
89+
.mockResolvedValueOnce(exportResponse())
90+
8791
const response = await POST(createMockRequest('POST', bodyFor(fileId)))
8892

8993
expect(response.status).toBe(400)

apps/sim/app/api/tools/google_drive/export/route.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
MAX_EXPORT_BYTES,
1818
VALID_EXPORT_FORMATS,
1919
} from '@/tools/google_drive/utils'
20+
import { safeUrlPathSegment } from '@/tools/url-path'
2021

2122
export const dynamic = 'force-dynamic'
2223

@@ -62,9 +63,22 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6263
const { accessToken, fileId, mimeType: exportMimeType, fileName } = parsed.data.body
6364
const authHeader = `Bearer ${accessToken}`
6465

66+
let fileIdSegment: string
67+
try {
68+
fileIdSegment = safeUrlPathSegment(fileId, 'fileId')
69+
} catch (error) {
70+
logger.warn(`[${requestId}] Rejected unsafe fileId`, {
71+
error: getErrorMessage(error, 'Invalid fileId'),
72+
})
73+
return NextResponse.json(
74+
{ success: false, error: getErrorMessage(error, 'Invalid fileId') },
75+
{ status: 400 }
76+
)
77+
}
78+
6579
logger.info(`[${requestId}] Getting file metadata from Google Drive`, { fileId })
6680

67-
const metadataUrl = `https://www.googleapis.com/drive/v3/files/${encodeURIComponent(fileId)}?fields=${ALL_FILE_FIELDS}&supportsAllDrives=true`
81+
const metadataUrl = `https://www.googleapis.com/drive/v3/files/${fileIdSegment}?fields=${ALL_FILE_FIELDS}&supportsAllDrives=true`
6882
const metadataUrlValidation = await validateUrlWithDNS(metadataUrl, 'metadataUrl')
6983
if (!metadataUrlValidation.isValid) {
7084
return NextResponse.json(
@@ -123,7 +137,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
123137
exportFormat: exportMimeType,
124138
})
125139

126-
const exportUrl = `https://www.googleapis.com/drive/v3/files/${encodeURIComponent(fileId)}/export?mimeType=${encodeURIComponent(exportMimeType)}`
140+
const exportUrl = `https://www.googleapis.com/drive/v3/files/${fileIdSegment}/export?mimeType=${encodeURIComponent(exportMimeType)}`
127141
const exportUrlValidation = await validateUrlWithDNS(exportUrl, 'exportUrl')
128142
if (!exportUrlValidation.isValid) {
129143
return NextResponse.json(
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Guards the Google Vault export-file download route against a dot-segment
5+
* pop, WITHOUT weakening its `%2F` encoding.
6+
*
7+
* A GCS object name legitimately contains `/`, and the JSON API requires it
8+
* percent-encoded as `%2F` inside the single `/o/{object}` segment. So
9+
* `safeUrlPath` (which preserves `/` as a separator) would be the wrong tool
10+
* here and `encodeURIComponent` must stay. What `encodeURIComponent` does not
11+
* stop is a value that is exactly `.` or `..`: those characters are
12+
* unreserved, survive encoding, and the WHATWG parser removes the segment
13+
* afterwards — `/b/{bucket}/o/..` resolves to `/b/{bucket}/`, the object
14+
* *list* endpoint, with the caller's bearer token attached.
15+
*
16+
* Only the whole value is checked, not each `/`-separated component: because
17+
* the value becomes one `%2F`-encoded segment, an interior `..` never forms a
18+
* URL segment and an object literally named `a/../b` is legally addressable.
19+
*
20+
* `objectName` and `bucketName` are `visibility: 'user-only'` on
21+
* `google_vault_download_export_file`, so this is not LLM-reachable — lower
22+
* severity than the Drive `fileId` sites, fixed for the same reason.
23+
*/
24+
import {
25+
createMockRequest,
26+
hybridAuthMockFns,
27+
inputValidationMock,
28+
inputValidationMockFns,
29+
} from '@sim/testing'
30+
import { beforeEach, describe, expect, it, vi } from 'vitest'
31+
32+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
33+
34+
import { POST } from '@/app/api/tools/google_vault/download-export-file/route'
35+
36+
const { mockValidateUrlWithDNS, mockSecureFetchWithPinnedIP } = inputValidationMockFns
37+
38+
const PINNED_IP = '93.184.216.34'
39+
const REJECTED = ['..', '.', ' .. '] as const
40+
41+
function downloadResponse() {
42+
return {
43+
ok: true,
44+
status: 200,
45+
statusText: '',
46+
headers: new Headers({ 'content-type': 'application/zip' }),
47+
body: null,
48+
text: async () => '',
49+
json: async () => ({}),
50+
arrayBuffer: async () => new ArrayBuffer(4),
51+
}
52+
}
53+
54+
function requestedUrl(): string {
55+
return String(mockValidateUrlWithDNS.mock.calls[0][0])
56+
}
57+
58+
beforeEach(() => {
59+
vi.clearAllMocks()
60+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
61+
success: true,
62+
userId: 'user-1',
63+
authType: 'internal_jwt',
64+
})
65+
mockValidateUrlWithDNS.mockResolvedValue({
66+
isValid: true,
67+
resolvedIP: PINNED_IP,
68+
originalHostname: 'storage.googleapis.com',
69+
})
70+
})
71+
72+
describe('POST /api/tools/google_vault/download-export-file traversal safety', () => {
73+
it.each(REJECTED)(
74+
'rejects objectName %j with a clean 400 and no outbound request',
75+
async (objectName) => {
76+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(downloadResponse())
77+
78+
const response = await POST(
79+
createMockRequest('POST', {
80+
accessToken: 'token-123',
81+
matterId: 'matter-1',
82+
bucketName: 'vault-bucket',
83+
objectName,
84+
})
85+
)
86+
87+
expect(response.status).toBe(400)
88+
const data = (await response.json()) as { success: boolean; error: string }
89+
expect(data.success).toBe(false)
90+
expect(data.error).toMatch(/objectName/)
91+
expect(mockValidateUrlWithDNS).not.toHaveBeenCalled()
92+
expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
93+
}
94+
)
95+
96+
it.each(REJECTED)(
97+
'rejects bucketName %j with a clean 400 and no outbound request',
98+
async (bucketName) => {
99+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(downloadResponse())
100+
101+
const response = await POST(
102+
createMockRequest('POST', {
103+
accessToken: 'token-123',
104+
matterId: 'matter-1',
105+
bucketName,
106+
objectName: 'exports/file.zip',
107+
})
108+
)
109+
110+
expect(response.status).toBe(400)
111+
const data = (await response.json()) as { success: boolean; error: string }
112+
expect(data.success).toBe(false)
113+
expect(data.error).toMatch(/bucketName/)
114+
expect(mockValidateUrlWithDNS).not.toHaveBeenCalled()
115+
expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
116+
}
117+
)
118+
119+
it('keeps a nested object name as one %2F-encoded segment, byte-identical to today', async () => {
120+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(downloadResponse())
121+
122+
const response = await POST(
123+
createMockRequest('POST', {
124+
accessToken: 'token-123',
125+
matterId: 'matter-1',
126+
bucketName: 'vault-bucket',
127+
objectName: 'matter-1/exports/file name.zip',
128+
})
129+
)
130+
expect(response.status).toBe(200)
131+
132+
expect(requestedUrl()).toBe(
133+
'https://storage.googleapis.com/storage/v1/b/vault-bucket/o/matter-1%2Fexports%2Ffile%20name.zip?alt=media'
134+
)
135+
const segments = new URL(requestedUrl()).pathname.split('/').filter(Boolean)
136+
expect(segments).toEqual([
137+
'storage',
138+
'v1',
139+
'b',
140+
'vault-bucket',
141+
'o',
142+
'matter-1%2Fexports%2Ffile%20name.zip',
143+
])
144+
})
145+
146+
it('preserves an interior ".." component, which never forms a URL segment', async () => {
147+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce(downloadResponse())
148+
149+
const response = await POST(
150+
createMockRequest('POST', {
151+
accessToken: 'token-123',
152+
matterId: 'matter-1',
153+
bucketName: 'vault-bucket',
154+
objectName: 'a/../b',
155+
})
156+
)
157+
expect(response.status).toBe(200)
158+
159+
const segments = new URL(requestedUrl()).pathname.split('/').filter(Boolean)
160+
expect(segments).toEqual(['storage', 'v1', 'b', 'vault-bucket', 'o', 'a%2F..%2Fb'])
161+
})
162+
})

apps/sim/app/api/tools/google_vault/download-export-file/route.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,29 @@ export const dynamic = 'force-dynamic'
1616

1717
const logger = createLogger('GoogleVaultDownloadExportFileAPI')
1818

19+
/**
20+
* Rejects a value that would collapse the URL path segment it occupies.
21+
*
22+
* This route deliberately keeps `encodeURIComponent` on the object name: a GCS
23+
* object name legitimately contains `/`, and the JSON API addresses it as a
24+
* single segment with those slashes as `%2F`, so the multi-segment
25+
* `safeUrlPath` helper would misaddress the object. But encoding never
26+
* neutralizes a dot segment — `.` and `..` are unreserved, so they survive
27+
* encoding and the WHATWG parser removes the segment afterwards, turning
28+
* `/b/{bucket}/o/..` into the object *list* endpoint with the caller's bearer
29+
* token attached. Only rejection closes that, and only the whole value can do
30+
* it: an interior `..` is encoded into the same segment and stays inert.
31+
*
32+
* The check trims before comparing but the caller still sends the untrimmed
33+
* value, so no legitimate name is silently rewritten.
34+
*/
35+
function assertNotDotSegment(value: string, paramName: string): void {
36+
const trimmed = value.trim()
37+
if (trimmed === '.' || trimmed === '..') {
38+
throw new Error(`${paramName} cannot be "${trimmed}" (path traversal is not allowed)`)
39+
}
40+
}
41+
1942
export const POST = withRouteHandler(async (request: NextRequest) => {
2043
const requestId = generateRequestId()
2144

@@ -39,6 +62,15 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
3962

4063
const { accessToken, bucketName, objectName, fileName } = validatedData
4164

65+
try {
66+
assertNotDotSegment(bucketName, 'bucketName')
67+
assertNotDotSegment(objectName, 'objectName')
68+
} catch (error) {
69+
const message = getErrorMessage(error, 'Invalid request')
70+
logger.warn(`[${requestId}] Rejected unsafe Vault object path`, { error: message })
71+
return NextResponse.json({ success: false, error: message }, { status: 400 })
72+
}
73+
4274
const bucket = encodeURIComponent(bucketName)
4375
const object = encodeURIComponent(objectName)
4476
const downloadUrl = `https://storage.googleapis.com/storage/v1/b/${bucket}/o/${object}?alt=media`

apps/sim/blocks/blocks/serper.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export const SerperBlock: BlockConfig<SearchResponse> = {
111111
searchResults: {
112112
type: 'json',
113113
description:
114-
'Results for the requested vertical: [{title, link, snippet, position, date, imageUrl, thumbnailUrl, source, channel, rating, ratingCount, address, latitude, longitude, category, phoneNumber, website, price, delivery, duration}]. Only title and position are always present; the rest depend on the vertical'
114+
'Results for the requested vertical: [{title, link, snippet, position, date, imageUrl, thumbnailUrl, source, channel, rating, ratingCount, address, latitude, longitude, category, phoneNumber, website, price, delivery, duration}]. Only title and position are always present; the rest depend on the vertical',
115115
},
116116
knowledgeGraph: {
117117
type: 'json',

apps/sim/tools/cloudflare/cloudflare.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,31 @@ describe('zone settings are read through the endpoints Cloudflare still supports
998998
expect(out.error).toBe('Invalid zone identifier')
999999
})
10001000

1001+
/**
1002+
* The zone id is guarded ONCE above the fan-out. Re-guarding it per setting
1003+
* raised the same error up to 40 times and filled `unreadable` with 40
1004+
* identical rows before the call failed.
1005+
*/
1006+
it('reports a bad zone id once, with no requests and no unreadable rows', async () => {
1007+
const fetchMock = vi.spyOn(globalThis, 'fetch')
1008+
1009+
const out = (await tool.directExecution!({
1010+
zoneId: '..',
1011+
apiKey,
1012+
settingIds: Array.from({ length: 40 }, (_, index) => `setting_${index}`).join(','),
1013+
} as never)) as {
1014+
success: boolean
1015+
error?: string
1016+
output: { settings: unknown[]; unreadable: unknown[] }
1017+
}
1018+
1019+
expect(out.success).toBe(false)
1020+
expect(out.error).toMatch(/zoneId/)
1021+
expect(out.output.unreadable).toEqual([])
1022+
expect(out.output.settings).toEqual([])
1023+
expect(fetchMock).not.toHaveBeenCalled()
1024+
})
1025+
10011026
it('refuses an unbounded fan-out instead of issuing the requests', async () => {
10021027
const fetchMock = vi.spyOn(globalThis, 'fetch')
10031028

apps/sim/tools/cloudflare/get_zone_settings.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,16 @@ import {
1515
import type { ToolConfig } from '@/tools/types'
1616
import { safeUrlPathSegment } from '@/tools/url-path'
1717

18-
/** Builds the per-setting endpoint Cloudflare directs integrations at. */
19-
function zoneSettingUrl(zoneId: string, settingId: string): string {
20-
return `https://api.cloudflare.com/client/v4/zones/${safeUrlPathSegment(zoneId, 'zoneId')}/settings/${safeUrlPathSegment(settingId, 'settingIds')}`
18+
/**
19+
* Builds the per-setting endpoint Cloudflare directs integrations at.
20+
*
21+
* `zoneId` arrives already guarded — see {@link zoneSettingUrl}'s caller in
22+
* `directExecution`, which validates it once above the fan-out. Re-guarding it inside the fan-out
23+
* would raise the same error once per requested setting, turning one bad zone id into up to
24+
* {@link MAX_ZONE_SETTING_IDS} identical `unreadable` rows before the call failed.
25+
*/
26+
function zoneSettingUrl(guardedZoneId: string, settingId: string): string {
27+
return `https://api.cloudflare.com/client/v4/zones/${guardedZoneId}/settings/${safeUrlPathSegment(settingId, 'settingIds')}`
2128
}
2229

2330
/**
@@ -69,7 +76,11 @@ export const getZoneSettingsTool: ToolConfig<
6976
},
7077

7178
request: {
72-
url: (params) => zoneSettingUrl(params.zoneId, requestedZoneSettingIds(params.settingIds)[0]),
79+
url: (params) =>
80+
zoneSettingUrl(
81+
safeUrlPathSegment(params.zoneId, 'zoneId'),
82+
requestedZoneSettingIds(params.settingIds)[0]
83+
),
7384
method: 'GET',
7485
headers: (params) => cloudflareHeaders(params.apiKey),
7586
},
@@ -96,7 +107,17 @@ export const getZoneSettingsTool: ToolConfig<
96107
}
97108
}
98109

99-
const zoneId = params.zoneId
110+
let zoneId: string
111+
try {
112+
zoneId = safeUrlPathSegment(params.zoneId, 'zoneId')
113+
} catch (error) {
114+
return {
115+
success: false,
116+
output: { settings: [], unreadable: [] },
117+
error: getErrorMessage(error, 'Invalid zoneId'),
118+
}
119+
}
120+
100121
const headers = cloudflareHeaders(params.apiKey)
101122

102123
const reads = await Promise.all(

0 commit comments

Comments
 (0)