From d7c305f767a0e2139580f8c5912367c2a134d598 Mon Sep 17 00:00:00 2001 From: derrickmehaffy Date: Mon, 12 Oct 2026 21:14:20 -0700 Subject: [PATCH] fix: say private for every route keyed per caller under scope public With `cacheControl.scope: "public"`, routes keyed per caller are downgraded to private, because a shared cache told their responses are public could hand one caller's response to another. The check compared `keys.useAuth === true`, while the key generator puts the caller in the key for any truthy useAuth. A raw string - `useAuth: env("X")` without env.bool - therefore keyed entries per caller and still went out public. The downgrade and its boot warning now use truthiness, as the key generator does. Only configurations with a truthy non-boolean useAuth under scope public change, and only from public to private. --- .../src/utils/config/resolveUserStrategy.ts | 3 +- .../utils/middlewares/buildCacheControl.ts | 8 +-- shared/config/cache-strategy.js | 7 ++- shared/tests/flag-cache-control.test.js | 51 +++++++++++++++++++ 4 files changed, 64 insertions(+), 5 deletions(-) diff --git a/packages/plugin-rest-cache/server/src/utils/config/resolveUserStrategy.ts b/packages/plugin-rest-cache/server/src/utils/config/resolveUserStrategy.ts index 7c1c63d4..7b916952 100644 --- a/packages/plugin-rest-cache/server/src/utils/config/resolveUserStrategy.ts +++ b/packages/plugin-rest-cache/server/src/utils/config/resolveUserStrategy.ts @@ -322,7 +322,8 @@ export const resolveUserStrategy = function ( ) { for (const cacheConfig of cacheConfigs) { const keyedPerCaller = cacheConfig.routes.filter( - (route) => route.keys?.useAuth === true + // Truthiness, as buildCacheControl and the key generator decide it. + (route) => Boolean(route.keys?.useAuth) ); if (!keyedPerCaller.length) { diff --git a/packages/plugin-rest-cache/server/src/utils/middlewares/buildCacheControl.ts b/packages/plugin-rest-cache/server/src/utils/middlewares/buildCacheControl.ts index 539fc911..86980073 100644 --- a/packages/plugin-rest-cache/server/src/utils/middlewares/buildCacheControl.ts +++ b/packages/plugin-rest-cache/server/src/utils/middlewares/buildCacheControl.ts @@ -46,10 +46,12 @@ export const buildCacheControl = function ( // strategy level cannot be expected to re-check it against every route's // keys. register.ts warns at boot when this applies. // See https://github.com/strapi-community/plugin-rest-cache/issues/113 + // + // Truthiness, not `=== true`: the key generator puts the caller in the key + // for any truthy useAuth - the raw string `useAuth: env("X")` included - so + // that is what decides whether the response is caller-specific. const scope = - config.scope === 'public' && cacheRouteConfig.keys?.useAuth === true - ? 'private' - : config.scope; + config.scope === 'public' && cacheRouteConfig.keys?.useAuth ? 'private' : config.scope; directives.push(scope); diff --git a/shared/config/cache-strategy.js b/shared/config/cache-strategy.js index 40baf6b2..26c085a2 100644 --- a/shared/config/cache-strategy.js +++ b/shared/config/cache-strategy.js @@ -62,7 +62,12 @@ module.exports = ({ env }) => ({ keys: { useQueryParams: false, useHeaders: ["accept-encoding"], - useAuth: env.bool("CATEGORY_USE_AUTH", true), + // CATEGORY_USE_AUTH_RAW passes the raw string through, as + // `useAuth: env("X")` without env.bool does: truthy, but not `true`. + useAuth: + env("CATEGORY_USE_AUTH_RAW", "") !== "" + ? env("CATEGORY_USE_AUTH_RAW") + : env.bool("CATEGORY_USE_AUTH", true), }, routes: [ // Deliberately configured for caching so the plugin's refusal to cache diff --git a/shared/tests/flag-cache-control.test.js b/shared/tests/flag-cache-control.test.js index 95d719a0..5c11048b 100644 --- a/shared/tests/flag-cache-control.test.js +++ b/shared/tests/flag-cache-control.test.js @@ -205,3 +205,54 @@ describe("cacheControl: enabled with scope public", () => { expect(cacheControlOf(first)).toBe("public, max-age=3600"); }); }); + +// keys.useAuth set to a truthy value that is not the boolean true - the raw +// string `useAuth: env("X")` gives - keys entries per caller exactly as `true` +// does. The downgrade compared `=== true`, so those caller-specific responses +// went out `public` and a shared cache could hand one caller's to another. +describe("cacheControl: scope public, useAuth set to a truthy non-boolean", () => { + let bootOutput = ""; + + beforeAll(async () => { + process.env.KEYS_PREFIX = undefined; + process.env.CACHE_CONTROL_ENABLED = "true"; + process.env.CACHE_CONTROL_SCOPE = "public"; + process.env.CATEGORY_USE_AUTH_RAW = "true"; + process.env.STRAPI_LOG_LEVEL = "warn"; + + const streams = [process.stdout, console._stdout].filter( + (stream, index, all) => stream && all.indexOf(stream) === index + ); + const spies = streams.map((stream) => { + const write = stream.write.bind(stream); + return jest.spyOn(stream, "write").mockImplementation((chunk, ...rest) => { + bootOutput += String(chunk); + return write(chunk, ...rest); + }); + }); + + try { + await setup(); + } finally { + spies.forEach((spy) => spy.mockRestore()); + } + }); + afterAll(async () => { + delete process.env.CACHE_CONTROL_ENABLED; + delete process.env.CACHE_CONTROL_SCOPE; + delete process.env.CATEGORY_USE_AUTH_RAW; + delete process.env.STRAPI_LOG_LEVEL; + await teardown(); + }); + + beforeEach(() => store().reset()); + + it("still downgrades to private", async () => { + const first = await agent().get("/api/categories"); + const second = await agent().get("/api/categories"); + + expect(second.get("x-cache")).toBe("HIT"); + expect(cacheControlOf(first)).toBe("private, max-age=3600"); + expect(cacheControlOf(second)).toBe("private, max-age=3600"); + }); +});