Skip to content

Commit cb75bbb

Browse files
mcollinamcollina
committed
fix(cache): do not cache Set-Cookie in shared caches
Prevent the shared cache interceptor from storing or re-serving responses containing Set-Cookie, including existing cache entries and synchronous or background revalidation paths, so cookies from one request are not disclosed to later callers while preserving private-cache behavior. Fixes: GHSA-2jfj-6hjv-fm6j CVE: CVE-2026-84933 Signed-off-by: Matteo Collina <hello@matteocollina.com>
1 parent 6d58312 commit cb75bbb

3 files changed

Lines changed: 223 additions & 19 deletions

File tree

‎lib/handler/cache-handler.js‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,13 @@ class CacheHandler {
218218
}
219219

220220
const cacheControlHeader = resHeaders['cache-control']
221+
const cacheControlDirectives = cacheControlHeader ? parseCacheControlHeader(cacheControlHeader) : {}
222+
223+
if (revalidationResponseDisallowsCachedReuse(this.#cacheType, resHeaders, cacheControlDirectives)) {
224+
deleteCachedValue(this.#store, this.#cacheKey)
225+
return downstreamOnHeaders()
226+
}
227+
221228
const heuristicallyCacheable = resHeaders['last-modified'] && arrayIncludes(HEURISTICALLY_CACHEABLE_STATUS_CODES, statusCode)
222229
if (
223230
!cacheControlHeader &&
@@ -234,7 +241,6 @@ class CacheHandler {
234241
return downstreamOnHeaders()
235242
}
236243

237-
const cacheControlDirectives = cacheControlHeader ? parseCacheControlHeader(cacheControlHeader) : {}
238244
if (!canCacheResponse(this.#cacheType, this.#cacheKey.method, statusCode, resHeaders, cacheControlDirectives, this.#cacheKey.headers)) {
239245
if (statusCode === 304 && (cacheControlHeader || revalidationResponseDisallowsCachedReuse(this.#cacheType, resHeaders, cacheControlDirectives))) {
240246
deleteCachedValue(this.#store, this.#cacheKey)
@@ -484,7 +490,10 @@ function deleteCachedValueIfNotModified (statusCode, store, cacheKey) {
484490
*/
485491
function revalidationResponseDisallowsCachedReuse (cacheType, resHeaders, cacheControlDirectives) {
486492
return cacheControlDirectives['no-store'] === true ||
487-
(cacheType === 'shared' && cacheControlDirectives.private === true) ||
493+
(cacheType === 'shared' && (
494+
cacheControlDirectives.private === true ||
495+
Object.hasOwn(resHeaders, 'set-cookie')
496+
)) ||
488497
(resHeaders.vary ? isInvalidOrWildcardVaryHeader(resHeaders.vary) : false)
489498
}
490499

@@ -522,7 +531,10 @@ function canCacheResponse (cacheType, method, statusCode, resHeaders, cacheContr
522531
return false
523532
}
524533

525-
if (cacheType === 'shared' && cacheControlDirectives.private === true) {
534+
if (cacheType === 'shared' && (
535+
cacheControlDirectives.private === true ||
536+
Object.hasOwn(resHeaders, 'set-cookie')
537+
)) {
526538
return false
527539
}
528540

‎lib/interceptor/cache.js‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,10 @@ function staleResponseRequiresRevalidation (result, cacheType) {
126126
* @returns {boolean}
127127
*/
128128
function revalidationResponseDisallowsCachedReuse (cacheType, headers) {
129-
if (headers.vary && isInvalidOrWildcardVaryHeader(headers.vary)) {
129+
if (
130+
(headers.vary && isInvalidOrWildcardVaryHeader(headers.vary)) ||
131+
(cacheType === 'shared' && Object.hasOwn(headers, 'set-cookie'))
132+
) {
130133
return true
131134
}
132135

@@ -417,6 +420,17 @@ function handleResult (
417420
return handleUncachedResponse(dispatch, globalOpts, cacheKey, handler, opts, reqCacheControl)
418421
}
419422

423+
// Shared stores may outlive the Undici version that wrote them. Do not
424+
// re-serve a Set-Cookie header from an existing shared-cache entry.
425+
if (globalOpts.type === 'shared' && Object.hasOwn(result.headers, 'set-cookie')) {
426+
if (util.isStream(result.body)) {
427+
result.body.on('error', nop).destroy()
428+
}
429+
430+
deleteCachedValue(globalOpts.store, cacheKey)
431+
return handleUncachedResponse(dispatch, globalOpts, cacheKey, handler, opts, reqCacheControl)
432+
}
433+
420434
const now = Date.now()
421435
if (now > result.deleteAt) {
422436
// Response is expired, cache store shouldn't have given this to us

‎test/interceptors/cache.js‎

Lines changed: 193 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,113 @@ describe('Cache Interceptor', () => {
5454
}
5555
})
5656

57+
test('shared cache does not store responses with Set-Cookie', async () => {
58+
let requestsToOrigin = 0
59+
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
60+
requestsToOrigin++
61+
const cookie = req.headers.cookie
62+
res.setHeader('cache-control', 'public, max-age=300')
63+
res.setHeader('set-cookie', `session-for-${cookie}`)
64+
res.end(`response for ${cookie}`)
65+
}).listen(0)
66+
67+
await once(server, 'listening')
68+
69+
const client = new Client(`http://localhost:${server.address().port}`)
70+
.compose(interceptors.cache())
71+
72+
try {
73+
{
74+
const res = await client.request({
75+
origin: 'localhost',
76+
method: 'GET',
77+
path: '/',
78+
headers: { cookie: 'user-a' }
79+
})
80+
equal(requestsToOrigin, 1)
81+
equal(res.headers['set-cookie'], 'session-for-user-a')
82+
strictEqual(await res.body.text(), 'response for user-a')
83+
}
84+
85+
{
86+
const res = await client.request({
87+
origin: 'localhost',
88+
method: 'GET',
89+
path: '/',
90+
headers: { cookie: 'user-b' }
91+
})
92+
equal(requestsToOrigin, 2)
93+
equal(res.headers['set-cookie'], 'session-for-user-b')
94+
strictEqual(await res.body.text(), 'response for user-b')
95+
}
96+
} finally {
97+
await client.close()
98+
await new Promise(resolve => server.close(resolve))
99+
}
100+
})
101+
102+
test('shared cache does not serve existing entries with Set-Cookie', async () => {
103+
let requestsToOrigin = 0
104+
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
105+
requestsToOrigin++
106+
const cookie = req.headers.cookie
107+
res.setHeader('cache-control', 'public, max-age=300')
108+
res.setHeader('set-cookie', `session-for-${cookie}`)
109+
res.end(`response for ${cookie}`)
110+
}).listen(0)
111+
112+
await once(server, 'listening')
113+
114+
const store = new MemoryCacheStore()
115+
const origin = `http://localhost:${server.address().port}`
116+
const privateClient = new Client(origin)
117+
.compose(interceptors.cache({ store, type: 'private' }))
118+
const sharedClient = new Client(origin)
119+
.compose(interceptors.cache({ store, type: 'shared' }))
120+
121+
try {
122+
{
123+
const res = await privateClient.request({
124+
origin: 'localhost',
125+
method: 'GET',
126+
path: '/',
127+
headers: { cookie: 'user-a' }
128+
})
129+
equal(requestsToOrigin, 1)
130+
equal(res.headers['set-cookie'], 'session-for-user-a')
131+
strictEqual(await res.body.text(), 'response for user-a')
132+
}
133+
134+
{
135+
const res = await privateClient.request({
136+
origin: 'localhost',
137+
method: 'GET',
138+
path: '/',
139+
headers: { cookie: 'user-a' }
140+
})
141+
equal(requestsToOrigin, 1)
142+
equal(res.headers['set-cookie'], 'session-for-user-a')
143+
strictEqual(await res.body.text(), 'response for user-a')
144+
}
145+
146+
{
147+
const res = await sharedClient.request({
148+
origin: 'localhost',
149+
method: 'GET',
150+
path: '/',
151+
headers: { cookie: 'user-b' }
152+
})
153+
equal(requestsToOrigin, 2)
154+
equal(res.headers['set-cookie'], 'session-for-user-b')
155+
strictEqual(await res.body.text(), 'response for user-b')
156+
}
157+
} finally {
158+
await privateClient.close()
159+
await sharedClient.close()
160+
await new Promise(resolve => server.close(resolve))
161+
}
162+
})
163+
57164
test('vary directives used to decide which response to use', async () => {
58165
let requestsToOrigin = 0
59166
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
@@ -1237,20 +1344,20 @@ describe('Cache Interceptor', () => {
12371344

12381345
test('qualified no-cache/private with OWS around = strip named headers from cached responses', async () => {
12391346
for (const cacheControl of [
1240-
'public, max-age=60, no-cache= "set-cookie"',
1241-
'public, max-age=60, no-cache ="set-cookie"',
1242-
'public, max-age=60, no-cache \t= \t"set-cookie"',
1243-
'public, max-age=60, no-cache="set-cookie"\t, immutable',
1244-
'public, max-age=60, private= "set-cookie"',
1245-
'public, max-age=60, private ="set-cookie"',
1246-
'public, max-age=60, private\t=\t"set-cookie"',
1247-
'public, max-age=60, private="set-cookie"\t, immutable'
1347+
'public, max-age=60, no-cache= "x-secret"',
1348+
'public, max-age=60, no-cache ="x-secret"',
1349+
'public, max-age=60, no-cache \t= \t"x-secret"',
1350+
'public, max-age=60, no-cache="x-secret"\t, immutable',
1351+
'public, max-age=60, private= "x-secret"',
1352+
'public, max-age=60, private ="x-secret"',
1353+
'public, max-age=60, private\t=\t"x-secret"',
1354+
'public, max-age=60, private="x-secret"\t, immutable'
12481355
]) {
12491356
let requestToOrigin = 0
12501357
const server = createServer({ joinDuplicateHeaders: true }, (_, res) => {
12511358
requestToOrigin++
12521359
res.setHeader('cache-control', cacheControl)
1253-
res.setHeader('set-cookie', 'session=secret')
1360+
res.setHeader('x-secret', 'secret')
12541361
res.end('ok')
12551362
}).listen(0)
12561363

@@ -1269,14 +1376,14 @@ describe('Cache Interceptor', () => {
12691376
{
12701377
const res = await client.request(request)
12711378
equal(requestToOrigin, 1, cacheControl)
1272-
equal(res.headers['set-cookie'], 'session=secret', cacheControl)
1379+
equal(res.headers['x-secret'], 'secret', cacheControl)
12731380
strictEqual(await res.body.text(), 'ok')
12741381
}
12751382

12761383
{
12771384
const res = await client.request(request)
12781385
equal(requestToOrigin, 1, cacheControl)
1279-
equal(res.headers['set-cookie'], undefined, cacheControl)
1386+
equal(res.headers['x-secret'], undefined, cacheControl)
12801387
strictEqual(await res.body.text(), 'ok')
12811388
}
12821389
} finally {
@@ -1291,8 +1398,8 @@ describe('Cache Interceptor', () => {
12911398
const server = createServer({ joinDuplicateHeaders: true }, (_, res) => {
12921399
requestToOrigin++
12931400
res.setHeader('cache-control', 's-maxage=10')
1294-
res.setHeader('connection', ['Set-Cookie, X-Secret', 'Keep-Alive, X-Empty'])
1295-
res.setHeader('set-cookie', 'session=secret')
1401+
res.setHeader('connection', ['X-Other-Secret, X-Secret', 'Keep-Alive, X-Empty'])
1402+
res.setHeader('x-other-secret', 'other secret')
12961403
res.setHeader('x-secret', 'secret')
12971404
res.setHeader('x-empty', '')
12981405
res.end('ok')
@@ -1317,7 +1424,7 @@ describe('Cache Interceptor', () => {
13171424
{
13181425
const res = await client.request(request)
13191426
equal(requestToOrigin, 1)
1320-
equal(res.headers['set-cookie'], 'session=secret')
1427+
equal(res.headers['x-other-secret'], 'other secret')
13211428
equal(res.headers['x-secret'], 'secret')
13221429
equal(res.headers['x-empty'], '')
13231430
strictEqual(await res.body.text(), 'ok')
@@ -1326,7 +1433,7 @@ describe('Cache Interceptor', () => {
13261433
{
13271434
const res = await client.request(request)
13281435
equal(requestToOrigin, 1)
1329-
equal(res.headers['set-cookie'], undefined)
1436+
equal(res.headers['x-other-secret'], undefined)
13301437
equal(res.headers['x-secret'], undefined)
13311438
equal(res.headers['x-empty'], undefined)
13321439
strictEqual(await res.body.text(), 'ok')
@@ -1607,6 +1714,7 @@ describe('Cache Interceptor', () => {
16071714
for (const testCase of [
16081715
{ name: 'no-store', headers: { 'cache-control': 'no-store' } },
16091716
{ name: 'private', headers: { 'cache-control': 'private' } },
1717+
{ name: 'set-cookie', headers: { 'set-cookie': 'session=secret' } },
16101718
{ name: 'vary-star', headers: { vary: '*' } },
16111719
{ name: 'malformed-vary', headers: { vary: 'cookie authorization' } }
16121720
]) {
@@ -1840,6 +1948,7 @@ describe('Cache Interceptor', () => {
18401948
for (const testCase of [
18411949
{ name: 'no-store', headers: { 'cache-control': 'no-store' } },
18421950
{ name: 'private', headers: { 'cache-control': 'private' } },
1951+
{ name: 'set-cookie', headers: { 'set-cookie': 'session=secret' } },
18431952
{ name: 'vary-star', headers: { vary: '*' } },
18441953
{ name: 'malformed-vary', headers: { vary: 'cookie authorization' } },
18451954
{ name: 'explicitly-stale', headers: { 'cache-control': 'public, max-age=0' } },
@@ -1922,6 +2031,75 @@ describe('Cache Interceptor', () => {
19222031
}
19232032
})
19242033

2034+
test('stale-while-revalidate evicts a shared entry when its replacement has Set-Cookie', async () => {
2035+
const clock = FakeTimers.install({
2036+
toFake: ['Date']
2037+
})
2038+
2039+
let requestsToOrigin = 0
2040+
let conditionalRequests = 0
2041+
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
2042+
requestsToOrigin++
2043+
res.setHeader('date', new Date().toUTCString())
2044+
2045+
if (req.headers['if-none-match'] || req.headers['if-modified-since']) {
2046+
conditionalRequests++
2047+
res.setHeader('cache-control', 'public, max-age=60')
2048+
res.setHeader('set-cookie', 'session=secret')
2049+
res.end('personalized')
2050+
return
2051+
}
2052+
2053+
if (requestsToOrigin === 1) {
2054+
res.setHeader('cache-control', 'public, max-age=1, stale-while-revalidate=10')
2055+
res.setHeader('etag', '"cached"')
2056+
res.end('cached')
2057+
return
2058+
}
2059+
2060+
res.setHeader('cache-control', 'public, max-age=60')
2061+
res.end('refetched')
2062+
}).listen(0)
2063+
2064+
const client = new Client(`http://localhost:${server.address().port}`)
2065+
.compose(interceptors.cache())
2066+
2067+
try {
2068+
await once(server, 'listening')
2069+
2070+
const request = {
2071+
origin: 'localhost',
2072+
method: 'GET',
2073+
path: '/'
2074+
}
2075+
2076+
{
2077+
const res = await client.request(request)
2078+
strictEqual(await res.body.text(), 'cached')
2079+
}
2080+
2081+
clock.tick(1500)
2082+
2083+
{
2084+
const res = await client.request(request)
2085+
strictEqual(await res.body.text(), 'cached')
2086+
}
2087+
2088+
await sleep(100)
2089+
equal(conditionalRequests, 1)
2090+
2091+
{
2092+
const res = await client.request(request)
2093+
equal(requestsToOrigin, 3)
2094+
strictEqual(await res.body.text(), 'refetched')
2095+
}
2096+
} finally {
2097+
await client.close()
2098+
await new Promise(resolve => server.close(resolve))
2099+
clock.uninstall()
2100+
}
2101+
})
2102+
19252103
test('stale-if-error (response)', async () => {
19262104
const clock = FakeTimers.install({
19272105
toFake: ['Date']

0 commit comments

Comments
 (0)