Skip to content

Commit 3082de6

Browse files
committed
fix(network): replay Request bodies across redirects
1 parent 18dd0b0 commit 3082de6

2 files changed

Lines changed: 108 additions & 15 deletions

File tree

apps/sim/lib/core/security/guarded-request-fetch.server.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,93 @@ describe('createSsrfGuardedFetchWithDispatcher (undici.request backed)', () => {
134134
expect(await response.text()).toBe('final-body')
135135
})
136136

137+
it.each([
138+
[307, 'POST'],
139+
[308, 'POST'],
140+
[301, 'PUT'],
141+
[302, 'PUT'],
142+
] as const)('replays a Request body through same-origin %s redirects', async (status, method) => {
143+
const payloads: string[] = []
144+
mockUndiciRequest.mockImplementation(async (_url, options: { body: Buffer | Readable }) => {
145+
const chunks: Buffer[] = []
146+
for await (const chunk of options.body instanceof Readable ? options.body : [options.body]) {
147+
chunks.push(Buffer.from(chunk))
148+
}
149+
payloads.push(Buffer.concat(chunks).toString())
150+
return payloads.length < 3
151+
? undiciReply(status, { location: `/hop-${payloads.length}` }, byteStream(''))
152+
: undiciReply(200, {}, byteStream('done'))
153+
})
154+
const transport = createSsrfGuardedFetchWithDispatcher({ profile: 'configuredEndpoint' })
155+
try {
156+
const response = await transport.fetch(
157+
new Request('https://api.example.com/start', {
158+
method,
159+
headers: { 'content-type': 'application/json' },
160+
body: '{"payload":"replay"}',
161+
})
162+
)
163+
expect(await response.text()).toBe('done')
164+
expect(payloads).toEqual(Array(3).fill('{"payload":"replay"}'))
165+
expect(response.redirected).toBe(true)
166+
expect(mockUndiciRequest.mock.calls.map(([, options]) => options.method)).toEqual(
167+
Array(3).fill(method)
168+
)
169+
} finally {
170+
await transport.dispatcher.destroy()
171+
}
172+
})
173+
174+
it.each(['manual', 'error'] as const)(
175+
'keeps Request bodies streaming in %s mode',
176+
async (redirect) => {
177+
const request = new Request('https://api.example.com/upload', {
178+
method: 'POST',
179+
redirect,
180+
body: 'payload',
181+
})
182+
const transport = createSsrfGuardedFetchWithDispatcher({ profile: 'configuredEndpoint' })
183+
mockUndiciRequest.mockImplementationOnce(async (_url, options: { body: Readable }) => {
184+
expect(options.body).toBeInstanceOf(Readable)
185+
const chunks: Buffer[] = []
186+
for await (const chunk of options.body) chunks.push(Buffer.from(chunk))
187+
expect(Buffer.concat(chunks).toString()).toBe('payload')
188+
return undiciReply(200, {}, byteStream('done'))
189+
})
190+
try {
191+
expect(await (await transport.fetch(request)).text()).toBe('done')
192+
} finally {
193+
await transport.dispatcher.destroy()
194+
}
195+
}
196+
)
197+
198+
it('does not read the Request body when init supplies a replacement', async () => {
199+
const request = new Request('https://api.example.com/upload', {
200+
method: 'POST',
201+
body: 'original',
202+
})
203+
const clone = vi.spyOn(request, 'clone')
204+
const bodyOverride = vi.fn().mockReturnValueOnce('replacement').mockReturnValue(undefined)
205+
mockUndiciRequest.mockResolvedValueOnce(undiciReply(200, {}, byteStream('done')))
206+
const transport = createSsrfGuardedFetchWithDispatcher({ profile: 'configuredEndpoint' })
207+
try {
208+
const response = await transport.fetch(request, {
209+
get body() {
210+
return bodyOverride()
211+
},
212+
})
213+
expect(await response.text()).toBe('done')
214+
expect(mockUndiciRequest.mock.calls[0][1].body).toBe('replacement')
215+
expect(request.bodyUsed).toBe(false)
216+
expect(clone).not.toHaveBeenCalled()
217+
expect(bodyOverride).toHaveBeenCalledTimes(1)
218+
} finally {
219+
await request.body?.cancel()
220+
await transport.dispatcher.destroy()
221+
}
222+
})
223+
137224
it('supports buffered reads (.json()) through the constructed body', async () => {
138225
mockUndiciRequest.mockResolvedValueOnce(
139226
undiciReply(

apps/sim/lib/core/security/input-validation.server.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -893,26 +893,32 @@ async function undiciRequestAsResponse(
893893
* fetch semantics) so a manual redirect follower can't silently downgrade a POST Request to a
894894
* bare GET or lose its headers.
895895
*/
896-
function liftFetchArgs(
896+
async function liftFetchArgs(
897897
input: RequestInfo | URL,
898898
init?: RequestInit
899-
): { target: string; effectiveInit: RequestInit } {
899+
): Promise<{ target: string; effectiveInit: RequestInit }> {
900900
const target = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url
901901
if (typeof Request !== 'undefined' && input instanceof Request) {
902902
const bodyAllowed = input.method !== 'GET' && input.method !== 'HEAD'
903-
return {
904-
target,
905-
effectiveInit: {
906-
method: input.method,
907-
headers: input.headers,
908-
body: bodyAllowed ? input.body : undefined,
909-
signal: input.signal,
910-
// Carry the Request's redirect mode so the pinned fetch honors `manual`/`error`
911-
// instead of defaulting a `Request({ redirect: 'manual' })` to `follow`.
912-
redirect: input.redirect,
913-
...init,
914-
},
903+
const effectiveInit: RequestInit = {
904+
method: input.method,
905+
headers: input.headers,
906+
body: bodyAllowed ? input.body : undefined,
907+
signal: input.signal,
908+
// Carry the Request's redirect mode so the pinned fetch honors `manual`/`error`
909+
// instead of defaulting a `Request({ redirect: 'manual' })` to `follow`.
910+
redirect: input.redirect,
911+
...init,
912+
}
913+
/** Request hides its original body source, so following redirects requires replayable bytes. */
914+
if (
915+
!Object.hasOwn(init ?? {}, 'body') &&
916+
effectiveInit.body &&
917+
(effectiveInit.redirect ?? 'follow') === 'follow'
918+
) {
919+
effectiveInit.body = await input.clone().arrayBuffer()
915920
}
921+
return { target, effectiveInit }
916922
}
917923
return { target, effectiveInit: init ?? {} }
918924
}
@@ -941,7 +947,7 @@ function createValidatedFetch(
941947
return {
942948
dispatcher,
943949
fetch: async (input: RequestInfo | URL, init?: RequestInit): Promise<Response> => {
944-
const { target, effectiveInit } = liftFetchArgs(input, init)
950+
const { target, effectiveInit } = await liftFetchArgs(input, init)
945951
const mode = effectiveInit.redirect ?? 'follow'
946952
// double-cast-allowed: DOM and Undici RequestInit represent the same wire request in this bridge
947953
const undiciInit = effectiveInit as unknown as UndiciRequestInit

0 commit comments

Comments
 (0)