From 80175f3bfca08154e13a0566629e565dd481de8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 2 Aug 2026 14:26:10 +0200 Subject: [PATCH] feat(sharing): key the query cache by the record it holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every cached read in this app is about one health record, and nothing in the key factory said which one — there was only ever the signed-in account's. Once a browser can act on somebody else's, `["dashboard","snapshot"]` names two different answers and the cache cannot tell them apart. That is this repo's known cache-poison class, on health data, across two people. One line at the root rather than a dimension threaded through twenty-three key modules: a `queryKeyHashFn` that folds the record id into the cache identity of every entry. Key arrays are untouched, which is the part that matters — prefix invalidation and the import-time dependent-key bundles both match on the array and keep working. Prefixing the arrays would have broken every bundle silently. The hash is defence in depth. Switching is a full page reload, so the in-memory cache is empty on the other side of it; the layers that survive a reload are the IndexedDB snapshot and the service worker's data cache, and those are handled by refusing to write either while a switch is on, and by skipping the restore. Nothing of the owner's record ever reaches the disk of a delegate's device, so a stale scope marker can cost an empty restore and never another person's data. The persister's per-account binding read the account payload's id, which under a switch is the delegate in both states — it could not tell the two apart. It now asks the record scope, which can. --- src/lib/pwa/query-client-options.ts | 10 ++ src/lib/pwa/query-persister.ts | 37 +++++++ src/lib/query-keys/index.ts | 18 ++++ src/lib/query-keys/record-scope.ts | 154 ++++++++++++++++++++++++++++ src/lib/query-keys/sharing.ts | 37 +++++++ 5 files changed, 256 insertions(+) create mode 100644 src/lib/query-keys/record-scope.ts create mode 100644 src/lib/query-keys/sharing.ts diff --git a/src/lib/pwa/query-client-options.ts b/src/lib/pwa/query-client-options.ts index 4f36b77dd..1d944428f 100644 --- a/src/lib/pwa/query-client-options.ts +++ b/src/lib/pwa/query-client-options.ts @@ -1,5 +1,7 @@ import type { DefaultOptions, QueryClient } from "@tanstack/react-query"; +import { recordScopedQueryKeyHashFn } from "@/lib/query-keys/record-scope"; + export const MEANINGFUL_HIDDEN_INTERVAL_MS = 60_000; interface VisibilityTarget { @@ -57,6 +59,13 @@ export function subscribeToMeaningfulVisibilityRefresh( * regardless, the offline `fetch` rejects immediately, and `onError` fires — an * honest failure the form surfaces (and the global `OfflineMutationToaster` * backstops), instead of a spinner-of-death plus silent write loss. + * + * v1.36.0 — `queryKeyHashFn` folds the record being read into the cache + * identity of every entry in the app. Without it `["dashboard","snapshot"]` + * names two different people's data and the cache cannot tell them apart. See + * `src/lib/query-keys/record-scope.ts` for why this sits on the hash rather + * than on the key arrays (the arrays feed prefix invalidation and the + * import-time dependent-key bundles, both of which would break). */ export const QUERY_CLIENT_DEFAULT_OPTIONS: DefaultOptions = { queries: { @@ -64,6 +73,7 @@ export const QUERY_CLIENT_DEFAULT_OPTIONS: DefaultOptions = { gcTime: 10 * 60 * 1000, refetchOnWindowFocus: false, retry: 1, + queryKeyHashFn: recordScopedQueryKeyHashFn, }, mutations: { networkMode: "always", diff --git a/src/lib/pwa/query-persister.ts b/src/lib/pwa/query-persister.ts index 805b44f04..7fbd5cdd3 100644 --- a/src/lib/pwa/query-persister.ts +++ b/src/lib/pwa/query-persister.ts @@ -33,6 +33,7 @@ import { } from "@tanstack/react-query"; import { queryKeys } from "@/lib/query-keys"; +import { isReadingSharedRecord } from "@/lib/query-keys/record-scope"; const DB_NAME = "healthlog-query-cache"; const STORE = "kv"; @@ -173,6 +174,12 @@ export async function restorePersistedQueryCache( buildVersion: string, ): Promise { if (typeof indexedDB === "undefined") return; + // v1.36.0 — a browser inside somebody else's record restores nothing. The + // store holds the caller's OWN dashboard (it is the only thing that was ever + // written to it, see the flush below), and painting that under a banner + // saying "viewing her record" is the mis-context failure this feature is + // built to prevent, in the one place a full reload cannot clear. + if (isReadingSharedRecord()) return; try { const payload = await idbGet(); if (!payload) return; @@ -245,6 +252,13 @@ export function startPersistingQueryCache( let timer: ReturnType | undefined; const flush = () => { + // v1.36.0 — never write another person's record to this device. The + // allowlist below decides WHICH families are low-sensitivity enough to + // persist; that judgement was made about the account's own data and does + // not carry over to a record they were merely given to read. Refusing at + // the flush (rather than filtering keys) also means the last good snapshot + // of the caller's own dashboard survives a switch untouched. + if (isReadingSharedRecord()) return; const state = dehydrate(queryClient, { shouldDehydrateQuery: (query) => query.state.status === "success" && isPersistableKey(query.queryKey), @@ -338,3 +352,26 @@ export async function clearOfflineCachesForSessionEnd(): Promise { } } } + +/** + * v1.36.0 — the same wipe, before a record switch in either direction. + * + * Deliberately the same bytes as the session-end wipe rather than a narrower + * one, and deliberately named for the act that calls it. + * + * The full page reload the switch performs clears the in-memory cache; these + * two layers are the ones that survive it. The IndexedDB snapshot holds the + * dashboard the browser was last looking at, and the service worker's + * `healthlog-data-*` cache answers `/api/…` reads from disk on a + * stale-while-revalidate branch — so without this call, the first paint after + * switching into a record can be the previous record's numbers, served by the + * worker, under a banner naming the new one. Nothing about the page's own code + * would be wrong; the answer would just come from somewhere the page does not + * control. + * + * Run it BEFORE the reload and await it. A wipe racing the navigation is a + * wipe that sometimes does not happen, and "sometimes" is the whole failure. + */ +export async function clearOfflineCachesForRecordSwitch(): Promise { + await clearOfflineCachesForSessionEnd(); +} diff --git a/src/lib/query-keys/index.ts b/src/lib/query-keys/index.ts index 572f3fee6..0085b1415 100644 --- a/src/lib/query-keys/index.ts +++ b/src/lib/query-keys/index.ts @@ -36,6 +36,7 @@ import { mentalHealthKeys } from "./mental-health"; import { moodKeys } from "./mood"; import { nutrientKeys } from "./nutrients"; import { settingsKeys } from "./settings"; +import { sharingKeys } from "./sharing"; import { workoutKeys } from "./workouts"; export const queryKeys = { @@ -62,8 +63,25 @@ export const queryKeys = { ...documentKeys, ...customMetrics, ...nutrientKeys, + ...sharingKeys, }; +/** + * v1.36.0 — every read a grant transition can make stale. + * + * Inviting, accepting, revoking and renouncing all change the same three + * answers: the grant list, the owner's record-activity feed, and + * `accountAccess` on the account payload — which is what drives the switcher + * and the banner. A revoke that refreshed the panel but not the payload would + * leave a switcher entry pointing at a record the server had just closed: the + * affordance still on screen, the click already refused. + */ +export const grantDependentKeys = [ + queryKeys.accountGrants(), + queryKeys.accountActivity(), + queryKeys.authMe(), +]; + /** * Keys that should be invalidated when a measurement is created, updated or * deleted. Kept here so dashboards, insights, and targets always stay in sync. diff --git a/src/lib/query-keys/record-scope.ts b/src/lib/query-keys/record-scope.ts new file mode 100644 index 000000000..a3bb61d86 --- /dev/null +++ b/src/lib/query-keys/record-scope.ts @@ -0,0 +1,154 @@ +/** + * v1.36.0 — the account dimension of the query cache. + * + * Every cached read in this app is about one health record. Nothing in the key + * factory says which one, because until now there was only ever one: the + * signed-in account's. Once a browser can act on somebody else's record, a key + * like `["dashboard","snapshot"]` names two different answers, and a cache that + * cannot tell them apart will hand the second reader the first one's data. + * That is this repository's known cache-poison class, on health data, across + * two people. + * + * The fix is one line at the root rather than a dimension threaded through + * twenty-three key modules: a `queryKeyHashFn` on the QueryClient. The hash is + * what the cache stores entries under and what `getQueryData` looks them up by, + * so folding the record id into it partitions every entry in the app at once. + * Key ARRAYS are untouched, which matters more than it looks: prefix + * invalidation (`invalidateQueries({ queryKey: ["measurements"] })`) and the + * module-scope dependent-key bundles in `./index.ts` both match on the array, + * were computed at import time before any record was known, and keep working + * unchanged. Prefixing the arrays instead would have silently broken every one + * of those bundles. + * + * ## Why this is defence in depth and not the primary control + * + * Switching records is a full page reload (design §2), so the in-memory cache + * is empty on the other side of it and this hash can only matter if that ever + * stops being true. The primary control for the layers that DO survive a + * reload — the IndexedDB snapshot and the service worker's data cache — is the + * wipe the switch performs before reloading, in `switchRecord()`. Both are + * cheap; only one of them is the reason the feature is safe. + * + * ## Why the mirror is in localStorage + * + * The scope has to be known synchronously at boot, before `/api/auth/me` + * answers, because the persister decides whether to restore from disk in the + * first effect after mount. A switch is a property of the SESSION — it is + * carried on the session row, not per tab — so every tab of the browser is + * switched at once, which is what localStorage models and sessionStorage does + * not. + * + * A mirror can go stale, and the honest question is what a stale one costs. + * Nothing, because the disk layers are wiped on every switch in both + * directions: a mirror claiming a record the session has left points the + * persister at a store that was emptied when it left, and a mirror claiming + * "own" while the session is inside a record points it at a store that was + * emptied on the way in and never written to since (the persister refuses to + * write while switched — see `isPersistableKey`'s caller). Either way the + * worst case is an empty restore and a skeleton, never another person's data. + * `fetchMe` re-stamps the mirror from the server's resolved answer on every + * boot, so the staleness window is one round trip. + */ +import type { QueryKey } from "@tanstack/react-query"; + +const STORAGE_KEY = "healthlog-record-scope"; + +/** + * The in-process copy. Seeded from storage on first read so the hash function + * — which runs on every single query — never touches localStorage. + */ +let scope: string | null = null; +let seeded = false; + +function readMirror(): string | null { + try { + const raw = localStorage.getItem(STORAGE_KEY); + return raw === null || raw.length === 0 ? null : raw; + } catch { + // Private mode, disabled storage, or SSR. "Own record" is the safe + // reading: it is what every browser reported before this feature existed. + return null; + } +} + +/** + * The record this browser is currently reading, or null when it is reading its + * own. + * + * Server-side this is always null. `Providers` renders on the server during + * SSR, where there is no storage to read and no cache to partition. + */ +export function getRecordScope(): string | null { + if (typeof window === "undefined") return null; + if (!seeded) { + scope = readMirror(); + seeded = true; + } + return scope; +} + +/** + * Point the cache at a record, or (with `null`) back at the caller's own. + * + * Called from exactly two places: the switch flow, before it reloads the page, + * and `fetchMe`, which corrects the mirror from the server's resolved + * `accountAccess.active` on every account read. The second is what keeps a + * mirror written by a switch that the server has since ended from surviving + * past one round trip. + */ +export function setRecordScope(accountId: string | null): void { + scope = accountId; + seeded = true; + if (typeof window === "undefined") return; + try { + if (accountId === null) { + localStorage.removeItem(STORAGE_KEY); + } else { + localStorage.setItem(STORAGE_KEY, accountId); + } + } catch { + /* storage unavailable — the in-process copy still partitions this tab */ + } +} + +/** True while this browser is reading somebody else's record. */ +export function isReadingSharedRecord(): boolean { + return getRecordScope() !== null; +} + +/** + * The QueryClient's key hash function. + * + * Identical to TanStack's own `hashKey` when the browser is in its own record + * — a `JSON.stringify` with sorted object keys — so nothing about the app's + * cache changes for the overwhelming majority of sessions. Inside a shared + * record every entry lands under a distinct hash, which is what makes an + * inherited entry unreachable rather than merely stale. + */ +export function recordScopedQueryKeyHashFn(queryKey: QueryKey): string { + const key = JSON.stringify(queryKey, (_, val) => + isPlainObject(val) + ? Object.keys(val) + .sort() + .reduce>((result, k) => { + result[k] = (val as Record)[k]; + return result; + }, {}) + : val, + ); + const record = getRecordScope(); + return record === null ? key : `record:${record}|${key}`; +} + +/** TanStack's own predicate, copied rather than imported — it is not exported. */ +function isPlainObject(value: unknown): value is Record { + if (Object.prototype.toString.call(value) !== "[object Object]") return false; + const prototype = Object.getPrototypeOf(value); + return prototype === null || prototype === Object.prototype; +} + +/** @internal — lets a test start from a known state. */ +export const __resetRecordScopeForTests = (): void => { + scope = null; + seeded = false; +}; diff --git a/src/lib/query-keys/sharing.ts b/src/lib/query-keys/sharing.ts new file mode 100644 index 000000000..7673b585d --- /dev/null +++ b/src/lib/query-keys/sharing.ts @@ -0,0 +1,37 @@ +/** + * Query keys — account sharing (v1.36.0). + * Part of the centralized factory; aggregated in `./index.ts`. + * + * Note what these keys deliberately do NOT carry: the acting account. That + * dimension lives on the QueryClient's `queryKeyHashFn` + * (`./record-scope.ts`), which folds it into every entry in the app at once + * — including these. Adding it here as well would partition the same cache + * twice under two rules, which is the two-deciders shape in miniature. + * + * The reads below are all non-delegable by construction: `/api/account/grants` + * and the record-activity feed both resolve through a bare `requireAuth()` and + * refuse while a switch is on. They are still record-scoped like everything + * else, so a session that switches out and back never reads a grant list + * cached under the wrong scope. + */ +export const sharingKeys = { + /** + * Grants in both directions (`GET /api/account/grants`). One key for one + * endpoint: the route answers "who can see my record, and whose can I see" + * in a single payload precisely so no client renders half of it. + */ + accountGrants: () => ["account", "grants"] as const, + + /** + * The owner's record-activity feed (`GET /api/account/activity`) — who + * opened their record, and when. + */ + accountActivity: () => ["account", "activity"] as const, + + /** Mutation keys — kept in the factory so no bare array reaches a call site. */ + accountGrantInvite: () => ["account", "grants", "invite"] as const, + accountGrantAccept: () => ["account", "grants", "accept"] as const, + accountGrantRevoke: () => ["account", "grants", "revoke"] as const, + accountGrantRenounce: () => ["account", "grants", "renounce"] as const, + accountSwitchMutation: () => ["account", "switch"] as const, +};