fix: make SSR auth middleware cache-aware to prevent user leak in __NUXT__ payload (#52) - #57
Closed
frederikprijck wants to merge 23 commits into
Closed
fix: make SSR auth middleware cache-aware to prevent user leak in __NUXT__ payload (#52)#57frederikprijck wants to merge 23 commits into
frederikprijck wants to merge 23 commits into
Conversation
The Nuxt app-level getRouteRules() composable reads a build-time manifest that omits bare Cache-Control header route rules, so the guard never detected the shared-cacheable case and PII still leaked into the payload. Read the resolved rules from h3Event.context._nitro.routeRules instead. Adds an end-to-end test (offline, stateless cookie session) demonstrating the leak on a non-cacheable route and its absence on a shared-cacheable route. (#52)
…ernal Replace the `h3Event.context._nitro.routeRules` private-field access with Nitro's supported `getRouteRules(event)` server util (dynamically imported from `nitropack/runtime`, server-only to keep it out of the client bundle). Same resolved rules, no private API. (#52)
CVE-2026-53721) Nitro's route-rule matcher is case-sensitive, but vue-router matches routes case-insensitively. A mixed-case request (e.g. /Cacheable) therefore renders the /cacheable page while resolving NO route rule — bypassing the shared-cacheable cache guard and writing the authenticated user into SSR HTML that the consumer marked publicly cacheable. Nuxt's own fix (4.4.7 / 3.21.7) lowercases the path only in the app-level and nitro-server matchers; the 'nitropack/runtime' getRouteRules path this middleware uses is not covered, so the bypass still reproduces on patched Nuxt. Harden in our own code: after checking the request path as-is, re-resolve the route rules for the lowercased path (via a synthetic event with a fresh context so getRouteRules recomputes) and skip the write if either is shared-cacheable. Skipping is always fail-safe — the client hydrates the user. - middleware: dual lookup (request path + lowercased path) - unit test: shared-cacheable bypass via casing skips the write - e2e: /Cacheable renders anonymous SSR HTML (leaked before the fix)
📝 WalkthroughWalkthroughThe module detects shared-cacheable SSR requests, omits authenticated users from their Nuxt payloads, and hydrates users client-side through a no-store ChangesCache-aware authentication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant SSRMiddleware
participant ProfileHandler
participant Auth0Client
Browser->>SSRMiddleware: request shared-cacheable page
SSRMiddleware->>SSRMiddleware: detect shared cacheability
SSRMiddleware-->>Browser: anonymous SSR payload
Browser->>ProfileHandler: fetch /auth/profile after suspense
ProfileHandler->>Auth0Client: getUser()
Auth0Client-->>ProfileHandler: user or null
ProfileHandler-->>Browser: profile response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ort.meta.server (#52) The cache guard's `import('nitropack/runtime')` was pulled into the client bundle, breaking `nuxt build` (`Could not load .nuxt/nitro.client.mjs`). `nitropack/runtime` resolves to server-only Nitro virtual modules that cannot load in the client. The `importMetaServer` helper guard does not let the bundler eliminate the import: it is an opaque re-export Vite cannot statically evaluate. Only a literal `import.meta.server` guard folds to `false` in the client build so Rollup tree-shakes the whole block (and the dynamic import) away. But Nuxt constant-folds `import.meta.*` at transform time even under Vitest's node environment, so a literal guard makes the middleware body unreachable in unit tests — which is exactly why the mockable helper existed. To keep fast unit coverage of the branching cache-guard logic (including the CVE-2026-53721 lowercase re-check), it is extracted into a pure, dependency-injected `isRequestSharedCacheable` with its own unit test. The middleware becomes a thin `import.meta.server` shell, covered by the e2e caching fixture. - Add `isRequestSharedCacheable(getRouteRules, event)` + unit spec. - Gate middleware on literal `import.meta.server`; delegate to the pure function. - Remove `auth.server.spec.ts` (logic now covered by the pure-fn spec + e2e). Verified: package + both example apps build, 66 unit tests pass, lint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
frederikprijck
marked this pull request as ready for review
July 14, 2026 09:58
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/auth0-nuxt/src/runtime/middleware/auth.server.ts (2)
34-38: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the dynamic import to avoid overhead on every request.
While
import()is cached by the module system, calling it still creates a Promise and queues microtasks on every SSR request. For middleware running on every navigation, it is more efficient to cache the resolved function in a module-scoped variable after the first load.⚡ Proposed fix
+let getNitroRouteRules: typeof import('nitropack/runtime').getRouteRules; + export default defineNuxtRouteMiddleware(async () => { if (import.meta.server) { const app = useNuxtApp(); const h3Event = app.ssrContext!.event; - const { getRouteRules } = await import('nitropack/runtime'); + if (!getNitroRouteRules) { + const nitroRuntime = await import('nitropack/runtime'); + getNitroRouteRules = nitroRuntime.getRouteRules; + } - if (isRequestSharedCacheable(getRouteRules, h3Event)) { + if (isRequestSharedCacheable(getNitroRouteRules, h3Event)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/auth0-nuxt/src/runtime/middleware/auth.server.ts` around lines 34 - 38, Cache the resolved getRouteRules function at module scope instead of awaiting the Nitro runtime import on every request. Update the middleware flow around isRequestSharedCacheable to lazily initialize and reuse the cached function while preserving the existing fail-closed behavior when route rules are unavailable.
34-38: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the dynamic import to avoid overhead on every request.
While
import()is cached by the module system, calling it still creates a Promise and queues microtasks on every SSR request. For middleware running on every navigation, it is more efficient to cache the resolved function in a module-scoped variable after the first load.⚡ Proposed fix
+let getNitroRouteRules: typeof import('nitropack/runtime').getRouteRules; + export default defineNuxtRouteMiddleware(async () => { if (import.meta.server) { const app = useNuxtApp(); const h3Event = app.ssrContext!.event; - const { getRouteRules } = await import('nitropack/runtime'); + if (!getNitroRouteRules) { + const nitroRuntime = await import('nitropack/runtime'); + getNitroRouteRules = nitroRuntime.getRouteRules; + } - if (isRequestSharedCacheable(getRouteRules, h3Event)) { + if (isRequestSharedCacheable(getNitroRouteRules, h3Event)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/auth0-nuxt/src/runtime/middleware/auth.server.ts` around lines 34 - 38, Cache the resolved getRouteRules function at module scope instead of awaiting import('nitropack/runtime') on every request in the middleware flow. Update the initialization path around isRequestSharedCacheable to load it once and reuse the cached function for subsequent requests, while preserving the existing fail-closed behavior when route rules are unavailable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/auth0-nuxt/src/runtime/middleware/auth.server.ts`:
- Around line 34-38: Cache the resolved getRouteRules function at module scope
instead of awaiting the Nitro runtime import on every request. Update the
middleware flow around isRequestSharedCacheable to lazily initialize and reuse
the cached function while preserving the existing fail-closed behavior when
route rules are unavailable.
- Around line 34-38: Cache the resolved getRouteRules function at module scope
instead of awaiting import('nitropack/runtime') on every request in the
middleware flow. Update the initialization path around isRequestSharedCacheable
to load it once and reuse the cached function for subsequent requests, while
preserving the existing fail-closed behavior when route rules are unavailable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a679150b-68e0-41b6-a538-41c9eab99a96
📒 Files selected for processing (21)
packages/auth0-nuxt/EXAMPLES.mdpackages/auth0-nuxt/README.mdpackages/auth0-nuxt/src/module.spec.tspackages/auth0-nuxt/src/module.tspackages/auth0-nuxt/src/runtime/helpers/import-meta.tspackages/auth0-nuxt/src/runtime/middleware/auth.server.tspackages/auth0-nuxt/src/runtime/plugins/auth.client.spec.tspackages/auth0-nuxt/src/runtime/plugins/auth.client.tspackages/auth0-nuxt/src/runtime/server/api/auth/profile.get.spec.tspackages/auth0-nuxt/src/runtime/server/api/auth/profile.get.tspackages/auth0-nuxt/src/runtime/server/utils/is-request-shared-cacheable.spec.tspackages/auth0-nuxt/src/runtime/server/utils/is-request-shared-cacheable.tspackages/auth0-nuxt/src/runtime/server/utils/is-shared-cacheable.spec.tspackages/auth0-nuxt/src/runtime/server/utils/is-shared-cacheable.tspackages/auth0-nuxt/src/types.tspackages/auth0-nuxt/test/caching.test.tspackages/auth0-nuxt/test/fixtures/caching/app.vuepackages/auth0-nuxt/test/fixtures/caching/nuxt.config.tspackages/auth0-nuxt/test/fixtures/caching/package.jsonpackages/auth0-nuxt/test/fixtures/caching/pages/cacheable.vuepackages/auth0-nuxt/test/fixtures/caching/pages/private.vue
…che guard (#52) The cache guard only flagged `public`/`s-maxage`, so a `Cache-Control: max-age=N` route rule still wrote the user into the __NUXT__ payload. RFC 9111's authenticated-response restriction keys on the Authorization header, not cookies, so a path-keyed shared cache can store a cookie-authenticated `max-age` response and serve it across users. Treat a positive `max-age` as shared-cacheable, honor explicit `private`/`no-store` opt-outs, and let `public`/`s-maxage` win over a contradictory `private`.
| // cannot be loaded in the client environment and would otherwise break `nuxt build`. The | ||
| // branching cache-guard logic lives in `isRequestSharedCacheable` so it stays unit-testable | ||
| // (this body is unreachable under Vitest, where the same guard folds to `false`). | ||
| if (import.meta.server) { |
Contributor
There was a problem hiding this comment.
Hey @frederikprijck Just double-checking this one: does the client build still need to resolve nitropack/runtime here despite the import.meta.server guard?
While trying out this PR I ran into the following build error:
Package import specifier "#nitro-internal-virtual/error-handler" is not defined in package
I believe it's coming from:
const { getRouteRules } = await import('nitropack/runtime');
I believe since this middleware is also bundled into the client layer, it looks like the bundler still resolves nitropack/runtime (and its transitive #nitro-internal-virtual/* deps) even though the branch is dead in the client, so the guard isn't fully stripping it, and nuxt build fails.
Member
Author
There was a problem hiding this comment.
As discussed, let's see if we can get rid of the middleware instead in a non breaking way, and avoid the problem altogether.
Move the shared-cacheability check from middleware into a request-lifecycle plugin that runs before route middleware, avoiding the dynamic import in middleware and providing better separation of concerns. Also updates test dependencies to @nuxt/test-utils 4.1.0.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/auth0-nuxt/src/runtime/server/plugins/cache-detection.server.spec.ts`:
- Around line 18-45: Update the getRouteRules mock in the cacheability tests so
unmatched paths return an empty route-rules object rather than undefined,
allowing the case-sensitivity test to exercise the lowercase retry. Add a
separate assertion verifying that an unavailable route-rule result of undefined
makes isRequestSharedCacheable return true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d7b7019-f163-413b-a20d-0b135b70cc61
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
packages/auth0-nuxt/.nuxtrcpackages/auth0-nuxt/src/runtime/server/plugins/cache-detection.server.spec.tspackages/auth0-nuxt/src/runtime/server/plugins/cache-detection.server.ts
Comment on lines
+18
to
+45
| const getRouteRules = vi.fn((event: { path: string }) => { | ||
| if (event.path === '/cacheable') { | ||
| return { cache: { swr: 60 } }; | ||
| } | ||
| return undefined; | ||
| }); | ||
|
|
||
| const cacheableEvent = { path: '/cacheable', context: {} }; | ||
| const privateEvent = { path: '/private', context: {} }; | ||
|
|
||
| expect(isRequestSharedCacheable(getRouteRules, cacheableEvent)).toBe(true); | ||
| expect(isRequestSharedCacheable(getRouteRules, privateEvent)).toBe(false); | ||
| }); | ||
|
|
||
| it('handles case-sensitivity bypass (CVE-2026-53721)', async () => { | ||
| const { isRequestSharedCacheable } = await import('../utils/is-request-shared-cacheable'); | ||
|
|
||
| const getRouteRules = vi.fn((event: { path: string }) => { | ||
| if (event.path === '/cacheable') { | ||
| return { cache: { swr: 60 } }; | ||
| } | ||
| return undefined; | ||
| }); | ||
|
|
||
| const uppercaseEvent = { path: '/Cacheable', context: {} }; | ||
|
|
||
| // The function should detect that the lowercased path matches a cacheable rule | ||
| expect(isRequestSharedCacheable(getRouteRules, uppercaseEvent)).toBe(true); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not model an unmatched route as unavailable route rules.
undefined deliberately means route-rule lookup is unavailable and therefore fails closed to true. Consequently, Line 29 fails, and the case-variant assertion at Line 45 succeeds without exercising the lowercase retry. Return {} for unmatched paths; add a separate assertion that undefined returns true.
Proposed fix
const getRouteRules = vi.fn((event: { path: string }) => {
if (event.path === '/cacheable') {
return { cache: { swr: 60 } };
}
- return undefined;
+ return {};
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const getRouteRules = vi.fn((event: { path: string }) => { | |
| if (event.path === '/cacheable') { | |
| return { cache: { swr: 60 } }; | |
| } | |
| return undefined; | |
| }); | |
| const cacheableEvent = { path: '/cacheable', context: {} }; | |
| const privateEvent = { path: '/private', context: {} }; | |
| expect(isRequestSharedCacheable(getRouteRules, cacheableEvent)).toBe(true); | |
| expect(isRequestSharedCacheable(getRouteRules, privateEvent)).toBe(false); | |
| }); | |
| it('handles case-sensitivity bypass (CVE-2026-53721)', async () => { | |
| const { isRequestSharedCacheable } = await import('../utils/is-request-shared-cacheable'); | |
| const getRouteRules = vi.fn((event: { path: string }) => { | |
| if (event.path === '/cacheable') { | |
| return { cache: { swr: 60 } }; | |
| } | |
| return undefined; | |
| }); | |
| const uppercaseEvent = { path: '/Cacheable', context: {} }; | |
| // The function should detect that the lowercased path matches a cacheable rule | |
| expect(isRequestSharedCacheable(getRouteRules, uppercaseEvent)).toBe(true); | |
| const getRouteRules = vi.fn((event: { path: string }) => { | |
| if (event.path === '/cacheable') { | |
| return { cache: { swr: 60 } }; | |
| } | |
| return {}; | |
| }); | |
| const cacheableEvent = { path: '/cacheable', context: {} }; | |
| const privateEvent = { path: '/private', context: {} }; | |
| expect(isRequestSharedCacheable(getRouteRules, cacheableEvent)).toBe(true); | |
| expect(isRequestSharedCacheable(getRouteRules, privateEvent)).toBe(false); | |
| }); | |
| it('handles case-sensitivity bypass (CVE-2026-53721)', async () => { | |
| const { isRequestSharedCacheable } = await import('../utils/is-request-shared-cacheable'); | |
| const getRouteRules = vi.fn((event: { path: string }) => { | |
| if (event.path === '/cacheable') { | |
| return { cache: { swr: 60 } }; | |
| } | |
| return undefined; | |
| }); | |
| const uppercaseEvent = { path: '/Cacheable', context: {} }; | |
| // The function should detect that the lowercased path matches a cacheable rule | |
| expect(isRequestSharedCacheable(getRouteRules, uppercaseEvent)).toBe(true); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/auth0-nuxt/src/runtime/server/plugins/cache-detection.server.spec.ts`
around lines 18 - 45, Update the getRouteRules mock in the cacheability tests so
unmatched paths return an empty route-rules object rather than undefined,
allowing the case-sensitivity test to exercise the lowercase retry. Add a
separate assertion verifying that an unavailable route-rule result of undefined
makes isRequestSharedCacheable return true.
Contributor
|
Closing this as PR #59 consolidated changes from this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The global SSR auth middleware wrote the authenticated user into
useState('auth0_user'), which Nuxt serializes into the__NUXT__payload of the server-rendered HTML. On any shared-cacheable route (CDN /s-maxage/swr/isr), that HTML — with one user's PII baked in — could be stored and served to other visitors.Fix
The middleware now consults the Nitro route rules resolved for the request and skips the SSR user write when the response is shared-cacheable. The user is instead hydrated client-side from a
no-store/auth/profileendpoint, so authenticated UI still works without ever placing PII into a cacheable payload.isSharedCacheable()predicate detectscache/swr/isrrules andpublic/s-maxageCache-Control headers.getRouteRulesserver util (nitropack/runtime).no-store/auth/profileendpoint +.clientplugin re-hydrate the user after load.Closes #52.
Tests
isSharedCacheablepredicate, middleware cache guard (incl. fail-closed + CVE case-variants), profile endpoint, client plugin.cachingfixture asserting the user is server-rendered on ano-storeroute but absent from a shared-cacheable route's raw HTML.Summary by CodeRabbit
/auth/profileroute (and optional customization) for profile fetching withno-storecaching behavior.