Skip to content

Commit 2be07bf

Browse files
committed
fix(cache): reject unsafe method response caching
Restrict cache reads and writes to safe HTTP methods while preserving successful unsafe-method invalidation. This prevents responses to methods such as POST or DELETE from being stored and replayed. Fixes: GHSA-8436-99hf-9mmv CVE: CVE-2026-85008 Signed-off-by: Matteo Collina <hello@matteocollina.com> (cherry picked from commit c655ee00ef5cb3272a46000be8b37c9ec8b295c5)
1 parent caf6194 commit 2be07bf

3 files changed

Lines changed: 87 additions & 2 deletions

File tree

‎lib/handler/cache-handler.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ class CacheHandler {
235235
}
236236

237237
const cacheControlDirectives = cacheControlHeader ? parseCacheControlHeader(cacheControlHeader) : {}
238-
if (!canCacheResponse(this.#cacheType, statusCode, resHeaders, cacheControlDirectives, this.#cacheKey.headers)) {
238+
if (!canCacheResponse(this.#cacheType, this.#cacheKey.method, statusCode, resHeaders, cacheControlDirectives, this.#cacheKey.headers)) {
239239
if (statusCode === 304 && (cacheControlHeader || revalidationResponseDisallowsCachedReuse(this.#cacheType, resHeaders, cacheControlDirectives))) {
240240
deleteCachedValue(this.#store, this.#cacheKey)
241241
}
@@ -492,12 +492,16 @@ function revalidationResponseDisallowsCachedReuse (cacheType, resHeaders, cacheC
492492
* @see https://www.rfc-editor.org/rfc/rfc9111.html#name-storing-responses-to-authen
493493
*
494494
* @param {import('../../types/cache-interceptor.d.ts').default.CacheOptions['type']} cacheType
495+
* @param {string} method
495496
* @param {number} statusCode
496497
* @param {import('../../types/header.d.ts').IncomingHttpHeaders} resHeaders
497498
* @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives
498499
* @param {import('../../types/header.d.ts').IncomingHttpHeaders} [reqHeaders]
499500
*/
500-
function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirectives, reqHeaders) {
501+
function canCacheResponse (cacheType, method, statusCode, resHeaders, cacheControlDirectives, reqHeaders) {
502+
if (!arrayIncludes(util.safeHTTPMethods, method)) {
503+
return false
504+
}
501505
// Status code must be final and understood.
502506
if (statusCode < 200 || arrayIncludes(NOT_UNDERSTOOD_STATUS_CODES, statusCode)) {
503507
return false

‎lib/interceptor/cache.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,11 @@ module.exports = (opts = {}) => {
624624
* @type {import('../../types/cache-interceptor.d.ts').default.CacheKey}
625625
*/
626626
const cacheKey = makeCacheKey(opts, requestOrigin)
627+
628+
if (!arrayIncludes(util.safeHTTPMethods, opts.method)) {
629+
return dispatch(opts, new CacheHandler(globalOpts, cacheKey, handler))
630+
}
631+
627632
const result = store.get(cacheKey)
628633

629634
if (result && typeof result.then === 'function') {

‎test/interceptors/cache.js‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,82 @@ describe('Cache Interceptor', () => {
10911091
}
10921092
})
10931093

1094+
test('unsafe methods are not served from cache', async () => {
1095+
// A heuristically-cacheable response (404) with an explicit max-age would
1096+
// previously be stored under the unsafe method and replayed on a second,
1097+
// identical state-changing request without ever hitting the origin.
1098+
let requestsToOrigin = 0
1099+
const server = createServer((req, res) => {
1100+
requestsToOrigin++
1101+
res.statusCode = 404
1102+
res.setHeader('cache-control', 'max-age=60')
1103+
res.end(`origin-hit-#${requestsToOrigin}`)
1104+
}).listen(0)
1105+
1106+
after(() => server.close())
1107+
1108+
const client = new Client(`http://localhost:${server.address().port}`)
1109+
.compose(interceptors.cache({
1110+
store: new MemoryCacheStore()
1111+
}))
1112+
1113+
for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) {
1114+
const req = { origin: 'localhost', method, path: '/resource/123' }
1115+
const before = requestsToOrigin
1116+
1117+
const res1 = await client.request(req)
1118+
const body1 = await res1.body.text()
1119+
1120+
const res2 = await client.request(req)
1121+
const body2 = await res2.body.text()
1122+
1123+
// Every unsafe-method request must reach the origin; none may be served from cache.
1124+
equal(requestsToOrigin - before, 2)
1125+
equal(body1, `origin-hit-#${before + 1}`)
1126+
equal(body2, `origin-hit-#${before + 2}`)
1127+
}
1128+
1129+
await client.close()
1130+
})
1131+
1132+
test('successful unsafe methods still invalidate cached entries', async () => {
1133+
// The fix routes unsafe methods through CacheHandler (not a bare dispatch) so a
1134+
// successful unsafe request keeps invalidating an existing cached GET for the same path.
1135+
let requestsToOrigin = 0
1136+
const server = createServer((req, res) => {
1137+
requestsToOrigin++
1138+
if (req.method === 'GET') {
1139+
res.statusCode = 200
1140+
res.setHeader('cache-control', 'max-age=60')
1141+
} else {
1142+
res.statusCode = 200
1143+
}
1144+
res.end(`origin-hit-#${requestsToOrigin}`)
1145+
}).listen(0)
1146+
1147+
after(() => server.close())
1148+
1149+
const client = new Client(`http://localhost:${server.address().port}`)
1150+
.compose(interceptors.cache({
1151+
store: new MemoryCacheStore()
1152+
}))
1153+
1154+
const get = { origin: 'localhost', method: 'GET', path: '/resource/123' }
1155+
1156+
await (await client.request(get)).body.text()
1157+
const afterPrime = requestsToOrigin
1158+
await (await client.request(get)).body.text()
1159+
equal(requestsToOrigin, afterPrime) // second GET served from cache
1160+
1161+
await (await client.request({ origin: 'localhost', method: 'POST', path: '/resource/123' })).body.text()
1162+
1163+
const beforeFinalGet = requestsToOrigin
1164+
await (await client.request(get)).body.text()
1165+
equal(requestsToOrigin - beforeFinalGet, 1) // GET re-fetched after invalidation
1166+
1167+
await client.close()
1168+
})
1169+
10941170
test('necessary headers are stripped', async () => {
10951171
const headers = [
10961172
// Headers defined in the spec that we need to strip

0 commit comments

Comments
 (0)