Should SqliteCacheStore (and perhaps other cache stores) be hashing headers before storing?

[alxndrsn]

Currently SqliteCacheStore writes plaintext headers to disk.

Would it be better to hash cache keys? This might have other advantages, such as faster lookup and lower storage size, but would also introduce risk of clashes.

Repro

Adapted from test/cache-interceptor/sqlite-cache-store-tests.js.

'use strict'

const { test, skip } = require('node:test')
const { doesNotMatch, notEqual, strictEqual, deepStrictEqual } = require('node:assert')
const { readFile, rm } = require('node:fs/promises')
const { cacheStoreTests, writeBody, compareGetResults } = require('./cache-store-test-utils.js')

let hasSqlite = false
try {
  require('node:sqlite')

  const SqliteCacheStore = require('../../lib/cache/sqlite-cache-store.js')
  cacheStoreTests(SqliteCacheStore)
  hasSqlite = true
} catch (err) {
  if (err.code === 'ERR_UNKNOWN_BUILTIN_MODULE') {
    skip('`node:sqlite` not present')
  } else {
    throw err
  }
}

test('SqliteCacheStore hashes key contents', async (t) => {
  if (!hasSqlite) {
    t.skip()
    return
  }

  const SqliteCacheStore = require('../../lib/cache/sqlite-cache-store.js')
  const sqliteLocation = 'cache-interceptor.sqlite'

  const store = new SqliteCacheStore({
    location: sqliteLocation
  })

  t.after(async () => {
    //await rm(sqliteLocation)
  })

  /**
   * @type {import('../../types/cache-interceptor.d.ts').default.CacheKey}
   */
  const key = {
    origin: 'localhost',
    path: '/',
    method: 'GET',
    headers: {}
  }

  /**
   * @type {import('../../types/cache-interceptor.d.ts').default.CacheValue}
   */
  const value = {
    statusCode: 200,
    statusMessage: '',
    headers: { foo: 'bar', Authorization: 'topsecret' },
    cachedAt: Date.now(),
    staleAt: Date.now() + 10000,
    deleteAt: Date.now() + 20000,
    body: Buffer.from('asd')
  }

  store.set(key, value)

  store.close()

  doesNotMatch(await readFile(sqliteLocation, { encoding:'utf8' }), /topsecret/);
})

[mcollina]

I'm not sure it's worth the computational work. Specifically, I'm not sure we should be storing anything sensitive there (it's cache data after all, explicitly set by the user). If we want to be cautious here, we might add a setting that always filter out some headers.

[metcoder95]

It might be worth it to also extend documentation with advice to be mindful sensitive data, just for overall awareness while caching.

Especially as headers should not be used for sensitive data.

[alxndrsn]

Especially as headers should not be used for sensitive data.

Is that a common recommendation? I often see session cookies and bearer tokens in headers.

[metcoder95]

That's a blur line between sensitive and security information; tho I was referring to response headers in specific.

[mcollina]

I'm somewhat wondering why you'd set cache headers on sensitive information.

Maybe can you make a couple of complete examples?

[alxndrsn]

@metcoder95

I was referring to response headers in specific

Let me re-check my example!

[alxndrsn]

Apologies - here's an updated failing test from a real example:

test('SqliteCacheStore hashes key contents - vary Authorization', async (t) => {
  if (!hasSqlite) {
    t.skip()
    return
  }

  const SqliteCacheStore = require('../../lib/cache/sqlite-cache-store.js')
  const sqliteLocation = 'cache-interceptor.sqlite'

  const store = new SqliteCacheStore({
    location: sqliteLocation
  })

  t.after(async () => {
    //await rm(sqliteLocation)
  })


  /**
   * @type {import('../../types/cache-interceptor.d.ts').default.CacheKey}
   */
  const key = {
    'origin': 'http://localhost:8383',
    'method': 'GET',
    'path': '/v1/projects/34',
    'headers': {
      'cache-control': 'max-stale=3600',
      'pragma': '',
      'authorization': 'Bearer topsecret',
      'accept': '*/*',
      'accept-language': '*',
      'sec-fetch-mode': 'cors',
      'user-agent': 'undici',
      'accept-encoding': 'gzip, deflate'
    }
  }

  /**
   * @type {import('../../types/cache-interceptor.d.ts').default.CacheValue}
   */
  const value = {
    'statusCode': 200,
    'statusMessage': 'OK',
    'headers': {
      'cache-control': 'private, no-cache',
      'vary': 'Accept-Encoding, Authorization, Cookie, Origin',
      'content-type': 'application/json; charset=utf-8',
      'content-length': '173',
      'etag': 'W/\'ad-aolWTkJuy5hxNqIlpkRs6D2F4/U\'',
      'date': 'Tue, 01 Apr 2025 09:07:17 GMT'
    },
    'vary': {
      'accept-encoding': 'gzip, deflate',
      'authorization': 'Bearer topsecret',
      'cookie': null,
      'origin': null
    },
    'cacheControlDirectives': {
      'private': true,
      'no-cache': true
    },
    'cachedAt': 1743498437001,
    'staleAt': 32503680000000, // year 3000
    'deleteAt': 32503680000000, // year 3000
    'etag': 'W/\'ad-aolWTkJuy5hxNqIlpkRs6D2F4/U\'',
    body: Buffer.from('asd')
  }
  
  store.set(key, value)

  store.close()

  doesNotMatch(await readFile(sqliteLocation, { encoding:'utf8' }), /topsecret/);
})

I'm somewhat wondering why you'd set cache headers on sensitive information.

@mcollina I'm guessing from this you'd recommend not including Authorization in the Vary header, and always using no-store for requests which require auth. Is that correct?

[mcollina]

Yes, that would be what I'd do.