From 154449d77acce21fbefe4b5c6c23d9cba11a8854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 3 Aug 2026 13:49:19 +0200 Subject: [PATCH 1/2] Stop a prefetching page from seeding the wrong person's record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A server-prefetching page dehydrates a health record into the HTML and seeds it into the client's query cache under the key the client cell reads. That copy is only right when the account the page read is the account the client's route would answer with, and under an account switch it never is: `getSession()` answers who is CALLING, deliberately and permanently, so all five prefetching pages seeded the delegate's own rows into a document opened on somebody else's record. The dashboard was the worst of it — snapshot, digest and widget layout ride routes that refuse the refetch, so the delegate's numbers stayed on the owner's dashboard, while the chart row refetched to the owner and left the page showing two people at once under a banner naming one. Resolve the session through `getUnswitchedSession()` instead, which answers null while the browser is acting on another record. The prefetch steps aside and every cell fetches through its route, which is where the grant check lives. Resolving the owner here instead was the other candidate and is worse: the grant check has exactly one home, half of what these pages prefetch rides routes that declared themselves not delegable, and re-deriving it in an RSC would be a second program deciding whose record this is. Close the two shell-level doors beside it. The Coach launcher, its drawer mount and the module tour hang off `` rather than off a route, so dropping Insights and Coach from the navigation under a switch left them painting on every page in the record and on the unavailable panel too — one tap from a drawer whose every action refuses, and a `POST /api/insights/coach/seen` fired by opening it at all. All three bind `useRecordCapabilities()`, the same resolved grant the banner and the switcher read. Absent, not disabled. The prefetch rule is structural: the guard walks the app tree, so a sixth prefetching page is covered the moment it lands rather than the moment somebody remembers. The pages stay on the session-surface allowlist with their reasons rewritten, because a wrapper one directory deeper is the easiest way for a surface to walk out of a guard. --- e2e/account-sharing.spec.ts | 15 ++ .../prefetch-page-caching-guard.test.ts | 60 ++++- src/__tests__/session-surface-guard.test.ts | 43 +++- .../prefetch-record-identity.test.tsx | 223 ++++++++++++++++++ .../coach/__tests__/page-prefetch.test.tsx | 28 ++- src/app/coach/page.tsx | 9 +- .../insights/__tests__/page-prefetch.test.tsx | 18 +- src/app/insights/page.tsx | 9 +- .../workouts/__tests__/page-prefetch.test.tsx | 22 +- src/app/insights/workouts/page.tsx | 9 +- .../__tests__/page-prefetch.test.tsx | 20 +- src/app/medications/page.tsx | 17 +- src/app/page.tsx | 19 +- .../__tests__/auth-shell-coach-hoist.test.ts | 9 +- .../auth-shell-shared-record-doors.test.tsx | 198 ++++++++++++++++ src/components/layout/auth-shell.tsx | 35 ++- .../auth/__tests__/unswitched-session.test.ts | 79 +++++++ src/lib/auth/acting-carrier.ts | 54 +++++ 18 files changed, 797 insertions(+), 70 deletions(-) create mode 100644 src/app/__tests__/prefetch-record-identity.test.tsx create mode 100644 src/components/layout/__tests__/auth-shell-shared-record-doors.test.tsx create mode 100644 src/lib/auth/__tests__/unswitched-session.test.ts diff --git a/e2e/account-sharing.spec.ts b/e2e/account-sharing.spec.ts index 6a19aff1d..7efa94349 100644 --- a/e2e/account-sharing.spec.ts +++ b/e2e/account-sharing.spec.ts @@ -279,6 +279,15 @@ test.describe("account sharing", () => { ).toHaveCount(0); await page.keyboard.press("Escape"); + // The positive control for the shell-door assertion further down: the + // delegate, in their OWN record, is offered the Coach launcher like anyone + // else. Without this line the later "it is gone" check would also pass on + // a build where the launcher had stopped rendering for everybody. + await page.goto("/"); + await expect(page.locator('[data-slot="coach-fab"]')).toBeVisible({ + timeout: 10_000, + }); + await page.locator('[data-slot="grant-accept"]').click(); await expect(row).toHaveAttribute("data-grant-state", "ACTIVE"); }); @@ -353,6 +362,12 @@ test.describe("account sharing", () => { await expect(page.locator('a[href="/settings/account"]')).toHaveCount(0); await expect(page.locator('a[href="/coach"]')).toHaveCount(0); await expect(page.locator('a[href="/insights"]')).toHaveCount(0); + + // And neither is the door that stood beside the removed entries. The Coach + // launcher hangs off the shell rather than off a route, so dropping the + // nav entry left it floating over every page in the record — one tap from + // a drawer whose every action resolves `requireAuth()` and refuses. + await expect(page.locator('[data-slot="coach-fab"]')).toHaveCount(0); }); test("a surface sharing does not cover says so", async ({ page }) => { diff --git a/src/__tests__/prefetch-page-caching-guard.test.ts b/src/__tests__/prefetch-page-caching-guard.test.ts index e7a21c0a0..39a54ba6a 100644 --- a/src/__tests__/prefetch-page-caching-guard.test.ts +++ b/src/__tests__/prefetch-page-caching-guard.test.ts @@ -5,9 +5,10 @@ import { describe, expect, it } from "vitest"; /** * Structural guard over the server-prefetching pages. * - * A page that calls `dehydrate()` inside a `HydrationBoundary` serialises the - * caller's own health record into the HTML document. Two things must hold for - * every such page, and neither is enforced by the type system: + * A page that calls `dehydrate()` inside a `HydrationBoundary` serialises a + * health record into the HTML document and seeds it into the client's query + * cache. Three things must hold for every such page, and none is enforced by + * the type system: * * 1. It must be session-gated at the edge, so `src/proxy.ts` stamps * `Cache-Control: private, no-store` on the document. A prefetching page @@ -17,9 +18,18 @@ import { describe, expect, it } from "vitest"; * revalidate = ` or `dynamic = "force-static"` would let Next serve one * account's prefetched HTML to the next caller out of its own cache, * before the request ever reaches a header. + * 3. It must resolve its session through `getUnswitchedSession()`, never + * `getSession()`. The first two rules keep one account's record away from + * the NEXT caller; this one keeps it away from the CURRENT one. Under an + * account switch `getSession()` answers with the delegate — that is its + * documented job — so a page that prefetches off it dehydrates the + * delegate's own numbers into a document opened on the owner's record. * * The guard walks the app tree rather than naming files, so a NEW prefetching - * page is covered the moment it lands. + * page is covered the moment it lands. That is the whole point of rule 3 being + * here rather than a line each page remembers: the five pages that prefetch + * today all made the same mistake independently, which is what a rule looks + * like when nothing states it. */ const APP_DIR = join(process.cwd(), "src", "app"); @@ -116,3 +126,45 @@ describe("server-prefetching pages cannot leak a cacheable record", () => { ).toEqual([]); }); }); + +describe("server-prefetching pages cannot seed a switched session", () => { + /** + * Both halves are asserted, and the pair is the guard. + * + * "Calls the right helper" alone passes a page that calls both — the import + * sits there, the accessor is used for a null-check, and the read underneath + * still goes through `getSession()`. "Does not call the wrong one" alone + * passes a page that resolves no session at all and prefetches nothing, + * which is green for a reason nobody wanted. Together they say the one thing + * meant: the session a prefetching page acts on came from the accessor that + * refuses under a switch. + */ + const UNSWITCHED = /\bgetUnswitchedSession\s*\(/; + const RAW_SESSION = /\bgetSession\s*\(/; + + it("resolves every prefetching page's session through getUnswitchedSession", () => { + // Non-vacuous, and it has to be said again in this block: `missing` is + // empty both when every page complies and when the walker found no pages + // at all. The first describe asserts the same thing, and asserting it once + // there would let a drifted detector take this leg down with it silently. + expect(prefetchingPages.length).toBeGreaterThan(0); + + const missing = prefetchingPages + .filter((p) => !UNSWITCHED.test(p.source)) + .map((p) => p.rel); + expect( + missing, + "a prefetching page must take its session from getUnswitchedSession()", + ).toEqual([]); + }); + + it("lets no prefetching page reach getSession directly", () => { + const offenders = prefetchingPages + .filter((p) => RAW_SESSION.test(p.source)) + .map((p) => p.rel); + expect( + offenders, + "getSession() answers who is CALLING — prefetching off it seeds the delegate's record onto the owner's page", + ).toEqual([]); + }); +}); diff --git a/src/__tests__/session-surface-guard.test.ts b/src/__tests__/session-surface-guard.test.ts index 0d49280d7..a1e42ed56 100644 --- a/src/__tests__/session-surface-guard.test.ts +++ b/src/__tests__/session-surface-guard.test.ts @@ -104,13 +104,27 @@ describe("S1 — the direct session-resolution set is frozen", () => { // RSC prefetch: the server component hydrates the client query cache // with the session user's own data. Absence of a session skips the // prefetch rather than failing the render. - "app/coach/page.tsx": "RSC prefetch of the session user's coach state", - "app/insights/page.tsx": "RSC prefetch of the session user's insights", + // + // These five resolve through `getUnswitchedSession()`, not `getSession()`, + // and they stay listed here BECAUSE of it. The helper is still a session + // derivation outside `requireAuth` — moving the call one file deeper into + // `lib/` would have taken them off this list while changing nothing about + // what they do, and a guard that a refactor can walk out of is not one. + // What the helper adds is the record identity: it answers null while the + // browser acts on somebody else's record, so the cache these pages seed + // can only ever hold the caller's own. See + // `prefetch-page-caching-guard.test.ts` for the rule that keeps a sixth + // prefetching page from reaching for the bare helper. + "app/coach/page.tsx": + "RSC prefetch of the session user's coach state, skipped under a switch", + "app/insights/page.tsx": + "RSC prefetch of the session user's insights, skipped under a switch", "app/insights/workouts/page.tsx": - "RSC prefetch of the session user's workouts", + "RSC prefetch of the session user's workouts, skipped under a switch", "app/medications/page.tsx": - "RSC prefetch of the session user's medications", - "app/page.tsx": "RSC prefetch of the session user's dashboard snapshot", + "RSC prefetch of the session user's medications, skipped under a switch", + "app/page.tsx": + "RSC prefetch of the session user's dashboard snapshot, skipped under a switch", // Onboarding routing. Reads the actor's own onboarding step to redirect; // renders no record data. "app/onboarding/[step]/page.tsx": @@ -119,15 +133,20 @@ describe("S1 — the direct session-resolution set is frozen", () => { }; /** - * The identity-deriving helpers exported by `lib/auth/session.ts`. The - * lifecycle exports (`createSession`, `destroySession`, `destroyAllSessions`, - * `destroySessionById`, `destroyOtherSessions`) are deliberately NOT here: - * they act on a session, they do not hand the handler an identity to scope - * a query by. Adding a sixth deriving helper to `session.ts` means adding - * it to this regex. + * The identity-deriving helpers exported by `lib/auth/session.ts`, plus the + * one that wraps it. The lifecycle exports (`createSession`, + * `destroySession`, `destroyAllSessions`, `destroySessionById`, + * `destroyOtherSessions`) are deliberately NOT here: they act on a session, + * they do not hand the handler an identity to scope a query by. Adding a + * further deriving helper means adding it to this regex. + * + * `getUnswitchedSession` (`lib/auth/acting-carrier.ts`) is listed for the + * reason a wrapper is the easiest way to disappear from a guard: it returns + * `getSession()`'s answer or null, so a caller of it derives an identity + * exactly as much as a caller of the thing it wraps. */ const SESSION_DERIVING_HELPERS = - /\b(getSession|getSessionUserLocale|validateSessionFromCookieValue|validateSessionWithCreatedAt)\s*\(/; + /\b(getSession|getUnswitchedSession|getSessionUserLocale|validateSessionFromCookieValue|validateSessionWithCreatedAt)\s*\(/; /** * Reaching for the cookie by name is the way around the helpers. Both the diff --git a/src/app/__tests__/prefetch-record-identity.test.tsx b/src/app/__tests__/prefetch-record-identity.test.tsx new file mode 100644 index 000000000..01228963b --- /dev/null +++ b/src/app/__tests__/prefetch-record-identity.test.tsx @@ -0,0 +1,223 @@ +/** + * Whose record a server-prefetching page serialises into the HTML. + * + * The prefetch is a server-side copy of what a route would answer. It is only + * ever correct when the account the RSC read equals the account the client's + * route would serve, and under an account switch those two are never the same + * account: + * + * - `getSession()` answers who is CALLING and deliberately never the record + * being acted on (`src/lib/auth/session.ts`), so the RSC read the + * delegate's own rows; + * - a NON-delegable route (`/api/dashboard/snapshot`, `/api/daily/digest`, + * `/api/dashboard/widgets`) refuses the refetch outright, so the seeded + * value is what stays on screen — under a banner naming somebody else; + * - a DELEGABLE route (`/api/measurements/series-batch`, `/api/medications`) + * answers with the owner's rows, so the same page paints two people at + * once until the refetch lands. + * + * These tests assert what reaches the cache, not which branch ran. Each one + * reads the dehydrated queries off the returned `` and + * checks the payload by a marker unique to the account it came from, so a + * prefetch that seeds the wrong person fails here even if every guard around + * it stays green. + * + * Mutation checks, run: + * - restore `getSession()` in `src/app/page.tsx` → "seeds nothing while the + * session is acting on another record" goes red, PRINTING the four cache + * cells it found, each carrying the delegate's marker. + * - restore it in `src/app/medications/page.tsx` → the medications leg goes + * red the same way, printing the delegate's list. + * - make `getUnswitchedSession` return the session regardless of the carrier + * → both go red at once, which is the whole feature failing in one place. + */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { HydrationBoundary, hashKey } from "@tanstack/react-query"; +import type { ReactElement } from "react"; + +import { queryKeys } from "@/lib/query-keys"; + +const getSession = vi.fn(); +const resolveModuleMap = vi.fn(); +const readDashboardSnapshotCached = vi.fn(); +const loadDailyDigest = vi.fn(); +const readSeriesBatch = vi.fn(); +const readMedicationsListCached = vi.fn(); + +// The acting-carrier module is deliberately NOT mocked. The property under +// test is what a real switched session does to a real page, so the only thing +// stubbed on that path is the session row itself and the header context the +// carrier decision reads. A test that mocked `getUnswitchedSession` would +// prove the pages honour a boolean, which is not the same claim. +vi.mock("@/lib/auth/session", () => ({ getSession: () => getSession() })); +vi.mock("next/headers", () => ({ + headers: async () => ({ get: () => null }), +})); +vi.mock("@/lib/modules/gate", () => ({ + resolveModuleMap: (id: string) => resolveModuleMap(id), +})); +vi.mock("@/lib/dashboard/snapshot-read", () => ({ + readDashboardSnapshotCached: (u: unknown) => readDashboardSnapshotCached(u), +})); +vi.mock("@/lib/dashboard/snapshot-flag", () => ({ + isDashboardSnapshotEnabled: () => true, +})); +vi.mock("@/lib/dashboard/batch-chart-types", () => ({ + computeBatchWindow: () => ({ + from: "2026-07-01T00:00:00.000Z", + to: "2026-07-31T00:00:00.000Z", + }), + deriveBatchChartTypes: () => ["WEIGHT"], +})); +vi.mock("@/lib/dashboard-layout", () => ({ + resolveDashboardLayout: (layout: unknown) => layout, +})); +vi.mock("@/lib/daily/load-digest", () => ({ + loadDailyDigest: (u: unknown) => loadDailyDigest(u), +})); +vi.mock("@/lib/measurements/series-batch-read", () => ({ + readSeriesBatch: (...args: unknown[]) => readSeriesBatch(...args), +})); +vi.mock("@/lib/medications/list-read", () => ({ + readMedicationsListCached: (u: unknown) => readMedicationsListCached(u), +})); +vi.mock("../page-client", () => ({ default: () => null })); +vi.mock("../medications/page-client", () => ({ default: () => null })); + +import DashboardPage from "../page"; +import MedicationsPage from "../medications/page"; + +/** + * The account the RSC would read from. `DELEGATE_MARKER` rides every payload + * these mocks return, so any assertion that finds it in the cache has found + * the caller's own record dehydrated into somebody else's page. + */ +const DELEGATE_MARKER = "delegate-own-record"; + +const OWN_SESSION = { + user: { id: "u-delegate", timezone: "Europe/Berlin" }, + session: { id: "s1", actingAsUserId: null }, +}; + +/** The same caller, stamped onto the owner's record by the switch endpoint. */ +const SWITCHED_SESSION = { + user: { id: "u-delegate", timezone: "Europe/Berlin" }, + session: { id: "s1", actingAsUserId: "u-owner" }, +}; + +const SNAPSHOT = { + body: { + marker: DELEGATE_MARKER, + layout: ["weight"], + tiles: { summaries: { WEIGHT: { count: 3 } } }, + }, + locale: "en", +}; +const DIGEST = { marker: DELEGATE_MARKER, headline: "3 kg down" }; +const SERIES = { WEIGHT: [{ marker: DELEGATE_MARKER, value: 81.4 }] }; +const MEDICATIONS = [{ id: "m1", name: DELEGATE_MARKER }]; + +afterEach(() => { + vi.clearAllMocks(); + delete process.env.DASHBOARD_SSR_PREFETCH; +}); + +/** Every dehydrated query on a HydrationBoundary element, keyed by hash. */ +function seededCache(el: ReactElement): Record { + if (el.type !== HydrationBoundary) return {}; + const props = el.props as { + state?: { queries: { queryHash: string; state: { data: unknown } }[] }; + }; + const out: Record = {}; + for (const q of props.state?.queries ?? []) out[q.queryHash] = q.state.data; + return out; +} + +function primeDashboard() { + resolveModuleMap.mockResolvedValue({ insights: true }); + readDashboardSnapshotCached.mockResolvedValue(SNAPSHOT); + loadDailyDigest.mockResolvedValue(DIGEST); + readSeriesBatch.mockResolvedValue({ series: SERIES }); +} + +describe("the dashboard prefetch and the record on screen", () => { + it("seeds the caller's own record when no switch is active", async () => { + // The positive half, and it is not decoration: it fixes what the negative + // half is the absence of. Without it, a page that prefetched nothing at + // all for anybody would pass the switch test perfectly. + getSession.mockResolvedValue(OWN_SESSION); + primeDashboard(); + + const cache = seededCache((await DashboardPage()) as ReactElement); + + expect(cache[hashKey(queryKeys.dashboardSnapshot("en"))]).toMatchObject({ + marker: DELEGATE_MARKER, + }); + expect(cache[hashKey(queryKeys.dailyDigest())]).toMatchObject({ + marker: DELEGATE_MARKER, + }); + expect( + cache[ + hashKey( + queryKeys.chartSeriesBatch( + "WEIGHT", + "2026-07-01T00:00:00.000Z", + "2026-07-31T00:00:00.000Z", + ), + ) + ], + ).toMatchObject(SERIES); + }); + + it("seeds nothing while the session is acting on another record", async () => { + // The session row carries the owner's id, which is what a switch IS on the + // cookie transport. Asserted on the CACHE rather than on a call count: a + // future prefetch that reads through some other helper still has to put + // its result somewhere, and this is where it would show up. + getSession.mockResolvedValue(SWITCHED_SESSION); + primeDashboard(); + + const el = (await DashboardPage()) as ReactElement; + const cache = seededCache(el); + + // Asserted as a whole-object equality on purpose: the failure message + // then PRINTS the record that reached the cache, marker and all, instead + // of saying a boolean was wrong. + expect(cache).toEqual({}); + expect(el.type).not.toBe(HydrationBoundary); + expect(readDashboardSnapshotCached).not.toHaveBeenCalled(); + expect(loadDailyDigest).not.toHaveBeenCalled(); + expect(readSeriesBatch).not.toHaveBeenCalled(); + }); +}); + +describe("the medications prefetch and the record on screen", () => { + it("seeds the caller's own list when no switch is active", async () => { + getSession.mockResolvedValue(OWN_SESSION); + resolveModuleMap.mockResolvedValue({ medications: true }); + readMedicationsListCached.mockResolvedValue(MEDICATIONS); + + const cache = seededCache((await MedicationsPage()) as ReactElement); + expect(cache[hashKey(queryKeys.medications())]).toEqual(MEDICATIONS); + }); + + it("seeds nothing while the session is acting on another record", async () => { + // `/api/medications` IS delegable, so the client cell corrects itself on + // its `refetchOnMount: "always"` pass. What it cannot undo is the frame + // before that: somebody else's medicine cabinet, painted under a banner + // naming the owner. + getSession.mockResolvedValue(SWITCHED_SESSION); + resolveModuleMap.mockResolvedValue({ medications: true }); + readMedicationsListCached.mockResolvedValue(MEDICATIONS); + + const el = (await MedicationsPage()) as ReactElement; + const cache = seededCache(el); + + // Asserted as a whole-object equality on purpose: the failure message + // then PRINTS the record that reached the cache, marker and all, instead + // of saying a boolean was wrong. + expect(cache).toEqual({}); + expect(el.type).not.toBe(HydrationBoundary); + expect(readMedicationsListCached).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/coach/__tests__/page-prefetch.test.tsx b/src/app/coach/__tests__/page-prefetch.test.tsx index c12ee7115..3912f4f11 100644 --- a/src/app/coach/__tests__/page-prefetch.test.tsx +++ b/src/app/coach/__tests__/page-prefetch.test.tsx @@ -15,11 +15,13 @@ import { queryKeys } from "@/lib/query-keys"; * error path fails soft. The streaming conversation is never prefetched here. */ -const getSession = vi.fn(); +const getUnswitchedSession = vi.fn(); const requireAssistantSurface = vi.fn(); const readCoachNudgeStatus = vi.fn(); -vi.mock("@/lib/auth/session", () => ({ getSession: () => getSession() })); +vi.mock("@/lib/auth/acting-carrier", () => ({ + getUnswitchedSession: () => getUnswitchedSession(), +})); vi.mock("@/lib/feature-flags", () => ({ requireAssistantSurface: (s: string) => requireAssistantSurface(s), })); @@ -54,7 +56,9 @@ function dehydratedQuery( describe("/coach RSC prefetch", () => { it("dehydrates the nudge status under the EXACT client key", async () => { - getSession.mockResolvedValue({ user: { id: "u1", disableCoach: false } }); + getUnswitchedSession.mockResolvedValue({ + user: { id: "u1", disableCoach: false }, + }); requireAssistantSurface.mockResolvedValue({}); readCoachNudgeStatus.mockResolvedValue(NUDGE); @@ -66,7 +70,9 @@ describe("/coach RSC prefetch", () => { }); it("skips the prefetch when the user opted out of the Coach", async () => { - getSession.mockResolvedValue({ user: { id: "u1", disableCoach: true } }); + getUnswitchedSession.mockResolvedValue({ + user: { id: "u1", disableCoach: true }, + }); const el = (await CoachPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); expect(requireAssistantSurface).not.toHaveBeenCalled(); @@ -74,7 +80,9 @@ describe("/coach RSC prefetch", () => { }); it("fails soft when the operator Coach flag is off (surface throws)", async () => { - getSession.mockResolvedValue({ user: { id: "u1", disableCoach: false } }); + getUnswitchedSession.mockResolvedValue({ + user: { id: "u1", disableCoach: false }, + }); requireAssistantSurface.mockRejectedValue(new Error("assistant disabled")); const el = (await CoachPage()) as ReactElement; @@ -83,7 +91,9 @@ describe("/coach RSC prefetch", () => { }); it("fails soft when the read throws", async () => { - getSession.mockResolvedValue({ user: { id: "u1", disableCoach: false } }); + getUnswitchedSession.mockResolvedValue({ + user: { id: "u1", disableCoach: false }, + }); requireAssistantSurface.mockResolvedValue({}); readCoachNudgeStatus.mockRejectedValue(new Error("db blip")); @@ -91,8 +101,10 @@ describe("/coach RSC prefetch", () => { expect(el.type).not.toBe(HydrationBoundary); }); + // Null also means "acting on somebody else's record" — `getUnswitchedSession` + // collapses both into the same answer, and the page treats them the same. it("fails soft when there is no session", async () => { - getSession.mockResolvedValue(null); + getUnswitchedSession.mockResolvedValue(null); const el = (await CoachPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); }); @@ -101,6 +113,6 @@ describe("/coach RSC prefetch", () => { process.env.DASHBOARD_SSR_PREFETCH = "false"; const el = (await CoachPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); - expect(getSession).not.toHaveBeenCalled(); + expect(getUnswitchedSession).not.toHaveBeenCalled(); }); }); diff --git a/src/app/coach/page.tsx b/src/app/coach/page.tsx index 134c08b2e..5a1655500 100644 --- a/src/app/coach/page.tsx +++ b/src/app/coach/page.tsx @@ -4,7 +4,7 @@ import { QueryClient, } from "@tanstack/react-query"; -import { getSession } from "@/lib/auth/session"; +import { getUnswitchedSession } from "@/lib/auth/acting-carrier"; import { requireAssistantSurface } from "@/lib/feature-flags"; import { readCoachNudgeStatus } from "@/lib/ai/coach/nudge-status"; import { queryKeys } from "@/lib/query-keys"; @@ -44,6 +44,11 @@ import CoachPageClient from "./page-client"; * thread, snapshot, any provider-touching path) is NOT prefetched or warmed * — it depends on URL params + the nudge outcome and must never be triggered * server-side. + * - Record identity: the session comes from `getUnswitchedSession()`, which + * answers null while the browser is acting on somebody else's record. Every + * read below scopes to the CALLER, so under a switch the prefetch would + * serialise the delegate's own rows into a page opened on the owner's + * record. The accessor's docblock carries the argument. * - Fail-soft: no session, the Coach being off, or a DB blip renders the page * exactly as before this wrapper existed — the client cell owns the fetch. */ @@ -57,7 +62,7 @@ export default async function CoachPage() { let dehydratedState = null; try { - const session = await getSession(); + const session = await getUnswitchedSession(); if (session && !session.user.disableCoach) { // Throws `AssistantDisabledError` when the operator flag is off → caught // below, prefetch skipped, client path stands (and redirects to /insights). diff --git a/src/app/insights/__tests__/page-prefetch.test.tsx b/src/app/insights/__tests__/page-prefetch.test.tsx index 48c73e65b..38faac229 100644 --- a/src/app/insights/__tests__/page-prefetch.test.tsx +++ b/src/app/insights/__tests__/page-prefetch.test.tsx @@ -19,10 +19,12 @@ import { queryKeys } from "@/lib/query-keys"; * soft to the bare client. */ -const getSession = vi.fn(); +const getUnswitchedSession = vi.fn(); const readDashboardSnapshotCached = vi.fn(); -vi.mock("@/lib/auth/session", () => ({ getSession: () => getSession() })); +vi.mock("@/lib/auth/acting-carrier", () => ({ + getUnswitchedSession: () => getUnswitchedSession(), +})); vi.mock("@/lib/dashboard/snapshot-read", () => ({ readDashboardSnapshotCached: (u: unknown) => readDashboardSnapshotCached(u), })); @@ -49,7 +51,7 @@ function queriesOf( describe("/insights RSC prefetch", () => { it("dehydrates the snapshot under the locale-keyed client key", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); readDashboardSnapshotCached.mockResolvedValue({ body: { healthScore: { tension: "steady" }, @@ -71,7 +73,7 @@ describe("/insights RSC prefetch", () => { }); it("JSON-round-trips the snapshot body to the wire shape", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); const cachedAt = new Date("2026-07-18T06:00:00.000Z"); readDashboardSnapshotCached.mockResolvedValue({ body: { cachedAt, layout: { version: 1, widgets: [] } }, @@ -88,15 +90,17 @@ describe("/insights RSC prefetch", () => { }); it("fails soft to the bare client when the read throws", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); readDashboardSnapshotCached.mockRejectedValue(new Error("db blip")); const el = (await InsightsPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); }); + // Null also means "acting on somebody else's record" — `getUnswitchedSession` + // collapses both into the same answer, and the page treats them the same. it("fails soft when there is no session", async () => { - getSession.mockResolvedValue(null); + getUnswitchedSession.mockResolvedValue(null); const el = (await InsightsPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); expect(readDashboardSnapshotCached).not.toHaveBeenCalled(); @@ -106,6 +110,6 @@ describe("/insights RSC prefetch", () => { process.env.DASHBOARD_SSR_PREFETCH = "false"; const el = (await InsightsPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); - expect(getSession).not.toHaveBeenCalled(); + expect(getUnswitchedSession).not.toHaveBeenCalled(); }); }); diff --git a/src/app/insights/page.tsx b/src/app/insights/page.tsx index bb0ae6b11..880f4b2d7 100644 --- a/src/app/insights/page.tsx +++ b/src/app/insights/page.tsx @@ -4,7 +4,7 @@ import { QueryClient, } from "@tanstack/react-query"; -import { getSession } from "@/lib/auth/session"; +import { getUnswitchedSession } from "@/lib/auth/acting-carrier"; import { readDashboardSnapshotCached } from "@/lib/dashboard/snapshot-read"; import { queryKeys } from "@/lib/query-keys"; @@ -43,6 +43,11 @@ import InsightsPageClient from "./page-client"; * read; and the heavy thick-analytics / comprehensive aggregations, whose * COLD SWR cell is a multi-query scan — prefetching them synchronously would * trade FCP for TTFB. Both stay on the client cells that own them. + * - Record identity: the session comes from `getUnswitchedSession()`, which + * answers null while the browser is acting on somebody else's record. Every + * read below scopes to the CALLER, so under a switch the prefetch would + * serialise the delegate's own rows into a page opened on the owner's + * record. The accessor's docblock carries the argument. * - Fail-soft: no session or a builder hiccup renders the page exactly as * before this wrapper existed — the client cells own the fetch. */ @@ -56,7 +61,7 @@ export default async function InsightsPage() { let dehydratedState = null; try { - const session = await getSession(); + const session = await getUnswitchedSession(); if (session) { const { user } = session; const { body, locale } = await readDashboardSnapshotCached(user); diff --git a/src/app/insights/workouts/__tests__/page-prefetch.test.tsx b/src/app/insights/workouts/__tests__/page-prefetch.test.tsx index 13386612d..ef17a2ffa 100644 --- a/src/app/insights/workouts/__tests__/page-prefetch.test.tsx +++ b/src/app/insights/workouts/__tests__/page-prefetch.test.tsx @@ -19,11 +19,13 @@ import { queryKeys } from "@/lib/query-keys"; * client path (the client cell owns the fetch). */ -const getSession = vi.fn(); +const getUnswitchedSession = vi.fn(); const resolveModuleMap = vi.fn(); const readWorkoutsListCached = vi.fn(); -vi.mock("@/lib/auth/session", () => ({ getSession: () => getSession() })); +vi.mock("@/lib/auth/acting-carrier", () => ({ + getUnswitchedSession: () => getUnswitchedSession(), +})); vi.mock("@/lib/modules/gate", () => ({ resolveModuleMap: (id: string) => resolveModuleMap(id), })); @@ -74,7 +76,7 @@ function dehydratedQuery( describe("/insights/workouts RSC prefetch", () => { it("dehydrates the list under the EXACT client key (byte-identical hash)", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); resolveModuleMap.mockResolvedValue({ workouts: true }); readWorkoutsListCached.mockResolvedValue(LIST); @@ -88,7 +90,7 @@ describe("/insights/workouts RSC prefetch", () => { }); it("reads the same filter the client's `limit: 100` request would", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); resolveModuleMap.mockResolvedValue({ workouts: true }); readWorkoutsListCached.mockResolvedValue(LIST); @@ -105,7 +107,7 @@ describe("/insights/workouts RSC prefetch", () => { }); it("JSON-round-trips the value to the wire shape (Dates → ISO strings)", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); resolveModuleMap.mockResolvedValue({ workouts: true }); const startedAt = new Date("2026-07-18T08:30:00.000Z"); readWorkoutsListCached.mockResolvedValue({ @@ -129,7 +131,7 @@ describe("/insights/workouts RSC prefetch", () => { }); it("fails soft to the bare client when the read throws", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); resolveModuleMap.mockResolvedValue({ workouts: true }); readWorkoutsListCached.mockRejectedValue(new Error("db blip")); @@ -138,7 +140,7 @@ describe("/insights/workouts RSC prefetch", () => { }); it("skips the prefetch when the workouts module is off", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); resolveModuleMap.mockResolvedValue({ workouts: false }); const el = (await InsightsWorkoutsPage()) as ReactElement; @@ -146,8 +148,10 @@ describe("/insights/workouts RSC prefetch", () => { expect(readWorkoutsListCached).not.toHaveBeenCalled(); }); + // Null also means "acting on somebody else's record" — `getUnswitchedSession` + // collapses both into the same answer, and the page treats them the same. it("fails soft when there is no session", async () => { - getSession.mockResolvedValue(null); + getUnswitchedSession.mockResolvedValue(null); const el = (await InsightsWorkoutsPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); expect(readWorkoutsListCached).not.toHaveBeenCalled(); @@ -157,6 +161,6 @@ describe("/insights/workouts RSC prefetch", () => { process.env.DASHBOARD_SSR_PREFETCH = "false"; const el = (await InsightsWorkoutsPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); - expect(getSession).not.toHaveBeenCalled(); + expect(getUnswitchedSession).not.toHaveBeenCalled(); }); }); diff --git a/src/app/insights/workouts/page.tsx b/src/app/insights/workouts/page.tsx index 0a05a5f6c..5c085580c 100644 --- a/src/app/insights/workouts/page.tsx +++ b/src/app/insights/workouts/page.tsx @@ -4,7 +4,7 @@ import { QueryClient, } from "@tanstack/react-query"; -import { getSession } from "@/lib/auth/session"; +import { getUnswitchedSession } from "@/lib/auth/acting-carrier"; import { readWorkoutsListCached } from "@/lib/workouts/list-read"; import { resolveModuleMap } from "@/lib/modules/gate"; import { queryKeys } from "@/lib/query-keys"; @@ -42,6 +42,11 @@ import InsightsWorkoutsPageClient from "./page-client"; * fresh data, so the mounted cell paints immediately and only refetches once * the TTL lapses — the "empty then fills" flash is gone without dropping the * freshness path. + * - Record identity: the session comes from `getUnswitchedSession()`, which + * answers null while the browser is acting on somebody else's record. Every + * read below scopes to the CALLER, so under a switch the prefetch would + * serialise the delegate's own rows into a page opened on the owner's + * record. The accessor's docblock carries the argument. * - Fail-soft: no session, a module lookup hiccup, or a DB blip renders the * page exactly as before this wrapper existed — the client cell owns the * fetch. The prefetch is an accelerator, never a gate. @@ -57,7 +62,7 @@ export default async function InsightsWorkoutsPage() { let dehydratedState = null; try { - const session = await getSession(); + const session = await getUnswitchedSession(); if (session) { const { user } = session; const modules = await resolveModuleMap(user.id); diff --git a/src/app/medications/__tests__/page-prefetch.test.tsx b/src/app/medications/__tests__/page-prefetch.test.tsx index 2618b2176..846f1865e 100644 --- a/src/app/medications/__tests__/page-prefetch.test.tsx +++ b/src/app/medications/__tests__/page-prefetch.test.tsx @@ -16,11 +16,13 @@ import { queryKeys } from "@/lib/query-keys"; * bare client path (the client cell owns the fetch). */ -const getSession = vi.fn(); +const getUnswitchedSession = vi.fn(); const resolveModuleMap = vi.fn(); const readMedicationsListCached = vi.fn(); -vi.mock("@/lib/auth/session", () => ({ getSession: () => getSession() })); +vi.mock("@/lib/auth/acting-carrier", () => ({ + getUnswitchedSession: () => getUnswitchedSession(), +})); vi.mock("@/lib/modules/gate", () => ({ resolveModuleMap: (id: string) => resolveModuleMap(id), })); @@ -56,7 +58,7 @@ function dehydratedQuery( describe("/medications RSC prefetch", () => { it("dehydrates the list under the EXACT client key (byte-identical hash)", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); resolveModuleMap.mockResolvedValue({ medications: true }); readMedicationsListCached.mockResolvedValue([ { id: "m1", name: "Ramipril" }, @@ -72,7 +74,7 @@ describe("/medications RSC prefetch", () => { }); it("JSON-round-trips the value to the wire shape (Dates → ISO strings)", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); resolveModuleMap.mockResolvedValue({ medications: true }); const takenAt = new Date("2026-07-18T08:30:00.000Z"); readMedicationsListCached.mockResolvedValue([ @@ -89,7 +91,7 @@ describe("/medications RSC prefetch", () => { }); it("fails soft to the bare client when the read throws", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); resolveModuleMap.mockResolvedValue({ medications: true }); readMedicationsListCached.mockRejectedValue(new Error("db blip")); @@ -98,7 +100,7 @@ describe("/medications RSC prefetch", () => { }); it("skips the prefetch when the medications module is off", async () => { - getSession.mockResolvedValue(SESSION); + getUnswitchedSession.mockResolvedValue(SESSION); resolveModuleMap.mockResolvedValue({ medications: false }); const el = (await MedicationsPage()) as ReactElement; @@ -106,8 +108,10 @@ describe("/medications RSC prefetch", () => { expect(readMedicationsListCached).not.toHaveBeenCalled(); }); + // Null also means "acting on somebody else's record" — `getUnswitchedSession` + // collapses both into the same answer, and the page treats them the same. it("fails soft when there is no session", async () => { - getSession.mockResolvedValue(null); + getUnswitchedSession.mockResolvedValue(null); const el = (await MedicationsPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); expect(readMedicationsListCached).not.toHaveBeenCalled(); @@ -117,6 +121,6 @@ describe("/medications RSC prefetch", () => { process.env.DASHBOARD_SSR_PREFETCH = "false"; const el = (await MedicationsPage()) as ReactElement; expect(el.type).not.toBe(HydrationBoundary); - expect(getSession).not.toHaveBeenCalled(); + expect(getUnswitchedSession).not.toHaveBeenCalled(); }); }); diff --git a/src/app/medications/page.tsx b/src/app/medications/page.tsx index 44a96538b..201e4e239 100644 --- a/src/app/medications/page.tsx +++ b/src/app/medications/page.tsx @@ -4,7 +4,7 @@ import { QueryClient, } from "@tanstack/react-query"; -import { getSession } from "@/lib/auth/session"; +import { getUnswitchedSession } from "@/lib/auth/acting-carrier"; import { readMedicationsListCached } from "@/lib/medications/list-read"; import { resolveModuleMap } from "@/lib/modules/gate"; import { queryKeys } from "@/lib/query-keys"; @@ -40,9 +40,16 @@ import MedicationsPageClient from "./page-client"; * place the hydrated cards paint immediately and that refetch runs in the * background — the "empty then fills" flash is gone WITHOUT dropping the * freshness guarantee. - * - Fail-soft: no session, a module lookup hiccup, or a DB blip renders the - * page exactly as before this wrapper existed — the client cell owns the - * fetch. The prefetch is an accelerator, never a gate. + * - Record identity: the session comes from `getUnswitchedSession()`, which + * answers null while the browser is acting on somebody else's record. + * `/api/medications` IS delegable, so the client cell corrects itself on its + * `refetchOnMount: "always"` pass — but the frame before that correction + * painted the DELEGATE's medicine cabinet under a banner naming the owner, + * and a flash of the wrong person's medication is still the wrong person's + * medication. + * - Fail-soft: no session, an active switch, a module lookup hiccup, or a DB + * blip renders the page exactly as before this wrapper existed — the client + * cell owns the fetch. The prefetch is an accelerator, never a gate. */ export default async function MedicationsPage() { // Global SSR-prefetch kill-switch shared with the dashboard wrapper. The e2e @@ -54,7 +61,7 @@ export default async function MedicationsPage() { let dehydratedState = null; try { - const session = await getSession(); + const session = await getUnswitchedSession(); if (session) { const { user } = session; const modules = await resolveModuleMap(user.id); diff --git a/src/app/page.tsx b/src/app/page.tsx index 883dc9921..174360d7d 100644 --- a/src/app/page.tsx +++ b/src/app/page.tsx @@ -4,7 +4,7 @@ import { QueryClient, } from "@tanstack/react-query"; -import { getSession } from "@/lib/auth/session"; +import { getUnswitchedSession } from "@/lib/auth/acting-carrier"; import { readDashboardSnapshotCached } from "@/lib/dashboard/snapshot-read"; import { isDashboardSnapshotEnabled } from "@/lib/dashboard/snapshot-flag"; import { @@ -64,9 +64,18 @@ function softTimeout(ms: number): Promise { * key by construction, so the prefetched slice lands instead of refetching. * - Module-gate parity: the digest route gates on `insights`; skip its * prefetch when the module is off (the client hook is disabled then too). - * - Fail-soft: no session, a builder hiccup, or a DB blip renders the page - * exactly as before this wrapper existed — the client cells own the fetch. - * Each prefetch is independent; one failing never blocks the others. + * - Record identity: the session comes from `getUnswitchedSession()`, which + * answers null while the browser is acting on somebody else's record. Every + * read below scopes to `user`, and under a switch `user` is the DELEGATE — + * so the snapshot, the digest and the widget layout would seed one person's + * numbers under a banner naming another, on routes that then refuse the + * refetch that would have corrected them. The accessor's docblock carries + * the full argument, including why resolving the owner here instead is the + * worse answer. + * - Fail-soft: no session, an active switch, a builder hiccup, or a DB blip + * renders the page exactly as before this wrapper existed — the client cells + * own the fetch. Each prefetch is independent; one failing never blocks the + * others. */ export default async function DashboardPage() { // Operator/test escape hatch: `DASHBOARD_SSR_PREFETCH=false` renders the @@ -79,7 +88,7 @@ export default async function DashboardPage() { let dehydratedState = null; let batchWindow: BatchWindow | undefined; try { - const session = await getSession(); + const session = await getUnswitchedSession(); if (session) { const { user } = session; const { body, locale } = await readDashboardSnapshotCached(user); diff --git a/src/components/layout/__tests__/auth-shell-coach-hoist.test.ts b/src/components/layout/__tests__/auth-shell-coach-hoist.test.ts index 2ce3fbc61..07dfc2821 100644 --- a/src/components/layout/__tests__/auth-shell-coach-hoist.test.ts +++ b/src/components/layout/__tests__/auth-shell-coach-hoist.test.ts @@ -50,7 +50,14 @@ describe("v1.4.34 IW-B — CoachLaunchProvider hoist", () => { // `/api/insights/chat` is not in the proxy's DEMO_MUTATION_ALLOWLIST, // so a tappable FAB in demo mode produces a raw 403 on send. The mount // must be conditioned on the demo flag. - expect(source).toContain("{!demoMode && }"); + // + // v1.36.x — `!inSharedRecord` joins the same condition. What that gate is + // FOR, and the paint it produces at both grant levels, is asserted against + // rendered markup in `auth-shell-shared-record-doors.test.tsx`; this leg + // only holds the demo half that predates it. + expect(source).toContain( + "{!demoMode && !inSharedRecord && }", + ); }); it("no longer mounts any Coach surface in the routed insights layout", () => { diff --git a/src/components/layout/__tests__/auth-shell-shared-record-doors.test.tsx b/src/components/layout/__tests__/auth-shell-shared-record-doors.test.tsx new file mode 100644 index 000000000..f0e112eff --- /dev/null +++ b/src/components/layout/__tests__/auth-shell-shared-record-doors.test.tsx @@ -0,0 +1,198 @@ +/** + * The shell-level doors, under a switch. + * + * Three surfaces hang off `` rather than off a route, so they paint + * on every page the shell serves and on the unavailable panel too — the panel + * whose entire message is that a page is not part of what was shared. All + * three are owner-only: + * + * - the Coach FAB, whose drawer carries chat, conversation rename and + * delete, attachments, feedback, an adopt action and suggested actions, + * every one of them resolving `requireAuth()` and refusing under a switch — + * and which fires `POST /api/insights/coach/seen` on being opened at all; + * - the drawer mount behind it; + * - the module tour, which `POST`s `/api/onboarding/tour`. + * + * The nav dropped Insights and Coach under a switch on purpose. These were the + * doors left standing beside the removed entries. + * + * Rendered, not grepped. The shell is mounted through `renderToStaticMarkup` + * with the three doors replaced by sentinels, so what is asserted is the paint + * — which of them the shell put on the page — rather than the shape of a JSX + * condition. The chrome around them is stubbed for the same reason: this file + * is about the mount decision and nothing else. + * + * SSR-only, per project convention: `@testing-library/react` is not a + * dependency, effects do not run, and no click is available here. That is + * enough for this property, which is purely what renders. The click paths + * behind these doors are driven in `e2e/account-sharing.spec.ts`. + * + * Mutation checks, run: + * - drop `!inSharedRecord` from the FAB mount → "mounts none of them under a + * read grant" and its write sibling both go red, naming the FAB sentinel + * they found in the markup. + * - drop it from the tour mount, or from `` → the same + * two cases go red naming that sentinel instead. + * - make `resolveRecordCapabilities` return `inSharedRecord: false` always → + * all three go red at once, while "mounts all three in the caller's own + * record" stays green, which is how the pair distinguishes "gated" from + * "removed". + */ +import { describe, expect, it, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import { I18nProvider } from "@/lib/i18n/context"; +import type { + AccountAccess, + AccountAccessEntry, +} from "@/lib/sharing/account-access-view"; + +const mockAccessRef: { value: AccountAccess } = { + value: { accounts: [], active: null, canSwitch: false }, +}; + +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ + user: { + id: "delegate", + username: "delegate", + email: null, + role: "USER", + avatarUrl: null, + modules: {}, + accountAccess: mockAccessRef.value, + }, + isAuthenticated: true, + isAuthUnknown: false, + isLoading: false, + refetch: vi.fn(), + }), + clearCachesForSessionEnd: vi.fn(), +})); + +const mockPathRef = { value: "/" }; + +vi.mock("next/navigation", () => ({ + usePathname: () => mockPathRef.value, + useRouter: () => ({ replace: vi.fn(), push: vi.fn() }), +})); + +// The three doors under test, reduced to sentinels: this file asserts whether +// the shell mounted them, not what they would have rendered. +vi.mock("@/components/insights/layout-coach-fab", () => ({ + LayoutCoachFab: () =>
, +})); +vi.mock("@/components/insights/layout-coach-mount", () => ({ + LayoutCoachMount: () =>
, +})); +vi.mock("@/components/onboarding/tour-launcher", () => ({ + TourLauncher: () =>
, +})); + +// Chrome that is not the subject. Each pulls its own query cells and portals; +// rendering them here would measure the renderer, not the mount decision. +vi.mock("../sidebar-nav", () => ({ SidebarNav: () => null })); +vi.mock("../bottom-nav", () => ({ BottomNav: () => null })); +vi.mock("../top-bar", () => ({ TopBar: () => null })); +vi.mock("../offline-banner", () => ({ OfflineBanner: () => null })); +vi.mock("../demo-banner", () => ({ DemoBanner: () => null })); +vi.mock("../shared-record-banner", () => ({ SharedRecordBanner: () => null })); +vi.mock("../shared-record-unavailable", () => ({ + SharedRecordUnavailable: () =>
, +})); +vi.mock("@/components/i18n/maintainership-banner", () => ({ + MaintainershipBanner: () => null, +})); +vi.mock("@/components/gamification/achievement-unlock-notifier", () => ({ + AchievementUnlockNotifier: () => null, +})); + +import { AuthShell } from "../auth-shell"; + +const OWNER_READ: AccountAccessEntry = { + accountId: "acct-owner", + username: "grandma", + displayName: "Margarethe", + access: "read" as const, + canWrite: false, +}; +const OWNER_WRITE: AccountAccessEntry = { + ...OWNER_READ, + access: "write", + canWrite: true, +}; + +function render(access: AccountAccess, pathname = "/"): string { + mockAccessRef.value = access; + mockPathRef.value = pathname; + return renderToStaticMarkup( + + + +
+ + + , + ); +} + +const OWN_RECORD: AccountAccess = { + accounts: [], + active: null, + canSwitch: false, +}; +const SWITCHED = (entry: AccountAccessEntry): AccountAccess => ({ + accounts: [entry], + active: entry, + canSwitch: true, +}); + +const DOORS = [ + "sentinel-coach-fab", + "sentinel-coach-mount", + "sentinel-tour-launcher", +] as const; + +describe(" — the owner-only doors", () => { + it("mounts all three in the caller's own record", () => { + // The half that makes the rest mean something. Without it, a shell that + // had simply dropped the Coach and the tour for everybody would satisfy + // every assertion below. + const html = render(OWN_RECORD); + for (const slot of DOORS) { + expect(html, `${slot} must exist in one's own record`).toContain(slot); + } + }); + + it.each([ + ["read", OWNER_READ], + ["write", OWNER_WRITE], + ])("mounts none of them under a %s grant", (_level, entry) => { + // Both levels, because the grant level is not the question. A WRITE grant + // admits a short list of health-record creates and nothing about the + // account around the record — the Coach and the tour are outside it either + // way. + const html = render(SWITCHED(entry)); + for (const slot of DOORS) { + expect(html, `${slot} must not paint under a switch`).not.toContain(slot); + } + // And the page itself still renders: the doors went, the record did not. + expect(html).toContain("sentinel-page"); + }); + + it("mounts none of them over the unavailable panel either", () => { + // The panel says a page is not part of what was shared. A floating Coach + // launcher on top of that sentence is the product contradicting itself in + // one viewport. + const html = render(SWITCHED(OWNER_READ), "/settings"); + // First prove this render actually reached the panel. Without this line + // the case is indistinguishable from the previous one and would stay green + // if `outsideSharedRecord` stopped resolving at all. + expect(html).toContain("sentinel-unavailable"); + expect(html).not.toContain("sentinel-page"); + for (const slot of DOORS) { + expect(html).not.toContain(slot); + } + }); +}); diff --git a/src/components/layout/auth-shell.tsx b/src/components/layout/auth-shell.tsx index 01f10881a..a16a22a86 100644 --- a/src/components/layout/auth-shell.tsx +++ b/src/components/layout/auth-shell.tsx @@ -10,6 +10,7 @@ import { LayoutCoachFab } from "@/components/insights/layout-coach-fab"; import { LayoutCoachMount } from "@/components/insights/layout-coach-mount"; import { TourLauncher } from "@/components/onboarding/tour-launcher"; import { clearCachesForSessionEnd, useAuth } from "@/hooks/use-auth"; +import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import { useTranslations } from "@/lib/i18n/context"; import { CoachLaunchProvider } from "@/lib/insights/coach-launch-context"; import { isDestinationInSharedRecord } from "./nav-model"; @@ -72,14 +73,19 @@ export function AuthShell({ pathname.startsWith("/about") || pathname.startsWith("/c/"); const isAdminPage = pathname.startsWith("/admin"); + // v1.36.0 — is this browser inside somebody else's record. Read from the + // capability hook rather than off `accountAccess.active` directly: the shell + // decides what to mount and the surfaces beneath it decide what to offer, + // and the two disagreeing is the whole class of defect this hook exists to + // close. + const { inSharedRecord } = useRecordCapabilities(); // v1.36.0 — a page reached while acting on somebody else's record that // sharing does not cover. The nav already drops these entries, so arriving // here means a bookmark or a browser back button; the panel below explains // it instead of letting the page paint a row of 403 error cards. Paint only // — every route behind these pages refuses on its own. const outsideSharedRecord = - user?.accountAccess?.active != null && - !isDestinationInSharedRecord(pathname); + inSharedRecord && !isDestinationInSharedRecord(pathname); const isOnboardingPage = pathname === "/onboarding"; const showUnlockNotifier = isAuthenticated && !isPublicPage && !!user?.id; @@ -405,20 +411,39 @@ export function AuthShell({
- + {/* + The three mounts below hang off the shell rather than off a route, so + they paint on every page the shell serves — including the unavailable + panel, which exists to say a page is not part of what was shared. All + three are owner-only surfaces, and all three are gated on + `!inSharedRecord` for the same reason: Insights and the Coach were + deliberately kept out of what sharing covers (`nav-model.ts` — the + owner's consent for server-managed LLM egress is theirs, not + transferable), and the tour walks an account through modules a delegate + does not administer. Every route behind them resolves `requireAuth()` + and refuses under a switch, so leaving them mounted offered a delegate + a door onto seven modules that answer 403 — chat, conversation delete + and rename, attachments, feedback, adopt, suggested actions, and a + `POST /api/insights/coach/seen` fired by merely opening the drawer. + + Absent, not disabled: a greyed-out launcher would still say "this + exists and does not work for you", and what is true is that it is not + part of what this person was given. + */} + {!inSharedRecord && } {/* v1.18.6 — the module-tour launcher lives at the shell level so its overlay survives the cross-page `router.push`es the tour makes. It self-gates: it only auto-opens on the dashboard for a user who has not finished/dismissed the tour, and otherwise renders null. Stays out of demo mode (the tour PATCHes `/api/onboarding/tour`, which the proxy blocks for demo visitors). */} - {!demoMode && } + {!demoMode && !inSharedRecord && } {/* v1.16.13 — the Coach FAB stays hidden in demo mode. Its send hits `/api/insights/chat`, which is not in the proxy's `DEMO_MUTATION_ALLOWLIST`, so a demo visitor who tapped it and sent got a raw 403. Demo is read-only by design; the FAB has no job here, so it's mounted only outside demo mode. */} - {!demoMode && } + {!demoMode && !inSharedRecord && } ); } diff --git a/src/lib/auth/__tests__/unswitched-session.test.ts b/src/lib/auth/__tests__/unswitched-session.test.ts new file mode 100644 index 000000000..7ec309f41 --- /dev/null +++ b/src/lib/auth/__tests__/unswitched-session.test.ts @@ -0,0 +1,79 @@ +/** + * `getUnswitchedSession()` — the one gate every server-prefetching page sits + * behind. + * + * The five pages that prefetch are otherwise unrelated to each other, so the + * property they share has to live somewhere they all pass through, and this is + * it. The four cases below are the whole state space of the cookie transport: + * no session, a session in its own record, a session stamped onto somebody + * else's, and a session carrying a selector header it has no business sending. + * + * Mutation checks, run: + * - return `session` unconditionally → "refuses while the session is + * stamped onto another record" goes red ("expected { user: … } to be + * null"). + * - drop the `header` argument from the `decideActingCarrier` call → "refuses + * a session carrying a misplaced selector header" goes red the same way. + * - return `null` unconditionally → "hands back the session in the caller's + * own record" goes red, which is what stops the whole file from being + * satisfiable by a function that refuses everybody. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const getSession = vi.fn(); +const headerGet = vi.fn(); + +vi.mock("@/lib/auth/session", () => ({ getSession: () => getSession() })); +vi.mock("next/headers", () => ({ + headers: async () => ({ get: (name: string) => headerGet(name) }), +})); + +import { + ACCOUNT_SELECTOR_HEADER, + getUnswitchedSession, +} from "../acting-carrier"; + +const OWN = { + user: { id: "u-delegate" }, + session: { id: "s1", actingAsUserId: null }, +}; + +beforeEach(() => { + vi.clearAllMocks(); + headerGet.mockReturnValue(null); +}); + +describe("getUnswitchedSession", () => { + it("hands back the session in the caller's own record", async () => { + getSession.mockResolvedValue(OWN); + await expect(getUnswitchedSession()).resolves.toBe(OWN); + }); + + it("answers null when there is no session at all", async () => { + getSession.mockResolvedValue(null); + await expect(getUnswitchedSession()).resolves.toBeNull(); + }); + + it("refuses while the session is stamped onto another record", async () => { + // The stamp is what a switch IS on the cookie transport. A page that + // prefetched through this would serialise `u-delegate`'s health record + // into a document opened on `u-owner`'s. + getSession.mockResolvedValue({ + ...OWN, + session: { id: "s1", actingAsUserId: "u-owner" }, + }); + await expect(getUnswitchedSession()).resolves.toBeNull(); + }); + + it("refuses a session carrying a misplaced selector header", async () => { + // The header names no account on this transport, so it is not a switch and + // the request would be served as the caller. It is still a client saying + // it meant to be somewhere else, and a prefetch has nothing to gain by + // guessing which of the two it meant. + getSession.mockResolvedValue(OWN); + headerGet.mockImplementation((name: string) => + name === ACCOUNT_SELECTOR_HEADER ? "u-owner" : null, + ); + await expect(getUnswitchedSession()).resolves.toBeNull(); + }); +}); diff --git a/src/lib/auth/acting-carrier.ts b/src/lib/auth/acting-carrier.ts index 1d958b0b8..b7f25f52c 100644 --- a/src/lib/auth/acting-carrier.ts +++ b/src/lib/auth/acting-carrier.ts @@ -111,6 +111,60 @@ export function selectorNamesAnAccount(value: string): boolean { return value.length > 0 && value.length <= MAX_SELECTOR_LENGTH; } +/** + * The session a server-rendered PAGE may prefetch from, or null. + * + * A server-prefetching page dehydrates a health record into the HTML document + * and seeds it into the client's TanStack cache under the key the client cell + * will read. That copy is only ever right when the account the page read is + * the account the client's ROUTE would answer with, and under a switch it + * never is. `getSession()` answers who is CALLING, deliberately and + * permanently (`session.ts` — it is the same property that keeps `requireAdmin` + * out of a delegate's reach), so a page that prefetches off it seeds the + * DELEGATE's record onto a page whose banner names the owner. What happens + * next depends only on which mode the route declared, and both endings are + * wrong: + * + * - a non-delegable route refuses the client's refetch, so the delegate's + * own numbers stay on the owner's dashboard until the tab is closed; + * - a delegable route answers with the owner's rows, so the page shows two + * people at once for as long as the refetch takes. + * + * Resolving the acting account here instead — reading the record the way a + * route does — is the tempting other answer and is worse. The grant check + * lives in exactly one function (`requireRecordAuth`), the pages cannot reach + * it without an HTTP context, and re-deriving it here would be a second + * program deciding "whose record is this" — the thing `grants.ts` warns must + * have one answer. Worse, half of what these pages prefetch rides routes that + * declared themselves NOT delegable; prefetching the owner's copy would hand a + * delegate data the API is on record refusing them. + * + * So the prefetch is an accelerator that steps aside. Under a switch this + * returns null, the page renders its bare client shell, and every cell fetches + * through the routes — which is where the grant check is, and the only place + * it should ever have been. The cost is one skeleton frame on a delegated + * visit; nothing is lost that a fetch does not restore. + * + * Fail-closed on the ambiguous case too: a selector HEADER on this transport + * is `misplaced-header`, which names no account and is not a switch, but it is + * a client asserting it meant to be somewhere else. A page has nothing to gain + * by prefetching through that, so it does not. + */ +export async function getUnswitchedSession(): Promise +> | null> { + const session = await getSession(); + if (!session) return null; + const carrier = decideActingCarrier({ + // A page render is a document request: cookie transport by construction, + // and `getSession()` returning a session is what that means here. + transport: "cookie", + stamped: session.session.actingAsUserId ?? null, + header: await readSelectorHeader(), + }); + return carrier.kind === "none" ? session : null; +} + /** * The account this request CLAIMS, resolved without an auth context in hand. * From 24c45ad9e971c6536093558d74867179ba7fcdbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 3 Aug 2026 15:56:28 +0200 Subject: [PATCH 2/2] Return the sharing journey to the page Accept lives on The coach positive control navigates to the dashboard to prove the launcher renders for a delegate in their own record. It left the browser there, and the next step clicks Accept, which lives on the access page. The click waited out its timeout against a page that does not carry the button. An assertion inserted mid-journey has to restore the state it borrowed. --- e2e/account-sharing.spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/e2e/account-sharing.spec.ts b/e2e/account-sharing.spec.ts index 7efa94349..b7e0a7984 100644 --- a/e2e/account-sharing.spec.ts +++ b/e2e/account-sharing.spec.ts @@ -288,6 +288,11 @@ test.describe("account sharing", () => { timeout: 10_000, }); + // Back to where the rest of this journey happens. The check above leaves + // the browser on the dashboard, and Accept lives on the access page. + await page.goto("/settings/access"); + await expect(row).toBeVisible(); + await page.locator('[data-slot="grant-accept"]').click(); await expect(row).toHaveAttribute("data-grant-state", "ACTIVE"); });