From 3122dd08a8bc08deb937236aaa3f750119a979b9 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Tue, 12 May 2026 22:53:49 +0200 Subject: [PATCH] fix(customer-routes): Cache-Control: no-store on customer endpoints (#470) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trigger: PR #458 mounted requireCustomerPortalEnabled which 410'd every /api/customer/* + /api/admin/customers/* request when the master toggle was off. Some browsers cached that 410 (no Cache-Control header was set, so heuristic freshness applied — the wrong default for an authenticated/sensitive surface). PR #470 reverted the middleware, but a customer whose tab cached the 410 still saw 410s until they hard-refreshed. Add noStoreCache middleware and mount it in front of both route groups. Every response (200, 4xx, 5xx) now carries `Cache-Control: no-store, no-cache, must-revalidate, private` plus the HTTP/1.0 Pragma + Expires fallbacks. Any future transient error from these endpoints can no longer get pinned in browser or proxy caches and outlive its cause. Cost is one setHeader per request; applied per route group rather than globally so static assets + galleries keep their own caching strategy unchanged. Includes a dedicated unit test pinning the header set so a future cleanup pass can't quietly drop it and re-introduce the bug. --- backend/server.js | 14 +++++-- backend/src/__tests__/noStoreCache.test.js | 48 ++++++++++++++++++++++ backend/src/middleware/noStoreCache.js | 39 ++++++++++++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 backend/src/__tests__/noStoreCache.test.js create mode 100644 backend/src/middleware/noStoreCache.js diff --git a/backend/server.js b/backend/server.js index dd0d9ee2..fbcdb2f1 100644 --- a/backend/server.js +++ b/backend/server.js @@ -593,11 +593,19 @@ app.use('/api/admin/users', require('./src/routes/adminUsers')); // Putting the global flag in the kill-switch role was a mistake — a // stray click in Settings → Features would lock every paying // customer out at once. PR-revert moved the gate back to per-record. -app.use('/api/admin/customers', require('./src/routes/adminCustomers')); +// +// `noStoreCache` belt-and-braces the cache-control story for both +// surfaces: any response — 200, 4xx, 5xx — carries `Cache-Control: +// no-store` so a transient error (the now-reverted #458 410, a +// permission flip mid-session, a backend restart) can't get pinned +// in browser or intermediate caches and outlive its cause. See the +// PR #458 → #470 history in the middleware file for context. +const { noStoreCache } = require('./src/middleware/noStoreCache'); +app.use('/api/admin/customers', noStoreCache, require('./src/routes/adminCustomers')); // Customer-side surface (#354). Strictly separate from /api/admin/* — // distinct token type, distinct cookie, distinct middleware. -app.use('/api/customer/auth', require('./src/routes/customerAuth')); -app.use('/api/customer', require('./src/routes/customer')); +app.use('/api/customer/auth', noStoreCache, require('./src/routes/customerAuth')); +app.use('/api/customer', noStoreCache, require('./src/routes/customer')); app.use('/api/admin/event-types', require('./src/routes/adminEventTypes')); app.use('/api/admin/api-tokens', require('./src/routes/adminApiTokens')); app.use('/api/admin/webhooks', require('./src/routes/adminWebhooks')); diff --git a/backend/src/__tests__/noStoreCache.test.js b/backend/src/__tests__/noStoreCache.test.js new file mode 100644 index 00000000..048cc480 --- /dev/null +++ b/backend/src/__tests__/noStoreCache.test.js @@ -0,0 +1,48 @@ +/** + * Unit test for the noStoreCache middleware (#470 follow-up). + * + * The middleware exists because of a real production-class bug: when + * #458 mounted a 410-returning kill-switch in front of customer + * endpoints, browsers cached the 410 (no Cache-Control was set) and + * kept serving it after #470 reverted the middleware. This test pins + * the contract so a future cleanup pass doesn't quietly drop the + * header set and re-introduce the bug. + */ + +const { noStoreCache } = require('../middleware/noStoreCache'); + +function makeRes() { + const headers = {}; + return { + setHeader: (k, v) => { headers[k] = v; }, + headers, + }; +} + +describe('noStoreCache middleware', () => { + it('sets Cache-Control: no-store + private and calls next()', () => { + const res = makeRes(); + const next = jest.fn(); + + noStoreCache({}, res, next); + + expect(res.headers['Cache-Control']).toBe( + 'no-store, no-cache, must-revalidate, private', + ); + // HTTP/1.0 fallbacks — old proxies in front of customer-facing + // surfaces (corporate VPN gateways, legacy CDNs) honour these. + expect(res.headers.Pragma).toBe('no-cache'); + expect(res.headers.Expires).toBe('0'); + expect(next).toHaveBeenCalledTimes(1); + }); + + it('runs as middleware regardless of response status', () => { + // The header must land on EVERY response coming from the route + // group — including 4xx/5xx — so a stale 410 from a + // now-reverted middleware can't get pinned in browser cache like + // it did in the #458 → #470 sequence. + const res = makeRes(); + noStoreCache({}, res, () => {}); + expect(res.headers['Cache-Control']).toContain('no-store'); + }); +}); diff --git a/backend/src/middleware/noStoreCache.js b/backend/src/middleware/noStoreCache.js new file mode 100644 index 00000000..f95de501 --- /dev/null +++ b/backend/src/middleware/noStoreCache.js @@ -0,0 +1,39 @@ +/** + * Cache-Control: no-store helper for sensitive endpoints. + * + * Why a dedicated middleware: shipping the wrong cache-control header + * on a session-bearing endpoint is a class-of-bug that bites long + * after the original mistake. The PR #458 / PR #470 history is the + * concrete trigger: + * + * - #458 mounted requireCustomerPortalEnabled which 410'd every + * /api/customer/* and /api/admin/customers/* request when the + * master toggle was off. + * - Some browsers cached the 410 (the response carried no explicit + * Cache-Control header, so heuristic freshness applied — for an + * authenticated/sensitive surface that's the wrong default). + * - #470 reverted the middleware, but a customer whose tab cached + * the 410 still saw 410s until they hard-refreshed. + * + * Mounting `noStoreCache` in front of these routes belt-and-braces + * the future: any 4xx/5xx (or 200) response from these endpoints + * carries `Cache-Control: no-store`, so a transient kill-switch, + * permission flip, or backend restart can never get pinned in + * intermediate caches. + * + * No-op cost (one setHeader per request); applied per route group + * rather than globally so static assets + galleries keep their + * own caching strategy. + */ + +function noStoreCache(req, res, next) { + // `no-store` is the strongest signal — no cache, no revalidation, + // no offline retention. Pair with `private` so any well-behaved + // intermediate proxy treats the response as user-specific. + res.setHeader('Cache-Control', 'no-store, no-cache, must-revalidate, private'); + res.setHeader('Pragma', 'no-cache'); // HTTP/1.0 fallback for older proxies + res.setHeader('Expires', '0'); + next(); +} + +module.exports = { noStoreCache };