Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/strip-upstream-set-cookie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'mppx': patch
---

Stripped `Set-Cookie` from upstream responses in `Proxy.scrubResponse` so an upstream service cannot set cookies under the proxy's origin.
14 changes: 7 additions & 7 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pnpm-workspace.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ overrides:
path-to-regexp@<8.4.0: '8.4.0'
tar@<=7.5.10: '7.5.11'
'@modelcontextprotocol/sdk@>=1.10.0 <=1.25.3': '1.26.0'
qs@>=6.7.0 <=6.14.1: '6.14.2'
qs@>=6.7.0 <=6.15.1: '6.15.2'
ip-address@<=10.1.0: '10.1.1'
minimatch@>=5.0.0 <5.1.8: '5.1.8'
minimatch@>=9.0.0 <9.0.7: '9.0.7'
Expand Down
18 changes: 18 additions & 0 deletions src/proxy/internal/Headers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,22 @@ describe('scrubResponse', () => {
expect(result.statusText).toBe('Created')
expect(await result.text()).toBe('hello')
})

// Regression: an upstream service must never be able to issue a cookie
// under the proxy's origin. Otherwise a compromised or attacker-influenced
// upstream can session-fixate (`Set-Cookie: session=evil; Domain=…`) every
// sibling subdomain of the proxy. See the docblock on `scrubResponse`.
test('behavior: strips set-cookie so upstream cannot set cookies on proxy origin', () => {
const response = new Response('body', {
headers: [
['Set-Cookie', '__Secure-session=evil; Domain=.example.com; Secure; HttpOnly'],
['Set-Cookie', 'tracking=1; Path=/'],
['Content-Type', 'application/json'],
],
})
const result = Headers.scrubResponse(response)
expect(result.headers.has('set-cookie')).toBe(false)
expect(result.headers.getSetCookie?.() ?? []).toEqual([])
expect(result.headers.get('content-type')).toBe('application/json')
})
})
15 changes: 14 additions & 1 deletion src/proxy/internal/Headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,24 @@ export function scrub(headers: Headers): Headers {
return scrubbed
}

/** Strips `content-encoding` and `content-length` from an upstream response so the proxy can re-stream it. */
/**
* Strips re-streaming headers (`content-encoding`, `content-length`) and
* security-sensitive headers (`set-cookie`) from an upstream response.
*
* `set-cookie` is dropped because a paid API proxy must never let an upstream
* service set cookies in the user's browser under the proxy's origin. If a
* compromised, misbehaving, or attacker-influenced upstream returned
* `Set-Cookie: session=evil; Domain=.example.com`, the browser would honor it
* for every sibling subdomain of the proxy — turning any future path-confusion
* or open-redirect bug in the surrounding deployment into a session-fixation
* primitive. Proxied services authenticate via bearer tokens / signed
* payloads, never cookies, so dropping `set-cookie` is purely defensive.
*/
export function scrubResponse(response: Response): Response {
const headers = new Headers(response.headers)
headers.delete('content-encoding')
headers.delete('content-length')
headers.delete('set-cookie')
return new Response(response.body, {
status: response.status,
statusText: response.statusText,
Expand Down
Loading