Skip to content

Commit b61d943

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) (cherry picked from commit 92b90886b79091235e710df920bcf2351653279b)
1 parent 1858656 commit b61d943

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
@@ -224,7 +224,7 @@ class CacheHandler {
224224
}
225225

226226
const cacheControlDirectives = cacheControlHeader ? parseCacheControlHeader(cacheControlHeader) : {}
227-
if (!canCacheResponse(this.#cacheType, statusCode, resHeaders, cacheControlDirectives, this.#cacheKey.headers)) {
227+
if (!canCacheResponse(this.#cacheType, this.#cacheKey.method, statusCode, resHeaders, cacheControlDirectives, this.#cacheKey.headers)) {
228228
if (statusCode === 304 && (cacheControlHeader || revalidationResponseDisallowsCachedReuse(this.#cacheType, resHeaders, cacheControlDirectives))) {
229229
deleteCachedValue(this.#store, this.#cacheKey)
230230
}
@@ -473,12 +473,16 @@ function revalidationResponseDisallowsCachedReuse (cacheType, resHeaders, cacheC
473473
* @see https://www.rfc-editor.org/rfc/rfc9111.html#name-storing-responses-to-authen
474474
*
475475
* @param {import('../../types/cache-interceptor.d.ts').default.CacheOptions['type']} cacheType
476+
* @param {string} method
476477
* @param {number} statusCode
477478
* @param {import('../../types/header.d.ts').IncomingHttpHeaders} resHeaders
478479
* @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives
479480
* @param {import('../../types/header.d.ts').IncomingHttpHeaders} [reqHeaders]
480481
*/
481-
function canCacheResponse (cacheType, statusCode, resHeaders, cacheControlDirectives, reqHeaders) {
482+
function canCacheResponse (cacheType, method, statusCode, resHeaders, cacheControlDirectives, reqHeaders) {
483+
if (!arrayIncludes(util.safeHTTPMethods, method)) {
484+
return false
485+
}
482486
// Status code must be final and understood.
483487
if (statusCode < 200 || arrayIncludes(NOT_UNDERSTOOD_STATUS_CODES, statusCode)) {
484488
return false

‎lib/interceptor/cache.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,11 @@ module.exports = (opts = {}) => {
574574
* @type {import('../../types/cache-interceptor.d.ts').default.CacheKey}
575575
*/
576576
const cacheKey = makeCacheKey(opts)
577+
578+
if (!arrayIncludes(util.safeHTTPMethods, opts.method)) {
579+
return dispatch(opts, new CacheHandler(globalOpts, cacheKey, handler))
580+
}
581+
577582
const result = store.get(cacheKey)
578583

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

‎test/interceptors/cache.js‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,82 @@ describe('Cache Interceptor', () => {
931931
}
932932
})
933933

934+
test('unsafe methods are not served from cache', async () => {
935+
// A heuristically-cacheable response (404) with an explicit max-age would
936+
// previously be stored under the unsafe method and replayed on a second,
937+
// identical state-changing request without ever hitting the origin.
938+
let requestsToOrigin = 0
939+
const server = createServer((req, res) => {
940+
requestsToOrigin++
941+
res.statusCode = 404
942+
res.setHeader('cache-control', 'max-age=60')
943+
res.end(`origin-hit-#${requestsToOrigin}`)
944+
}).listen(0)
945+
946+
after(() => server.close())
947+
948+
const client = new Client(`http://localhost:${server.address().port}`)
949+
.compose(interceptors.cache({
950+
store: new MemoryCacheStore()
951+
}))
952+
953+
for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) {
954+
const req = { origin: 'localhost', method, path: '/resource/123' }
955+
const before = requestsToOrigin
956+
957+
const res1 = await client.request(req)
958+
const body1 = await res1.body.text()
959+
960+
const res2 = await client.request(req)
961+
const body2 = await res2.body.text()
962+
963+
// Every unsafe-method request must reach the origin; none may be served from cache.
964+
equal(requestsToOrigin - before, 2)
965+
equal(body1, `origin-hit-#${before + 1}`)
966+
equal(body2, `origin-hit-#${before + 2}`)
967+
}
968+
969+
await client.close()
970+
})
971+
972+
test('successful unsafe methods still invalidate cached entries', async () => {
973+
// The fix routes unsafe methods through CacheHandler (not a bare dispatch) so a
974+
// successful unsafe request keeps invalidating an existing cached GET for the same path.
975+
let requestsToOrigin = 0
976+
const server = createServer((req, res) => {
977+
requestsToOrigin++
978+
if (req.method === 'GET') {
979+
res.statusCode = 200
980+
res.setHeader('cache-control', 'max-age=60')
981+
} else {
982+
res.statusCode = 200
983+
}
984+
res.end(`origin-hit-#${requestsToOrigin}`)
985+
}).listen(0)
986+
987+
after(() => server.close())
988+
989+
const client = new Client(`http://localhost:${server.address().port}`)
990+
.compose(interceptors.cache({
991+
store: new MemoryCacheStore()
992+
}))
993+
994+
const get = { origin: 'localhost', method: 'GET', path: '/resource/123' }
995+
996+
await (await client.request(get)).body.text()
997+
const afterPrime = requestsToOrigin
998+
await (await client.request(get)).body.text()
999+
equal(requestsToOrigin, afterPrime) // second GET served from cache
1000+
1001+
await (await client.request({ origin: 'localhost', method: 'POST', path: '/resource/123' })).body.text()
1002+
1003+
const beforeFinalGet = requestsToOrigin
1004+
await (await client.request(get)).body.text()
1005+
equal(requestsToOrigin - beforeFinalGet, 1) // GET re-fetched after invalidation
1006+
1007+
await client.close()
1008+
})
1009+
9341010
test('necessary headers are stripped', async () => {
9351011
const headers = [
9361012
// Headers defined in the spec that we need to strip

0 commit comments

Comments
 (0)