fix(02-12): refuse malformed record access

- Preserve absent-server compatibility while rejecting published invalid blocks\n- Partition refused record reads and withhold protected presentation
This commit is contained in:
Marc-André Bombeck committed 2026-08-06 14:03:13 +02:00
1 parent b844d04676
commit 21e111c06c
11 files changed
+380 -18

No files matched your search

+109
View File
@@ -51,6 +51,45 @@ async function leaveRecord(page: Page) {
);
}
function withDivergentActiveAccountAccess(body: unknown): unknown {
if (body === null || typeof body !== "object") {
throw new Error("Expected an object from /api/auth/me");
}
const envelope = body as { data?: unknown };
const payload = "data" in envelope ? envelope.data : envelope;
if (payload === null || typeof payload !== "object") {
throw new Error("Expected an auth payload from /api/auth/me");
}
const auth = payload as {
accountAccess?: {
active?: {
access?: unknown;
level?: unknown;
canWrite?: unknown;
} | null;
};
};
if (!auth.accountAccess?.active) {
throw new Error("Expected a switched account-access payload");
}
const corrupted = {
...auth,
accountAccess: {
...auth.accountAccess,
active: {
...auth.accountAccess.active,
access: "write",
level: "write",
canWrite: true,
},
},
};
return "data" in envelope ? { ...envelope, data: corrupted } : corrupted;
}
test.describe.serial("scoped sharing browser journeys", () => {
test.use({ storageState: SCOPE_DELEGATE_STORAGE_STATE_PATH });
@@ -99,6 +138,76 @@ test.describe.serial("scoped sharing browser journeys", () => {
});
}
test("refuses a malformed switched payload before owner controls or reads mount", async ({
page,
}) => {
await page.goto("/");
await openSwitcher(page);
const entry = page.locator(
'[data-slot="account-switcher-entry"][data-account-username="e2e-scope-labs"]',
);
await expect(entry).toHaveCount(1);
let corruptNextAuthPayload = true;
await page.route("**/api/auth/me", async (route) => {
const response = await route.fetch();
if (!corruptNextAuthPayload) {
await route.fulfill({ response });
return;
}
corruptNextAuthPayload = false;
await route.fulfill({
response,
json: withDivergentActiveAccountAccess(await response.json()),
});
});
const ownerOnlyReads: string[] = [];
const trackOwnerOnlyRead = (request: {
method(): string;
url(): string;
}) => {
if (request.method() !== "GET") return;
const path = new URL(request.url()).pathname;
if (
[
"/api/labs/ocr/capability",
"/api/insights/coach/nudge-status",
"/api/coach/about-me/questions",
"/api/auth/me/notification-prefs",
].includes(path)
) {
ownerOnlyReads.push(path);
}
};
page.on("request", trackOwnerOnlyRead);
try {
await entry.click();
await expect(page).toHaveURL(/\/labs$/);
await expect(
page.locator('[data-slot="invalid-record-access-refusal"]'),
).toBeVisible();
await expect(
page.locator('[data-slot="shared-record-unavailable-leave"]'),
).toBeVisible();
await expect(page.locator('[data-tour-id="labs-hero"]')).toHaveCount(0);
await expect(
page.locator('[data-slot="shared-record-banner"]'),
).toHaveCount(0);
expect(ownerOnlyReads).toEqual([]);
await page
.locator('[data-slot="shared-record-unavailable-leave"]')
.click();
await expect(page).toHaveURL(/\/$/);
} finally {
page.off("request", trackOwnerOnlyRead);
await page.unroute("**/api/auth/me");
}
});
test("keeps an adult READ grant read-only and outside Settings", async ({
page,
}) => {
@@ -44,6 +44,12 @@ vi.mock("@/hooks/use-feature-flags", () => ({
vi.mock("@/hooks/use-disable-coach", () => ({
useDisableCoach: () => false,
}));
vi.mock("@/hooks/use-auth", () => ({
useAuth: () => ({ user: {}, isLoading: false }),
}));
vi.mock("@/hooks/use-record-capabilities", () => ({
useRecordCapabilities: () => ({ inSharedRecord: false }),
}));
vi.mock("@tanstack/react-query", () => ({
useQuery: () => ({ data: undefined }),
}));
+9 -3
View File
@@ -10,6 +10,8 @@ import type { CoachLaunchScope } from "@/lib/insights/coach-launch-context";
import { useCoachLaunch } from "@/lib/insights/coach-launch-context";
import { useFeatureFlags } from "@/hooks/use-feature-flags";
import { useDisableCoach } from "@/hooks/use-disable-coach";
import { useAuth } from "@/hooks/use-auth";
import { useRecordCapabilities } from "@/hooks/use-record-capabilities";
import { queryKeys } from "@/lib/query-keys";
import { apiGet } from "@/lib/api/api-fetch";
import { coachScopeSourceSchema } from "@/lib/ai/coach/types";
@@ -163,6 +165,8 @@ export default function CoachPageClient() {
const launch = useCoachLaunch();
const flags = useFeatureFlags();
const disableCoach = useDisableCoach();
const { user, isLoading } = useAuth();
const { inSharedRecord } = useRecordCapabilities();
const coachUnavailable = !flags.coach || disableCoach;
@@ -170,11 +174,13 @@ export default function CoachPageClient() {
// operator master flag OR per-user opt-out hides the Coach entirely.
// Send a direct navigator back to the Insights mother page so the
// route is never a dead-end.
const canRenderOwnCoach = user !== null && !isLoading && !inSharedRecord;
useEffect(() => {
if (coachUnavailable) {
if (canRenderOwnCoach && coachUnavailable) {
router.replace("/insights");
}
}, [coachUnavailable, router]);
}, [canRenderOwnCoach, coachUnavailable, router]);
// Reopening the bottom-right FAB drawer is no longer wired to a page
// control, but keep the launch context referenced so the lint rule does
@@ -182,7 +188,7 @@ export default function CoachPageClient() {
// the shared `<AuthShell>`.
void launch;
if (coachUnavailable) return null;
if (!canRenderOwnCoach || coachUnavailable) return null;
return (
<div
@@ -52,6 +52,9 @@ const mockAccessRef: { value: AccountAccess } = {
value: { accounts: [], active: null, canSwitch: false },
};
const mockAuthLoadingRef = { value: false };
const mockAccountAccessStatusRef = {
value: "valid" as "absent" | "valid" | "invalid",
};
vi.mock("@/hooks/use-auth", () => ({
useAuth: () => ({
@@ -63,6 +66,7 @@ vi.mock("@/hooks/use-auth", () => ({
avatarUrl: null,
modules: {},
accountAccess: mockAccessRef.value,
accountAccessStatus: mockAccountAccessStatusRef.value,
},
isAuthenticated: true,
isAuthUnknown: false,
@@ -138,9 +142,14 @@ const MANAGED_GUARDIAN: AccountAccessEntry = {
canWrite: true,
};
function render(access: AccountAccess, pathname = "/"): string {
function render(
access: AccountAccess,
pathname = "/",
accountAccessStatus: "absent" | "valid" | "invalid" = "valid",
): string {
mockAccessRef.value = access;
mockPathRef.value = pathname;
mockAccountAccessStatusRef.value = accountAccessStatus;
return renderToStaticMarkup(
<QueryClientProvider client={new QueryClient()}>
<I18nProvider initialLocale="en">
@@ -205,6 +214,18 @@ describe("<AuthShell> — the owner-only doors", () => {
}
});
it("refuses an invalid record block without mounting owner-only doors or children", () => {
const html = render(OWN_RECORD, "/", "invalid");
expect(html).toContain("sentinel-unavailable");
expect(html).not.toContain("sentinel-page");
for (const slot of DOORS) {
expect(html, `${slot} must not paint for a refused record`).not.toContain(
slot,
);
}
});
it.each([
["read", OWNER_READ],
["write", OWNER_WRITE],
+19 -1
View File
@@ -79,7 +79,8 @@ export function AuthShell({
// 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, recordKind, sections } = useRecordCapabilities();
const { inSharedRecord, recordKind, sections, accessRefused } =
useRecordCapabilities();
// A managed profile has a deliberately small Settings surface of its own.
// Let that route family reach its record-aware gate; adult shared records
// stay in the generic refusal branch even at MANAGE.
@@ -245,6 +246,23 @@ export function AuthShell({
);
}
// A present but malformed access block is not the older-server case. The
// server may still be switched, so mounting children here could fetch target
// data under owner controls. Offer only the actor-scoped way out.
if (accessRefused) {
return (
<div
data-slot="invalid-record-access-refusal"
className="flex min-h-dvh flex-col"
>
<MaintainershipBanner />
<main className="mx-auto w-full max-w-screen-xl px-4 py-10 md:px-6">
<SharedRecordUnavailable />
</main>
</div>
);
}
// Onboarding page — minimal shell, no sidebar/nav
if (isOnboardingPage) {
return (
@@ -0,0 +1,106 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import {
__resetRecordScopeForTests,
getRecordScope,
isReadingSharedRecord,
} from "@/lib/query-keys/record-scope";
const apiGet = vi.fn();
vi.mock("@/lib/api/api-fetch", () => ({
apiGet: (...args: unknown[]) => apiGet(...args),
apiFetchRaw: vi.fn(),
ApiError: class ApiError extends Error {},
}));
vi.mock("next/navigation", () => ({
useRouter: () => ({ push: vi.fn(), replace: vi.fn() }),
}));
import { fetchMe, NO_ACCOUNT_ACCESS } from "../use-auth";
const response = {
id: "delegate",
username: "delegate",
role: "USER",
timezone: "Europe/Berlin",
};
const canonicalEntry = {
accountId: "record-a",
username: "record-a",
displayName: "Record A",
access: "read",
level: "read",
recordKind: "shared",
sections: ["labs"],
canWrite: false,
};
beforeEach(() => {
apiGet.mockReset();
__resetRecordScopeForTests();
});
describe("fetchMe account-access resolution", () => {
it("keeps a genuinely absent block compatible with an older server", async () => {
apiGet.mockResolvedValue(response);
const user = await fetchMe();
expect(user.accountAccessStatus).toBe("absent");
expect(user.accountAccess).toEqual(NO_ACCOUNT_ACCESS);
expect(getRecordScope()).toBeNull();
expect(isReadingSharedRecord()).toBe(false);
});
it("uses a valid published block as the active record scope", async () => {
apiGet.mockResolvedValue({
...response,
accountAccess: {
accounts: [canonicalEntry],
active: canonicalEntry,
recordKind: "shared",
canSwitch: true,
},
});
const user = await fetchMe();
expect(user.accountAccessStatus).toBe("valid");
expect(user.accountAccess?.active).toEqual(canonicalEntry);
expect(getRecordScope()).toBe("record-a");
});
it.each([
["an explicitly undefined block", undefined],
["a null block", null],
[
"a divergent active entry",
{
accounts: [canonicalEntry],
active: {
...canonicalEntry,
access: "write",
level: "write",
canWrite: true,
},
recordKind: "shared",
canSwitch: true,
},
],
])(
"refuses %s without restoring an owner cache scope",
async (_name, block) => {
apiGet.mockResolvedValue({ ...response, accountAccess: block });
const user = await fetchMe();
expect(user.accountAccessStatus).toBe("invalid");
expect(user.accountAccess).toEqual(NO_ACCOUNT_ACCESS);
expect(getRecordScope()).not.toBeNull();
expect(isReadingSharedRecord()).toBe(true);
},
);
});
@@ -72,6 +72,19 @@ describe("resolveRecordCapabilities", () => {
);
});
it("refuses a malformed published block instead of treating it as own record", () => {
expect(resolveRecordCapabilities(null, true)).toEqual({
inSharedRecord: true,
canWrite: false,
canAdd: false,
canManage: false,
level: null,
sections: [],
recordKind: "shared",
accessRefused: true,
});
});
it("a read-only delegate adds nothing", () => {
expect(resolveRecordCapabilities(READ_ONLY)).toEqual({
inSharedRecord: true,
+41 -12
View File
@@ -23,7 +23,10 @@ import { storeTimezone } from "@/lib/timezone-mirror";
import type { ModuleKey } from "@/lib/modules/registry";
import type { TourProgress } from "@/lib/onboarding/tour-progress";
import { clearOfflineCachesForSessionEnd } from "@/lib/pwa/query-persister";
import { setRecordScope } from "@/lib/query-keys/record-scope";
import {
setRecordScope,
setRefusedRecordScope,
} from "@/lib/query-keys/record-scope";
import type { AccountAccess } from "@/lib/sharing/account-access-view";
import { parseAccountAccess } from "@/lib/sharing/account-access-schema";
@@ -39,6 +42,9 @@ export const NO_ACCOUNT_ACCESS: AccountAccess = {
canSwitch: false,
};
/** Whether the account-access block was absent, valid, or refused as malformed. */
export type AccountAccessStatus = "absent" | "valid" | "invalid";
/**
* v1.18.6 — resume point for the module tour as the client sees it on
* `/api/auth/me`. Mirrors the server-side `TourProgress` shape.
@@ -193,6 +199,13 @@ export interface AuthUser {
* the empty one — no accounts, no active record, no switcher.
*/
accountAccess?: AccountAccess;
/**
* The parser outcome for `accountAccess`. An absent block is the only
* owner-compatible fallback: it is how older server images answered before
* sharing existed. A present malformed block stays explicitly refused so a
* server-side switch cannot paint owner controls in the target record.
*/
accountAccessStatus?: AccountAccessStatus;
}
/**
@@ -253,7 +266,7 @@ export function isAuthVerdictUnknown(error: unknown): boolean {
return true;
}
async function fetchMe(): Promise<AuthUser> {
export async function fetchMe(): Promise<AuthUser> {
// v1.16.4 — routed through the typed wrapper; a non-OK /me (401)
// throws `ApiError`, which `useAuth` treats as "not authenticated"
// exactly like the old hand-rolled throw.
@@ -300,8 +313,13 @@ async function fetchMe(): Promise<AuthUser> {
// whatever the switch flow stamped before its reload, the block is what the
// resolver will actually honour on the next request, so the mirror tracks it
// rather than the client's memory of what it asked for.
const accountAccess = coerceAccountAccess(data.accountAccess);
setRecordScope(accountAccess.active?.accountId ?? null);
const { accountAccess, status: accountAccessStatus } =
resolveAccountAccess(data);
if (accountAccessStatus === "invalid") {
setRefusedRecordScope();
} else {
setRecordScope(accountAccess.active?.accountId ?? null);
}
return {
...(data as AuthUser),
disableCoach: data.disableCoach ?? false,
@@ -329,21 +347,32 @@ async function fetchMe(): Promise<AuthUser> {
? data.moduleAvailability
: {},
accountAccess,
accountAccessStatus,
};
}
/**
* Read the sharing block off a payload that may not carry one.
*
* Fails CLOSED, unlike the module maps above: an older server image (or a
* malformed block) resolves to no accounts, no active record, and no switcher.
* The module gates fail open because a missing map must never blank the nav;
* this one fails closed because the affordance it drives asks the server to
* open somebody else's health record, and an affordance offered on a server
* that cannot honour it is worse than one that is missing.
* A genuinely missing field is the pre-sharing server contract and remains an
* own-record fallback. Once a server publishes the field, every other value —
* including `undefined` as an own property and `null` — is a malformed block,
* not permission to fall back to the caller's own presentation.
*/
function coerceAccountAccess(value: unknown): AccountAccess {
return parseAccountAccess(value) ?? NO_ACCOUNT_ACCESS;
function resolveAccountAccess(value: object): {
accountAccess: AccountAccess;
status: AccountAccessStatus;
} {
if (!Object.hasOwn(value, "accountAccess")) {
return { accountAccess: NO_ACCOUNT_ACCESS, status: "absent" };
}
const accountAccess = parseAccountAccess(
(value as { accountAccess?: unknown }).accountAccess,
);
return accountAccess
? { accountAccess, status: "valid" }
: { accountAccess: NO_ACCOUNT_ACCESS, status: "invalid" };
}
export function useAuth() {
+19 -1
View File
@@ -71,6 +71,8 @@ import {
* search a button in gets a sentence.
*/
export interface RecordCapabilities {
/** A present account-access block was malformed and must not mount a record. */
accessRefused?: boolean;
/** Is this browser acting on somebody else's record right now. */
inSharedRecord: boolean;
/** The grant's resolved level for the record on screen. */
@@ -111,7 +113,20 @@ export interface RecordCapabilities {
*/
export function resolveRecordCapabilities(
active: AccountAccessEntry | null | undefined,
accessRefused = false,
): RecordCapabilities {
if (accessRefused) {
return {
accessRefused: true,
inSharedRecord: true,
canWrite: false,
canAdd: false,
canManage: false,
level: null,
sections: [],
recordKind: "shared",
};
}
if (!active) {
return {
inSharedRecord: false,
@@ -156,7 +171,10 @@ export function resolveRecordCapabilities(
*/
export function useRecordCapabilities(): RecordCapabilities {
const { user } = useAuth();
return resolveRecordCapabilities(user?.accountAccess?.active);
return resolveRecordCapabilities(
user?.accountAccess?.active,
user?.accountAccessStatus === "invalid",
);
}
/**
@@ -21,6 +21,7 @@ import {
getRecordScope,
isReadingSharedRecord,
recordScopedQueryKeyHashFn,
setRefusedRecordScope,
setRecordScope,
} from "@/lib/query-keys/record-scope";
@@ -101,6 +102,24 @@ describe("the cache cannot serve one record's data while reading another", () =>
expect(client.getQueryData(queryKeys.dashboardSnapshot())).toBeUndefined();
});
it("never puts data from a refused record into the caller's own cache slot", () => {
const client = appClient();
client.setQueryData(queryKeys.dashboardSnapshot(), { weightKg: 71 });
setRefusedRecordScope();
expect(isReadingSharedRecord()).toBe(true);
expect(client.getQueryData(queryKeys.dashboardSnapshot())).toBeUndefined();
});
it("keeps the auth identity reachable when a refused record changes scope", () => {
const ownAuthHash = recordScopedQueryKeyHashFn(queryKeys.authMe());
setRefusedRecordScope();
expect(recordScopedQueryKeyHashFn(queryKeys.authMe())).toBe(ownAuthHash);
});
it("keeps two records' entries apart at the same time", () => {
const client = appClient();
setRecordScope("acct-a");
+17
View File
@@ -52,6 +52,7 @@
import type { QueryKey } from "@tanstack/react-query";
const STORAGE_KEY = "healthlog-record-scope";
const REFUSED_RECORD_SCOPE = "__healthlog_refused_record__";
/**
* The in-process copy. Seeded from storage on first read so the hash function
@@ -113,6 +114,18 @@ export function setRecordScope(accountId: string | null): void {
}
}
/**
* Keep a malformed active-record response out of the caller's own cache slot.
*
* No record id from an invalid payload is trusted, but `null` would make
* record-scoped reads share the caller's own cache identity. This sentinel is
* a cache partition only; the shell refuses the presentation until the caller
* leaves the server-side switch.
*/
export function setRefusedRecordScope(): void {
setRecordScope(REFUSED_RECORD_SCOPE);
}
/** True while this browser is reading somebody else's record. */
export function isReadingSharedRecord(): boolean {
return getRecordScope() !== null;
@@ -138,6 +151,10 @@ export function recordScopedQueryKeyHashFn(queryKey: QueryKey): string {
}, {})
: val,
);
// `/api/auth/me` identifies the caller, never the record they are reading.
// Its query begins before `fetchMe` can resolve or refuse the record scope,
// so it must remain reachable across that transition.
if (queryKey[0] === "auth") return key;
const record = getRecordScope();
return record === null ? key : `record:${record}|${key}`;
}