From 60f91d27d706580846a4014e9ad73e40b7388fa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 6 Aug 2026 04:52:46 +0200 Subject: [PATCH] fix: validate account access payloads - Share the closed access schema with OpenAPI\n- Fail closed on malformed or stale record metadata --- src/hooks/use-auth.ts | 16 +--- src/lib/openapi/routes/account-sharing.ts | 72 ++--------------- src/lib/sharing/account-access-schema.ts | 94 +++++++++++++++++++++++ src/lib/sharing/account-access-view.ts | 3 +- 4 files changed, 104 insertions(+), 81 deletions(-) create mode 100644 src/lib/sharing/account-access-schema.ts diff --git a/src/hooks/use-auth.ts b/src/hooks/use-auth.ts index 53cd921f7..18899c26c 100644 --- a/src/hooks/use-auth.ts +++ b/src/hooks/use-auth.ts @@ -25,6 +25,7 @@ import type { TourProgress } from "@/lib/onboarding/tour-progress"; import { clearOfflineCachesForSessionEnd } from "@/lib/pwa/query-persister"; import { setRecordScope } from "@/lib/query-keys/record-scope"; import type { AccountAccess } from "@/lib/sharing/account-access-view"; +import { parseAccountAccess } from "@/lib/sharing/account-access-schema"; /** * v1.36.0 — an account with no sharing at all. The shape `fetchMe` falls back @@ -342,20 +343,7 @@ async function fetchMe(): Promise { * that cannot honour it is worse than one that is missing. */ function coerceAccountAccess(value: unknown): AccountAccess { - if (value === null || typeof value !== "object") return NO_ACCOUNT_ACCESS; - const block = value as Partial; - if (!Array.isArray(block.accounts)) return NO_ACCOUNT_ACCESS; - return { - accounts: block.accounts, - active: block.active ?? null, - recordKind: - block.recordKind === "self" || - block.recordKind === "shared" || - block.recordKind === "managed" - ? block.recordKind - : "self", - canSwitch: block.canSwitch === true, - }; + return parseAccountAccess(value) ?? NO_ACCOUNT_ACCESS; } export function useAuth() { diff --git a/src/lib/openapi/routes/account-sharing.ts b/src/lib/openapi/routes/account-sharing.ts index ecba5c64d..49511044f 100644 --- a/src/lib/openapi/routes/account-sharing.ts +++ b/src/lib/openapi/routes/account-sharing.ts @@ -23,6 +23,7 @@ import type { ZodOpenApiObject } from "zod-openapi"; import { z } from "zod/v4"; +import { accountAccessBlockSchema } from "@/lib/sharing/account-access-schema"; import { SHARE_DOMAINS } from "@/lib/sharing/scope"; import { inviteGrantSchema, @@ -210,72 +211,11 @@ export const accountSelectorParameter: NonNullable< // ── What the client is told it may do ──────────────────────────────────────── -const accountAccessEntry = z - .object({ - accountId: z - .string() - .describe( - "The account whose record it is — the value POST /api/account/switch takes, and the value the X-HealthLog-Account header carries.", - ), - username: z.string(), - displayName: z.string().nullable(), - access: z - .enum(["read", "write"]) - .describe( - "The legacy grant level that shipped clients decode. MANAGE serializes as `write` here; new code reads canonical `level`.", - ), - level: z - .enum(["read", "write", "manage"]) - .describe( - "The grant's resolved level. `manage` additionally admits changing and removing entries on the delegable surfaces; it never reaches the identity surfaces (settings, connections, tokens, consent, grant management), which stay refused on an invited record at every level. Render it; never derive it from `canWrite`.", - ), - recordKind: z - .enum(["self", "shared", "managed"]) - .describe( - "Server-resolved presentation metadata for the record. It is never accepted as an authorization input.", - ), - sections: z - .array(shareSection) - .nullable() - .describe( - "The sections of that record this caller may open, or null for the entire record. Null is first-class and is what every pre-v1.37.0 grant carries; a MANAGE grant always carries it too. A non-null array names the open sections and NOTHING else answers — a client that switches into a scoped record without honouring this gets the standard 403 on every out-of-scope route, fail-closed and byte-identical to every other sharing refusal, which is safe but reads as a wall of errors. An empty array means the grant opens nothing. Ordered by the consent screen's reading order.", - ), - canWrite: z - .boolean() - .describe( - "Whether this caller may ADD to that record: true for an accepted WRITE or MANAGE grant, false for a READ one. On its own it never means edit or delete — a WRITE grant adds and does nothing more. What MANAGE additionally admits is said by `level`. Resolved server-side. Render it; never derive it.", - ), - }) - .meta({ - id: "AccountAccessEntry", - description: - "One account this caller may act on. No avatar is published: the avatar bytes are owner-scoped, so a URL here would resolve to a refusal. Clients paint their initials fallback.", - }); - -export const accountAccessBlock = z - .object({ - accounts: z - .array(accountAccessEntry) - .describe("Every record this caller may open, newest grant first."), - active: accountAccessEntry - .nullable() - .describe( - "The record this session is inside right now, resolved to a full entry rather than an id to look up — so a banner can name the person without joining two fields. Null when the caller is in their own record, and ALWAYS null on the Bearer transport: a token carries its selector per request and this endpoint refuses one.", - ), - recordKind: z - .enum(["self", "shared", "managed"]) - .describe( - "The server-resolved kind of the record currently in view. `self` applies when `active` is null.", - ), - canSwitch: z - .boolean() - .describe("Whether there is anywhere to switch to. Bind it directly."), - }) - .meta({ - id: "AccountAccess", - description: - "Account sharing, resolved. This block is the ONLY source of switchability and writability: the server publishes what the caller may do and the client renders it. A client that computed either from grant data would be a second program deciding one person's access to a health record, and the two answer differently the first time an expiry or a revocation appears. Always present — an account nobody has shared with gets an empty list, not a missing field.", - }); +export const accountAccessBlock = accountAccessBlockSchema.meta({ + id: "AccountAccess", + description: + "Account sharing, resolved. This block is the ONLY source of switchability and writability: the server publishes what the caller may do and the client renders it. A client that computed either from grant data would be a second program deciding one person's access to a health record, and the two answer differently the first time an expiry or a revocation appears. Always present — an account nobody has shared with gets an empty list, not a missing field.", +}); /** * The account payload, documented for the one field this release adds. diff --git a/src/lib/sharing/account-access-schema.ts b/src/lib/sharing/account-access-schema.ts new file mode 100644 index 000000000..2d87bc1f0 --- /dev/null +++ b/src/lib/sharing/account-access-schema.ts @@ -0,0 +1,94 @@ +import { z } from "zod/v4"; + +import type { AccountAccess } from "./account-access-view"; +import { SHARE_DOMAINS } from "./scope"; + +const accountAccessLevelSchema = z.enum(["read", "write", "manage"]); +const accountRecordKindSchema = z.enum(["shared", "managed"]); +const shareSectionsSchema = z.array(z.enum(SHARE_DOMAINS)).nullable(); + +/** The browser's resolved account-access payload, shared with OpenAPI. */ +export const accountAccessEntrySchema = z + .object({ + accountId: z.string().min(1), + username: z.string().min(1), + displayName: z.string().nullable(), + access: z.enum(["read", "write"]), + level: accountAccessLevelSchema, + recordKind: accountRecordKindSchema, + sections: shareSectionsSchema, + canWrite: z.boolean(), + }) + .superRefine((entry, ctx) => { + const uniqueSections = new Set(entry.sections ?? []); + if (uniqueSections.size !== (entry.sections?.length ?? 0)) { + ctx.addIssue({ + code: "custom", + message: "Account access sections must not repeat a domain", + path: ["sections"], + }); + } + + const isManage = entry.level === "manage"; + if (isManage && (entry.access !== "write" || entry.sections !== null)) { + ctx.addIssue({ + code: "custom", + message: "Manage access must use legacy write access and whole scope", + }); + } + + if (entry.canWrite !== (entry.level !== "read")) { + ctx.addIssue({ + code: "custom", + message: "Account access write flag disagrees with its level", + path: ["canWrite"], + }); + } + + if (!isManage && entry.access !== entry.level) { + ctx.addIssue({ + code: "custom", + message: "Legacy access must agree with the resolved level", + path: ["access"], + }); + } + }); + +export const accountAccessBlockSchema = z.object({ + accounts: z.array(accountAccessEntrySchema), + active: accountAccessEntrySchema.nullable(), + recordKind: z.enum(["self", "shared", "managed"]), + canSwitch: z.boolean(), +}); + +/** Parse the server-resolved access block as a closed presentation contract. */ +export function parseAccountAccess(value: unknown): AccountAccess | null { + const parsed = accountAccessBlockSchema.safeParse(value); + if (!parsed.success) return null; + + const { accounts, active, recordKind, canSwitch } = parsed.data; + const accountIds = new Set(accounts.map((entry) => entry.accountId)); + if ( + accountIds.size !== accounts.length || + canSwitch !== accounts.length > 0 + ) { + return null; + } + + if (active === null) { + return recordKind === "self" + ? { accounts, active: null, recordKind, canSwitch } + : null; + } + + if (!accountIds.has(active.accountId) || recordKind !== active.recordKind) { + return null; + } + + const canonicalActive = accounts.find( + (entry) => entry.accountId === active.accountId, + ); + if (!canonicalActive) return null; + + return { accounts, active: canonicalActive, recordKind, canSwitch }; +} diff --git a/src/lib/sharing/account-access-view.ts b/src/lib/sharing/account-access-view.ts index d2caada5a..39b020583 100644 --- a/src/lib/sharing/account-access-view.ts +++ b/src/lib/sharing/account-access-view.ts @@ -28,6 +28,7 @@ export type LegacyAccountAccessLevel = "read" | "write"; /** The server-resolved kind of record a presentation is naming. */ export type AccountRecordKind = "self" | "shared" | "managed"; +export type SharedAccountRecordKind = Exclude; /** * Read the portion of an account-access entry that an older whole-record @@ -68,7 +69,7 @@ export interface AccountAccessEntry { * What kind of record this is. This is presentation metadata only; it is * resolved by the server and never accepted as an authorization input. */ - recordKind: AccountRecordKind; + recordKind: SharedAccountRecordKind; /** * v1.37.0 — which sections of the record this grant opens, or `null` for * the entire record.