Skip to content

Commit 2b06b80

Browse files
waleedlatif1claude
andcommitted
fix(build): keep agiloft/grafana tool configs client-safe
Tool config files are statically reachable from the client bundle (via tools/registry.ts → tools/{service}/index.ts). Importing `@/lib/core/security/input-validation.server` from these files pulled `node:dns/promises` into the Turbopack client bundle and broke the build. Split agiloft utils into client-safe (`utils.ts`, plain fetch + sync `validateExternalUrl`) and server-only (`utils.server.ts`, DNS-pinned variants). Routes that need TOCTOU protection import the pinned helpers; the executor-side tool path falls back to sync URL validation (matches the supabase precedent and pre-PR baseline). Grafana update tools likewise switch from `secureFetchWithValidation` (server-only) to inline sync `validateExternalUrl` + plain fetch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 9fe9cb2 commit 2b06b80

7 files changed

Lines changed: 165 additions & 167 deletions

File tree

apps/sim/app/api/tools/agiloft/attach/route.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ import type { RawFileInput } from '@/lib/uploads/utils/file-schemas'
1111
import { processFilesToUserFiles } from '@/lib/uploads/utils/file-utils'
1212
import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server'
1313
import { assertToolFileAccess } from '@/app/api/files/authorization'
14+
import { buildAttachFileUrl } from '@/tools/agiloft/utils'
1415
import {
15-
agiloftLogin,
16-
agiloftLogout,
17-
buildAttachFileUrl,
16+
agiloftLoginPinned,
17+
agiloftLogoutPinned,
1818
resolveAgiloftInstance,
19-
} from '@/tools/agiloft/utils'
19+
} from '@/tools/agiloft/utils.server'
2020

2121
export const dynamic = 'force-dynamic'
2222

@@ -87,7 +87,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8787
return NextResponse.json({ success: false, error: toError(error).message }, { status: 400 })
8888
}
8989

90-
const token = await agiloftLogin(data, resolvedIP)
90+
const token = await agiloftLoginPinned(data, resolvedIP)
9191
const base = data.instanceUrl.replace(/\/$/, '')
9292

9393
try {
@@ -139,7 +139,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
139139
},
140140
})
141141
} finally {
142-
await agiloftLogout(data.instanceUrl, data.knowledgeBase, token, resolvedIP)
142+
await agiloftLogoutPinned(data.instanceUrl, data.knowledgeBase, token, resolvedIP)
143143
}
144144
} catch (error) {
145145
logger.error(`[${requestId}] Error attaching file to Agiloft:`, error)

apps/sim/app/api/tools/agiloft/retrieve/route.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ import { checkInternalAuth } from '@/lib/auth/hybrid'
77
import { secureFetchWithPinnedIP } from '@/lib/core/security/input-validation.server'
88
import { generateRequestId } from '@/lib/core/utils/request'
99
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
10+
import { buildRetrieveAttachmentUrl } from '@/tools/agiloft/utils'
1011
import {
11-
agiloftLogin,
12-
agiloftLogout,
13-
buildRetrieveAttachmentUrl,
12+
agiloftLoginPinned,
13+
agiloftLogoutPinned,
1414
resolveAgiloftInstance,
15-
} from '@/tools/agiloft/utils'
15+
} from '@/tools/agiloft/utils.server'
1616

1717
export const dynamic = 'force-dynamic'
1818

@@ -63,7 +63,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6363
return NextResponse.json({ success: false, error: toError(error).message }, { status: 400 })
6464
}
6565

66-
const token = await agiloftLogin(data, resolvedIP)
66+
const token = await agiloftLoginPinned(data, resolvedIP)
6767
const base = data.instanceUrl.replace(/\/$/, '')
6868

6969
try {
@@ -127,7 +127,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
127127
},
128128
})
129129
} finally {
130-
await agiloftLogout(data.instanceUrl, data.knowledgeBase, token, resolvedIP)
130+
await agiloftLogoutPinned(data.instanceUrl, data.knowledgeBase, token, resolvedIP)
131131
}
132132
} catch (error) {
133133
logger.error(`[${requestId}] Error retrieving Agiloft attachment:`, error)
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { createLogger } from '@sim/logger'
2+
import {
3+
type SecureFetchResponse,
4+
secureFetchWithPinnedIP,
5+
validateUrlWithDNS,
6+
} from '@/lib/core/security/input-validation.server'
7+
import type { AgiloftBaseParams } from '@/tools/agiloft/types'
8+
9+
const logger = createLogger('AgiloftAuthServer')
10+
11+
/**
12+
* Validates the Agiloft instance URL and resolves its DNS once, returning the
13+
* resolved IP so subsequent requests can pin to it. This prevents DNS-rebinding
14+
* (TOCTOU) SSRF where the hostname could resolve to a private IP on a later
15+
* lookup. Server-only — uses node:dns/promises.
16+
*/
17+
export async function resolveAgiloftInstance(instanceUrl: string): Promise<string> {
18+
const validation = await validateUrlWithDNS(instanceUrl, 'instanceUrl')
19+
if (!validation.isValid || !validation.resolvedIP) {
20+
throw new Error(validation.error || 'Invalid Agiloft instance URL')
21+
}
22+
return validation.resolvedIP
23+
}
24+
25+
/**
26+
* DNS-pinned variant of agiloftLogin. Requires a pre-resolved IP so the
27+
* connection cannot be steered to a different host between validation and
28+
* the actual TCP connection.
29+
*/
30+
export async function agiloftLoginPinned(
31+
params: AgiloftBaseParams,
32+
resolvedIP: string
33+
): Promise<string> {
34+
const base = params.instanceUrl.replace(/\/$/, '')
35+
const kb = encodeURIComponent(params.knowledgeBase)
36+
const login = encodeURIComponent(params.login)
37+
const password = encodeURIComponent(params.password)
38+
39+
const url = `${base}/ewws/EWLogin?$KB=${kb}&$login=${login}&$password=${password}`
40+
const response = await secureFetchWithPinnedIP(url, resolvedIP, { method: 'POST' })
41+
42+
if (!response.ok) {
43+
const errorText = await response.text()
44+
throw new Error(`Agiloft login failed: ${response.status} - ${errorText}`)
45+
}
46+
47+
const data = (await response.json()) as { access_token?: string }
48+
const token = data.access_token
49+
50+
if (!token) {
51+
throw new Error('Agiloft login did not return an access token')
52+
}
53+
54+
return token
55+
}
56+
57+
/**
58+
* DNS-pinned variant of agiloftLogout. Best-effort — failures are logged but
59+
* not thrown.
60+
*/
61+
export async function agiloftLogoutPinned(
62+
instanceUrl: string,
63+
knowledgeBase: string,
64+
token: string,
65+
resolvedIP: string
66+
): Promise<void> {
67+
try {
68+
const base = instanceUrl.replace(/\/$/, '')
69+
const kb = encodeURIComponent(knowledgeBase)
70+
await secureFetchWithPinnedIP(`${base}/ewws/EWLogout?$KB=${kb}`, resolvedIP, {
71+
method: 'POST',
72+
headers: { Authorization: `Bearer ${token}` },
73+
})
74+
} catch (error) {
75+
logger.warn('Agiloft logout failed (best-effort)', { error })
76+
}
77+
}
78+
79+
export type { SecureFetchResponse }
Lines changed: 32 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { inputValidationMock, inputValidationMockFns } from '@sim/testing'
5-
import { beforeEach, describe, expect, it, vi } from 'vitest'
6-
7-
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
8-
9-
import { executeAgiloftRequest, resolveAgiloftInstance } from '@/tools/agiloft/utils'
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { executeAgiloftRequest } from '@/tools/agiloft/utils'
106

117
const baseParams = {
128
instanceUrl: 'https://example.agiloft.com',
@@ -16,77 +12,34 @@ const baseParams = {
1612
table: 'contracts',
1713
}
1814

19-
const PINNED_IP = '93.184.216.34'
20-
21-
function mockSecureFetchResponse(body: {
22-
ok?: boolean
23-
status?: number
24-
json?: unknown
25-
text?: string
26-
arrayBuffer?: ArrayBuffer
27-
}) {
15+
function mockFetchResponse(body: { ok?: boolean; status?: number; json?: unknown; text?: string }) {
2816
return {
2917
ok: body.ok ?? true,
3018
status: body.status ?? 200,
3119
statusText: '',
3220
headers: new Headers(),
33-
body: null,
3421
text: async () => body.text ?? '',
3522
json: async () => body.json ?? {},
36-
arrayBuffer: async () => body.arrayBuffer ?? new ArrayBuffer(0),
37-
}
23+
} as unknown as Response
3824
}
3925

26+
const fetchSpy = vi.fn<typeof fetch>()
27+
4028
beforeEach(() => {
41-
vi.clearAllMocks()
42-
inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValue({
43-
isValid: true,
44-
resolvedIP: PINNED_IP,
45-
originalHostname: 'example.agiloft.com',
46-
})
29+
fetchSpy.mockReset()
30+
vi.stubGlobal('fetch', fetchSpy)
4731
})
4832

49-
describe('resolveAgiloftInstance', () => {
50-
it('returns the resolved IP for a valid URL', async () => {
51-
const ip = await resolveAgiloftInstance('https://example.agiloft.com')
52-
expect(ip).toBe(PINNED_IP)
53-
expect(inputValidationMockFns.mockValidateUrlWithDNS).toHaveBeenCalledWith(
54-
'https://example.agiloft.com',
55-
'instanceUrl'
56-
)
57-
})
58-
59-
it('throws when the URL resolves to a blocked IP', async () => {
60-
inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValueOnce({
61-
isValid: false,
62-
error: 'instanceUrl resolves to a blocked IP address',
63-
})
64-
65-
await expect(resolveAgiloftInstance('https://attacker.example.com')).rejects.toThrow(
66-
'instanceUrl resolves to a blocked IP address'
67-
)
68-
})
69-
70-
it('throws when validation succeeds but no IP is returned', async () => {
71-
inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValueOnce({
72-
isValid: true,
73-
})
74-
75-
await expect(resolveAgiloftInstance('https://example.agiloft.com')).rejects.toThrow(
76-
'Invalid Agiloft instance URL'
77-
)
78-
})
33+
afterEach(() => {
34+
vi.unstubAllGlobals()
7935
})
8036

8137
describe('executeAgiloftRequest', () => {
82-
it('pins the resolved IP across login, operation, and logout', async () => {
83-
inputValidationMockFns.mockSecureFetchWithPinnedIP
84-
// EWLogin
85-
.mockResolvedValueOnce(mockSecureFetchResponse({ json: { access_token: 'tok-1' } }))
86-
// operation
87-
.mockResolvedValueOnce(mockSecureFetchResponse({ json: { id: 42, fields: { name: 'foo' } } }))
88-
// EWLogout
89-
.mockResolvedValueOnce(mockSecureFetchResponse({}))
38+
it('logs in, runs the operation with the bearer token, then logs out', async () => {
39+
fetchSpy
40+
.mockResolvedValueOnce(mockFetchResponse({ json: { access_token: 'tok-1' } }))
41+
.mockResolvedValueOnce(mockFetchResponse({ json: { id: 42, fields: { name: 'foo' } } }))
42+
.mockResolvedValueOnce(mockFetchResponse({}))
9043

9144
const result = await executeAgiloftRequest(
9245
baseParams,
@@ -106,43 +59,24 @@ describe('executeAgiloftRequest', () => {
10659

10760
expect(result).toEqual({ success: true, output: { id: '42', fields: { name: 'foo' } } })
10861

109-
const calls = inputValidationMockFns.mockSecureFetchWithPinnedIP.mock.calls
62+
const calls = fetchSpy.mock.calls
11063
expect(calls).toHaveLength(3)
111-
112-
// Every call MUST use the pre-resolved IP — this is the SSRF fix.
113-
for (const call of calls) {
114-
expect(call[1]).toBe(PINNED_IP)
115-
}
116-
117-
// Login URL preserves the original hostname (TLS SNI requirement).
11864
expect(calls[0][0]).toBe(
11965
'https://example.agiloft.com/ewws/EWLogin?$KB=demo&$login=admin&$password=secret'
12066
)
121-
expect(calls[0][2]).toEqual({ method: 'POST' })
122-
123-
// Operation request includes the bearer token issued by login.
12467
expect(calls[1][0]).toBe('https://example.agiloft.com/ewws/REST/demo/contracts/42')
125-
expect(calls[1][2]).toMatchObject({
68+
expect(calls[1][1]).toMatchObject({
12669
method: 'GET',
12770
headers: { Accept: 'application/json', Authorization: 'Bearer tok-1' },
12871
})
129-
130-
// Logout uses the bearer token and the original hostname.
13172
expect(calls[2][0]).toBe('https://example.agiloft.com/ewws/EWLogout?$KB=demo')
132-
expect(calls[2][2]).toMatchObject({
133-
method: 'POST',
134-
headers: { Authorization: 'Bearer tok-1' },
135-
})
136-
137-
// DNS lookup happens exactly once, before any HTTP request.
138-
expect(inputValidationMockFns.mockValidateUrlWithDNS).toHaveBeenCalledTimes(1)
13973
})
14074

14175
it('still calls logout when the operation throws', async () => {
142-
inputValidationMockFns.mockSecureFetchWithPinnedIP
143-
.mockResolvedValueOnce(mockSecureFetchResponse({ json: { access_token: 'tok-2' } }))
144-
.mockResolvedValueOnce(mockSecureFetchResponse({ ok: false, status: 500 }))
145-
.mockResolvedValueOnce(mockSecureFetchResponse({}))
76+
fetchSpy
77+
.mockResolvedValueOnce(mockFetchResponse({ json: { access_token: 'tok-2' } }))
78+
.mockResolvedValueOnce(mockFetchResponse({ ok: false, status: 500 }))
79+
.mockResolvedValueOnce(mockFetchResponse({}))
14680

14781
await expect(
14882
executeAgiloftRequest(
@@ -155,16 +89,14 @@ describe('executeAgiloftRequest', () => {
15589
)
15690
).rejects.toThrow('operation failed')
15791

158-
expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).toHaveBeenCalledTimes(3)
159-
expect(inputValidationMockFns.mockSecureFetchWithPinnedIP.mock.calls[2][0]).toContain(
160-
'/ewws/EWLogout'
161-
)
92+
expect(fetchSpy).toHaveBeenCalledTimes(3)
93+
expect(fetchSpy.mock.calls[2][0]).toContain('/ewws/EWLogout')
16294
})
16395

16496
it('swallows logout failures (best-effort)', async () => {
165-
inputValidationMockFns.mockSecureFetchWithPinnedIP
166-
.mockResolvedValueOnce(mockSecureFetchResponse({ json: { access_token: 'tok-3' } }))
167-
.mockResolvedValueOnce(mockSecureFetchResponse({ json: { ok: true } }))
97+
fetchSpy
98+
.mockResolvedValueOnce(mockFetchResponse({ json: { access_token: 'tok-3' } }))
99+
.mockResolvedValueOnce(mockFetchResponse({ json: { ok: true } }))
168100
.mockRejectedValueOnce(new Error('logout network error'))
169101

170102
const result = await executeAgiloftRequest(
@@ -177,10 +109,7 @@ describe('executeAgiloftRequest', () => {
177109
})
178110

179111
it('throws when login does not return an access token', async () => {
180-
inputValidationMockFns.mockSecureFetchWithPinnedIP.mockResolvedValueOnce(
181-
mockSecureFetchResponse({ json: {} })
182-
)
183-
// Login failure should still trigger no logout, since no token was issued.
112+
fetchSpy.mockResolvedValueOnce(mockFetchResponse({ json: {} }))
184113

185114
await expect(
186115
executeAgiloftRequest(
@@ -190,23 +119,18 @@ describe('executeAgiloftRequest', () => {
190119
)
191120
).rejects.toThrow('Agiloft login did not return an access token')
192121

193-
expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).toHaveBeenCalledTimes(1)
122+
expect(fetchSpy).toHaveBeenCalledTimes(1)
194123
})
195124

196-
it('refuses to call any external endpoint when validation rejects the URL', async () => {
197-
inputValidationMockFns.mockValidateUrlWithDNS.mockResolvedValueOnce({
198-
isValid: false,
199-
error: 'instanceUrl resolves to a blocked IP address',
200-
})
201-
125+
it('rejects an instance URL that fails synchronous URL validation', async () => {
202126
await expect(
203127
executeAgiloftRequest(
204-
{ ...baseParams, instanceUrl: 'https://attacker.example.com' },
128+
{ ...baseParams, instanceUrl: 'not-a-valid-url' },
205129
(base) => ({ url: `${base}/ewws/REST/demo/contracts/42`, method: 'GET' }),
206130
async () => ({ success: true, output: {} })
207131
)
208-
).rejects.toThrow('instanceUrl resolves to a blocked IP address')
132+
).rejects.toThrow(/Invalid Agiloft instance URL/)
209133

210-
expect(inputValidationMockFns.mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
134+
expect(fetchSpy).not.toHaveBeenCalled()
211135
})
212136
})

0 commit comments

Comments
 (0)