mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(02-12.1): reach fenced document routes through the app transport
A browser-issued <img src>, <iframe src> or <a href download> cannot carry the record-session assertion, so the fence refused all three on any session that had been inside a shared record — permanently, since none of them will ever send a header. - add a structural guard: no client file may name a record-fenced route in a src=, href= or window.location sink - fetch the thumbnail, the inline preview and the download through the app transport and hand the DOM an object URL, revoked on unmount - make the two API probes in the managed-profiles browser spec assert the same context the app beside them asserts
This commit is contained in:
1 parent
8fcbdcc0df
commit
ffe38c9bb6
7 files changed
+739
-27
No files matched your search
@@ -18,6 +18,43 @@ const DOMAIN_READS = {
|
||||
documents: "/api/documents/inbound",
|
||||
} as const;
|
||||
|
||||
/**
|
||||
* v1.37.0 — the record-session assertion the app is currently sending.
|
||||
*
|
||||
* The two API probes in this file are deliberate: they check a status code the
|
||||
* UI does not surface (a MANAGE-only generated read) and a payload the UI
|
||||
* paginates. But a bare `fetch` inside `page.evaluate` carries no
|
||||
* record-session headers, and the fence refuses an unasserted request on any
|
||||
* session that has been inside a shared record — so the probes started failing
|
||||
* with 403 for a reason that had nothing to do with what they test.
|
||||
*
|
||||
* They are NOT an old-bundle simulation and must not become one: that case is
|
||||
* covered deliberately in `src/__tests__/record-session-fence-compat.test.ts`
|
||||
* and `tests/integration/record-session-fence.test.ts`, where the 403 and the
|
||||
* recovery are the subject. Here the subject is the grant level, so the probe
|
||||
* asserts exactly what the app beside it asserts — copied off a real app
|
||||
* request rather than computed, so it cannot drift from what the client
|
||||
* actually believes.
|
||||
*/
|
||||
function trackFenceAssertion(page: Page): () => {
|
||||
"x-healthlog-record-epoch": string;
|
||||
"x-healthlog-record-scope": string;
|
||||
} {
|
||||
let latest = { epoch: "bootstrap", scope: "bootstrap" };
|
||||
page.on("request", (request) => {
|
||||
const headers = request.headers();
|
||||
const epoch = headers["x-healthlog-record-epoch"];
|
||||
const scope = headers["x-healthlog-record-scope"];
|
||||
if (epoch === undefined || scope === undefined) return;
|
||||
if (epoch === "bootstrap") return;
|
||||
latest = { epoch, scope };
|
||||
});
|
||||
return () => ({
|
||||
"x-healthlog-record-epoch": latest.epoch,
|
||||
"x-healthlog-record-scope": latest.scope,
|
||||
});
|
||||
}
|
||||
|
||||
async function openSwitcher(page: Page) {
|
||||
await page.getByRole("button", { name: "User menu" }).first().click();
|
||||
await page.locator('[data-slot="account-switcher-trigger"]').click();
|
||||
@@ -294,6 +331,7 @@ test.describe.serial("scoped sharing browser journeys", () => {
|
||||
test("lets an adult WRITE grant add only to the selected record", async ({
|
||||
page,
|
||||
}) => {
|
||||
const fenceHeaders = trackFenceAssertion(page);
|
||||
const record = E2E_LEVEL_RECORDS.find(
|
||||
(candidate) => candidate.access === "WRITE",
|
||||
);
|
||||
@@ -324,10 +362,10 @@ test.describe.serial("scoped sharing browser journeys", () => {
|
||||
expect((await post).status()).toBeLessThan(300);
|
||||
|
||||
await page.reload();
|
||||
const payload = await page.evaluate(async () => {
|
||||
const response = await fetch("/api/measurements");
|
||||
const payload = await page.evaluate(async (headers) => {
|
||||
const response = await fetch("/api/measurements", { headers });
|
||||
return { status: response.status, body: await response.json() };
|
||||
});
|
||||
}, fenceHeaders());
|
||||
expect(payload.status).toBeLessThan(300);
|
||||
expect(JSON.stringify(payload.body)).toContain("124");
|
||||
await expect(banner).toHaveAttribute("data-account-id", accountId);
|
||||
@@ -342,6 +380,7 @@ test.describe.serial("scoped sharing browser journeys", () => {
|
||||
test("opens MANAGE-only generated reads without opening adult Settings", async ({
|
||||
page,
|
||||
}) => {
|
||||
const fenceHeaders = trackFenceAssertion(page);
|
||||
const record = E2E_LEVEL_RECORDS.find(
|
||||
(candidate) =>
|
||||
candidate.access === "MANAGE" && candidate.recordKind === "shared",
|
||||
@@ -357,10 +396,10 @@ test.describe.serial("scoped sharing browser journeys", () => {
|
||||
|
||||
await page.goto("/");
|
||||
await expect(banner).toHaveAttribute("data-access-level", "manage");
|
||||
const generatedRead = await page.evaluate(async () => {
|
||||
const response = await fetch("/api/dashboard/summary");
|
||||
const generatedRead = await page.evaluate(async (headers) => {
|
||||
const response = await fetch("/api/dashboard/summary", { headers });
|
||||
return response.status;
|
||||
});
|
||||
}, fenceHeaders());
|
||||
expect(generatedRead).toBeLessThan(300);
|
||||
await expect(
|
||||
page.locator('[data-slot="shared-record-unavailable"]'),
|
||||
|
||||
@@ -0,0 +1,279 @@
|
||||
/**
|
||||
* A fenced route may not be reached by a transport that cannot carry a header.
|
||||
*
|
||||
* The record-session fence is asserted on request HEADERS, attached inside
|
||||
* `src/lib/api/api-fetch.ts`. Three browser transports never go through that
|
||||
* wrapper and cannot be made to:
|
||||
*
|
||||
* * `<img src="/api/…">` — the browser issues the subresource itself;
|
||||
* * `<iframe src="/api/…">` — likewise, and it is a document load;
|
||||
* * `<a href="/api/…" download>` — a navigation, not a fetch.
|
||||
*
|
||||
* On a session that has never entered a shared record this is invisible: the
|
||||
* fence exempts `record_epoch = 0`, so a headerless request is served exactly
|
||||
* as it always was. The moment the session enters or leaves a shared record the
|
||||
* epoch moves, the exemption ends, and every one of those three gets
|
||||
* `403 sharing.access.denied` — permanently, because nothing about them will
|
||||
* ever start sending a header. A thumbnail that silently stops rendering after
|
||||
* somebody visits a shared record is the shape of the bug.
|
||||
*
|
||||
* The fix shape already existed in the repository before this guard did:
|
||||
* `src/components/documents/document-ai-transport.ts` reaches
|
||||
* `/api/documents/inbound/{id}/original` through `apiFetchRaw` and turns the
|
||||
* blob into a `File`. Everything here follows it — fetch through the app
|
||||
* transport, hand the DOM an object URL, revoke it on unmount.
|
||||
*
|
||||
* ## What this matcher can and cannot see
|
||||
*
|
||||
* It resolves ONE level of local aliasing: `const src = \`/api/…\`` followed by
|
||||
* `src={src}` is caught, because that is the shape the real offenders had and a
|
||||
* matcher that only read inline literals would have found two of the six and
|
||||
* reported success. It does NOT trace a path built by a helper, assembled
|
||||
* across two hops, or returned from a hook — a future offender in that shape
|
||||
* slips this guard. Said here rather than discovered later.
|
||||
*
|
||||
* It also only knows the routes it can RESOLVE. A path whose route module it
|
||||
* cannot find is reported separately rather than silently passing, because an
|
||||
* unresolvable path is the one case where a green result would mean nothing.
|
||||
*/
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { existsSync, readFileSync, readdirSync, statSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
const SRC = join(process.cwd(), "src");
|
||||
const API_ROOT = join(SRC, "app", "api");
|
||||
|
||||
/** Client-side trees. `src/app/api` is the server end and is excluded. */
|
||||
const CLIENT_ROOTS = [join(SRC, "components"), join(SRC, "app")];
|
||||
|
||||
const SKIP_DIRS = new Set([
|
||||
"__tests__",
|
||||
"__mocks__",
|
||||
"node_modules",
|
||||
".next",
|
||||
"generated",
|
||||
]);
|
||||
|
||||
function walk(dir: string, out: string[] = []): string[] {
|
||||
for (const name of readdirSync(dir)) {
|
||||
if (SKIP_DIRS.has(name)) continue;
|
||||
const p = join(dir, name);
|
||||
if (statSync(p).isDirectory()) {
|
||||
if (p === API_ROOT) continue;
|
||||
walk(p, out);
|
||||
} else if (
|
||||
(name.endsWith(".ts") || name.endsWith(".tsx")) &&
|
||||
!name.endsWith(".test.ts") &&
|
||||
!name.endsWith(".test.tsx")
|
||||
) {
|
||||
out.push(p);
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve an `/api/...` path template to its route module.
|
||||
*
|
||||
* `${…}` segments match any `[param]` directory, which is how
|
||||
* `/api/documents/inbound/${id}/thumbnail` finds
|
||||
* `src/app/api/documents/inbound/[id]/thumbnail/route.ts`. Query strings and
|
||||
* trailing fragments are dropped before matching.
|
||||
*/
|
||||
function resolveRouteModules(path: string): string[] {
|
||||
const clean = path.split(/[?#]/)[0];
|
||||
const segments = clean
|
||||
.replace(/^\/api\//, "")
|
||||
.split("/")
|
||||
.filter((s) => s.length > 0);
|
||||
|
||||
let dirs = [API_ROOT];
|
||||
for (const segment of segments) {
|
||||
const templated = segment.includes("${");
|
||||
const next: string[] = [];
|
||||
for (const dir of dirs) {
|
||||
if (!existsSync(dir)) continue;
|
||||
if (!templated && existsSync(join(dir, segment))) {
|
||||
next.push(join(dir, segment));
|
||||
continue;
|
||||
}
|
||||
// A templated segment is a WILDCARD over every child directory, literal
|
||||
// and dynamic alike. `${id}` finds `[id]`; `${provider}` finds
|
||||
// `withings`, `fitbit`, `whoop`, `google-health`. Taking only the
|
||||
// dynamic children would have silently dropped the second shape, and a
|
||||
// path that resolves to nothing reads as "no fenced route here".
|
||||
if (!templated) continue;
|
||||
for (const child of readdirSync(dir)) {
|
||||
const p = join(dir, child);
|
||||
if (statSync(p).isDirectory()) next.push(p);
|
||||
}
|
||||
}
|
||||
dirs = next;
|
||||
}
|
||||
return dirs
|
||||
.map((dir) => join(dir, "route.ts"))
|
||||
.filter((module) => existsSync(module));
|
||||
}
|
||||
|
||||
/** Does this route resolve a RECORD, and therefore consult the fence? */
|
||||
function isFenced(routeModule: string): boolean {
|
||||
const src = readFileSync(routeModule, "utf8");
|
||||
return (
|
||||
/\brequireRecordAuth\s*\(/.test(src) ||
|
||||
/\brequireGuardianAuth\s*\(/.test(src)
|
||||
);
|
||||
}
|
||||
|
||||
interface Reference {
|
||||
file: string;
|
||||
line: number;
|
||||
sink: string;
|
||||
path: string;
|
||||
}
|
||||
|
||||
const API_LITERAL = /^[`"']((?:\/api\/)[^`"']*)[`"']$/;
|
||||
|
||||
/**
|
||||
* Every `/api/...` path handed to a headerless transport in one file.
|
||||
*
|
||||
* Sinks: `src=`, `href=`, and any `window.location` write. Each is resolved
|
||||
* against a one-level alias map built from the file's own
|
||||
* `const NAME = "/api/…"` declarations.
|
||||
*/
|
||||
/**
|
||||
* Comments are stripped before matching.
|
||||
*
|
||||
* A doc comment that NAMES `src="/api/…"` while explaining why the code no
|
||||
* longer does that is prose, not a transport — and this guard's own
|
||||
* explanations are written in exactly that shape. Commented-out code is
|
||||
* dropped with them, which is correct: it issues no request. Same treatment
|
||||
* and the same reason as `account-payload-consumer-guard.test.ts`.
|
||||
*/
|
||||
function stripComments(text: string): string {
|
||||
return text.replace(/\/\*[\s\S]*?\*\//g, "").replace(/(^|\s)\/\/.*$/gm, "$1");
|
||||
}
|
||||
|
||||
function headerlessReferences(file: string, source: string): Reference[] {
|
||||
const aliases = new Map<string, string>();
|
||||
// `const NAME = "/api/…"`, and the ternary form
|
||||
// `const NAME = cond ? \`/api/…\` : "#"` that the download links use. Both
|
||||
// are one hop, which is the documented limit of this resolution.
|
||||
for (const m of source.matchAll(
|
||||
/\b(?:const|let)\s+([A-Za-z_$][\w$]*)\s*(?::[^=\n]+)?=\s*[^;\n]*?([`"'])(\/api\/[^`"']*)\2/g,
|
||||
)) {
|
||||
if (!aliases.has(m[1])) aliases.set(m[1], m[3]);
|
||||
}
|
||||
|
||||
const found: Reference[] = [];
|
||||
const record = (index: number, sink: string, raw: string) => {
|
||||
const trimmed = raw.trim();
|
||||
const literal = API_LITERAL.exec(trimmed);
|
||||
const path = literal ? literal[1] : (aliases.get(trimmed) ?? null);
|
||||
if (path === null) return;
|
||||
found.push({
|
||||
file,
|
||||
line: source.slice(0, index).split("\n").length,
|
||||
sink,
|
||||
path,
|
||||
});
|
||||
};
|
||||
|
||||
// `src={expr}` / `src="literal"` and the same for `href`.
|
||||
//
|
||||
// The expression container is read with a brace COUNTER, not a
|
||||
// `[^}]*` class: a template literal like `` `/api/x/${id}/y` `` contains a
|
||||
// `}` of its own, and a non-counting matcher truncates at it and then fails
|
||||
// to recognise the literal at all. That is how the first draft of this guard
|
||||
// found two of six offenders and reported success.
|
||||
for (const m of source.matchAll(/\b(src|href)=/g)) {
|
||||
const start = m.index + m[0].length;
|
||||
const ch = source[start];
|
||||
if (ch === "{") {
|
||||
let depth = 0;
|
||||
let end = start;
|
||||
for (; end < source.length; end++) {
|
||||
if (source[end] === "{") depth++;
|
||||
else if (source[end] === "}") {
|
||||
depth -= 1;
|
||||
if (depth === 0) break;
|
||||
}
|
||||
}
|
||||
record(m.index, m[1], source.slice(start + 1, end));
|
||||
} else if (ch === '"' || ch === "'" || ch === "`") {
|
||||
const end = source.indexOf(ch, start + 1);
|
||||
if (end !== -1) record(m.index, m[1], source.slice(start, end + 1));
|
||||
}
|
||||
}
|
||||
// Any `window.location` write or navigation.
|
||||
for (const m of source.matchAll(
|
||||
/window\.location(?:\.href)?\s*=\s*([^;\n]+)|window\.location\.(?:assign|replace)\(\s*([^)\n]+)\)/g,
|
||||
)) {
|
||||
record(m.index, "window.location", m[1] ?? m[2] ?? "");
|
||||
}
|
||||
return found;
|
||||
}
|
||||
|
||||
describe("no headerless browser transport reaches a fenced route", () => {
|
||||
const files = CLIENT_ROOTS.flatMap((root) => walk(root));
|
||||
const references = files.flatMap((file) =>
|
||||
headerlessReferences(
|
||||
file.slice(SRC.length + 1),
|
||||
stripComments(readFileSync(file, "utf8")),
|
||||
),
|
||||
);
|
||||
|
||||
it("finds a plausible client tree and a plausible set of references", () => {
|
||||
// Non-zero proof, twice: a walk that found nothing, or a matcher that
|
||||
// matched nothing, would make every assertion below vacuous.
|
||||
expect(files.length).toBeGreaterThan(200);
|
||||
expect(references.length).toBeGreaterThan(3);
|
||||
});
|
||||
|
||||
it("resolves every referenced /api path to a route module", () => {
|
||||
// An unresolvable path is the one case where a green result would mean
|
||||
// nothing at all, so it fails loudly rather than being skipped.
|
||||
const unresolved = references.filter(
|
||||
(r) => resolveRouteModules(r.path).length === 0,
|
||||
);
|
||||
expect(unresolved.map((r) => `${r.file}:${r.line} → ${r.path}`)).toEqual(
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
it("resolves a known fenced route, so the fenced check is live", () => {
|
||||
// Without this the leg below would pass for an `isFenced` that always
|
||||
// returned false.
|
||||
const thumbnail = resolveRouteModules(
|
||||
"/api/documents/inbound/${id}/thumbnail",
|
||||
);
|
||||
expect(thumbnail).toHaveLength(1);
|
||||
expect(isFenced(thumbnail[0])).toBe(true);
|
||||
const connect = resolveRouteModules("/api/withings/connect");
|
||||
expect(connect).toHaveLength(1);
|
||||
expect(isFenced(connect[0])).toBe(false);
|
||||
// The wildcard shape resolves to several real routes, none of them fenced.
|
||||
const providers = resolveRouteModules("/api/${provider}/connect");
|
||||
expect(providers.length).toBeGreaterThan(2);
|
||||
expect(providers.some(isFenced)).toBe(false);
|
||||
});
|
||||
|
||||
it("names no fenced route in a src=, href= or window.location sink", () => {
|
||||
const offenders = references.filter((r) =>
|
||||
resolveRouteModules(r.path).some(isFenced),
|
||||
);
|
||||
|
||||
if (offenders.length > 0) {
|
||||
const report = offenders
|
||||
.map((o) => ` ❌ ${o.file}:${o.line} — ${o.sink}= ${o.path}`)
|
||||
.join("\n");
|
||||
throw new Error(
|
||||
`${offenders.length} headerless reference(s) to a record-fenced route:\n${report}\n\n` +
|
||||
"These cannot carry the record-session fence headers, so they are refused with " +
|
||||
"403 sharing.access.denied on any session that has entered or left a shared " +
|
||||
"record. Fetch through `apiFetchRaw` and hand the DOM an object URL — see " +
|
||||
"`src/components/documents/document-ai-transport.ts` for the shape, and revoke " +
|
||||
"the URL when the component unmounts.",
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -118,7 +118,20 @@ describe("<DocumentCard>", () => {
|
||||
expect(html).toContain("befund.docx");
|
||||
});
|
||||
|
||||
it("renders the preview thumbnail image when hasThumbnail is set", () => {
|
||||
it("never emits a browser-issued /api subresource for the thumbnail", () => {
|
||||
// v1.37.0 — this used to assert `src="/api/documents/inbound/…/thumbnail"`
|
||||
// in the markup, and that contract is deliberately gone. A `<img src>` is
|
||||
// issued by the BROWSER, so it cannot carry the record-session assertion,
|
||||
// and the fence refuses an unasserted request on any session that has been
|
||||
// inside a shared record — a thumbnail that silently stops rendering for
|
||||
// good. The bytes now come through the app transport as an object URL
|
||||
// (`useFencedObjectUrl`), which means nothing paints on a server render.
|
||||
//
|
||||
// The assertion is inverted rather than deleted: the markup must contain no
|
||||
// `/api/` reference at all. `src/__tests__/record-fence-headerless-transport-guard.test.ts`
|
||||
// holds the same property across the whole client tree, and
|
||||
// `src/hooks/__tests__/use-fenced-object-url.test.ts` proves the request
|
||||
// really does carry the headers.
|
||||
const html = render(
|
||||
<DocumentCard
|
||||
document={doc({ hasThumbnail: true })}
|
||||
@@ -128,9 +141,11 @@ describe("<DocumentCard>", () => {
|
||||
highlighted={false}
|
||||
/>,
|
||||
);
|
||||
expect(html).toContain('data-slot="document-thumbnail"');
|
||||
expect(html).toContain('src="/api/documents/inbound/doc-1/thumbnail"');
|
||||
expect(html).toContain('loading="lazy"');
|
||||
expect(html).not.toContain("/api/documents");
|
||||
// Until the blob lands the card shows its kind icon, exactly as a card
|
||||
// with no thumbnail does — never a broken image.
|
||||
expect(html).not.toContain('data-slot="document-thumbnail"');
|
||||
expect(html).toContain("lucide-scan-line");
|
||||
});
|
||||
|
||||
it("shows the kind icon (no thumbnail image) when hasThumbnail is false", () => {
|
||||
|
||||
@@ -23,6 +23,7 @@ import { Badge } from "@/components/ui/badge";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Card, CardContent } from "@/components/ui/card";
|
||||
import { Checkbox } from "@/components/ui/checkbox";
|
||||
import { useFencedObjectUrl } from "@/hooks/use-fenced-object-url";
|
||||
import { useFormatters, useTranslations } from "@/lib/i18n/context";
|
||||
import { cn } from "@/lib/utils";
|
||||
import type { InboundDocumentDto } from "@/lib/validations/inbound-documents";
|
||||
@@ -75,6 +76,16 @@ export function DocumentCard({
|
||||
// A preview thumbnail that fails to load (still rendering, decrypt error,
|
||||
// 404) falls back to the kind icon — never a broken image.
|
||||
const [thumbFailed, setThumbFailed] = useState(false);
|
||||
// v1.37.0 — fetched through the app transport rather than issued by the
|
||||
// browser as a subresource, so it carries the record-session assertion. The
|
||||
// null path means "this card has no thumbnail, or one already failed", which
|
||||
// is what keeps the timeline from firing a request per card for documents
|
||||
// that have nothing to show.
|
||||
const { url: thumbnailUrl, failed: thumbnailFailed } = useFencedObjectUrl(
|
||||
document.hasThumbnail && !thumbFailed
|
||||
? `/api/documents/inbound/${document.id}/thumbnail`
|
||||
: null,
|
||||
);
|
||||
|
||||
// Touch long-press selects instead of opening; the click that the
|
||||
// browser fires after the release is swallowed once.
|
||||
@@ -128,24 +139,26 @@ export function DocumentCard({
|
||||
>
|
||||
<CardContent className="flex h-full flex-col gap-2 px-4">
|
||||
<div className="flex items-start gap-2">
|
||||
{document.hasThumbnail && !thumbFailed ? (
|
||||
// Leading-edge preview tile. `loading="lazy"` + the timeline's
|
||||
// virtualization means only cards near the viewport fetch their
|
||||
// thumbnail. Decorative (alt="") — the title beside it names the
|
||||
// document; an authed same-origin subresource, never cached
|
||||
// cross-user. onError falls back to the kind icon.
|
||||
{thumbnailUrl && !thumbnailFailed ? (
|
||||
// Leading-edge preview tile. Decorative (alt="") — the title
|
||||
// beside it names the document. The laziness that `loading="lazy"`
|
||||
// used to provide now lives in the hook's null path: a card with no
|
||||
// thumbnail, or one whose thumbnail already failed, issues no
|
||||
// request at all. onError falls back to the kind icon.
|
||||
<span
|
||||
data-slot="document-thumbnail"
|
||||
className="bg-muted mt-0.5 block size-12 shrink-0 overflow-hidden rounded-md"
|
||||
>
|
||||
{/* Authed, private (no-store) same-origin subresource — next/image
|
||||
would try to proxy/optimize it, which is wrong for a PHI blob
|
||||
we deliberately never cache. */}
|
||||
{/* An object URL over bytes the app transport already fetched,
|
||||
not a `/api/…` subresource: the browser issues a subresource
|
||||
itself and so cannot carry the record-session assertion, which
|
||||
the fence refuses on any session that has been inside a shared
|
||||
record. `next/image` is wrong here for the older reason too —
|
||||
it would proxy a PHI blob we deliberately never cache. */}
|
||||
{/* eslint-disable-next-line @next/next/no-img-element */}
|
||||
<img
|
||||
src={`/api/documents/inbound/${document.id}/thumbnail`}
|
||||
src={thumbnailUrl}
|
||||
alt=""
|
||||
loading="lazy"
|
||||
decoding="async"
|
||||
onError={() => setThumbFailed(true)}
|
||||
className="size-full object-cover"
|
||||
|
||||
@@ -64,6 +64,7 @@ import {
|
||||
import { useFormatters, useTranslations } from "@/lib/i18n/context";
|
||||
import { useCoachLaunch } from "@/lib/insights/coach-launch-context";
|
||||
import { invalidateKeys, queryKeys } from "@/lib/query-keys";
|
||||
import { useFencedObjectUrl } from "@/hooks/use-fenced-object-url";
|
||||
import { cn } from "@/lib/utils";
|
||||
import {
|
||||
INBOUND_DOCUMENT_KINDS,
|
||||
@@ -105,7 +106,12 @@ function InlinePreview({
|
||||
title: string;
|
||||
}) {
|
||||
const [loaded, setLoaded] = useState(false);
|
||||
const src = `/api/documents/inbound/${documentId}/original`;
|
||||
// v1.37.0 — the bytes come through the app transport, which carries the
|
||||
// record-session assertion; a browser-issued `src="/api/…"` cannot, and the
|
||||
// fence refuses it on any session that has been inside a shared record.
|
||||
const { url: src, failed } = useFencedObjectUrl(
|
||||
`/api/documents/inbound/${documentId}/original`,
|
||||
);
|
||||
const isPdf = mimeType === "application/pdf";
|
||||
// Chromium's PDF viewer grabs focus while it initialises — BEFORE the
|
||||
// frame's `load` event — stranding keyboard events inside the embedded
|
||||
@@ -144,12 +150,23 @@ function InlinePreview({
|
||||
|
||||
return (
|
||||
<div className="relative max-h-[55vh] overflow-auto overscroll-contain rounded-lg">
|
||||
{!loaded ? (
|
||||
{failed ? (
|
||||
// The preview could not be fetched — refused, offline, or still
|
||||
// rendering server-side. A permanent skeleton would read as "loading
|
||||
// forever"; the sheet's own metadata and the download control below
|
||||
// still work.
|
||||
<p
|
||||
data-slot="document-preview-unavailable"
|
||||
className="text-muted-foreground py-8 text-center text-sm"
|
||||
>
|
||||
{title}
|
||||
</p>
|
||||
) : !loaded || src === null ? (
|
||||
<Skeleton
|
||||
className={cn("w-full rounded-lg", isPdf ? "h-[55vh]" : "h-64")}
|
||||
/>
|
||||
) : null}
|
||||
{isPdf ? (
|
||||
{src === null || failed ? null : isPdf ? (
|
||||
// No `sandbox` attribute by design: Chromium force-downloads PDFs
|
||||
// in sandboxed frames instead of rendering them. The serve
|
||||
// response itself carries `default-src 'none'` +
|
||||
@@ -487,7 +504,13 @@ export function DocumentDetailSheet({
|
||||
|
||||
const title = doc?.title ?? doc?.filename ?? t("documents.card.untitled");
|
||||
const Icon = doc ? DOCUMENT_KIND_ICONS[doc.kind] : null;
|
||||
const originalHref = doc ? `/api/documents/inbound/${doc.id}/original` : "#";
|
||||
// v1.37.0 — an object URL over transport-fetched bytes rather than a bare
|
||||
// `/api/…` href. A download navigation carries no record-session assertion,
|
||||
// so the fence refuses it once the session has been inside a shared record.
|
||||
// Null until the blob lands; the anchor is disabled rather than dead.
|
||||
const { url: originalHref } = useFencedObjectUrl(
|
||||
doc ? `/api/documents/inbound/${doc.id}/original` : null,
|
||||
);
|
||||
|
||||
return (
|
||||
<>
|
||||
@@ -590,7 +613,11 @@ export function DocumentDetailSheet({
|
||||
data-slot="document-download"
|
||||
aria-label={t("documents.detail.download")}
|
||||
>
|
||||
<a href={originalHref} download={doc.filename ?? undefined}>
|
||||
<a
|
||||
href={originalHref ?? "#"}
|
||||
download={doc.filename ?? undefined}
|
||||
aria-disabled={originalHref === null}
|
||||
>
|
||||
<Download className="size-4" aria-hidden />
|
||||
<span className="hidden sm:inline">
|
||||
{t("documents.detail.download")}
|
||||
@@ -637,7 +664,11 @@ export function DocumentDetailSheet({
|
||||
{formatBytes(doc.byteSize, locale)}
|
||||
</p>
|
||||
<Button asChild size="sm" className="mt-1">
|
||||
<a href={originalHref} download={doc.filename ?? undefined}>
|
||||
<a
|
||||
href={originalHref ?? "#"}
|
||||
download={doc.filename ?? undefined}
|
||||
aria-disabled={originalHref === null}
|
||||
>
|
||||
<Download className="size-4" aria-hidden />
|
||||
{t("documents.detail.download")}
|
||||
</a>
|
||||
|
||||
@@ -0,0 +1,197 @@
|
||||
/**
|
||||
* Reaching a fenced route from the DOM without a headerless request.
|
||||
*
|
||||
* Two properties, both of which go wrong quietly:
|
||||
*
|
||||
* * the bytes travel through the app transport, so the request carries the
|
||||
* record-session assertion. A plain `fetch` here would reintroduce the
|
||||
* exact class `record-fence-headerless-transport-guard.test.ts` exists to
|
||||
* catch, and nothing visible would change until somebody entered a shared
|
||||
* record;
|
||||
* * every object URL is revoked. Getting that wrong leaks a decrypted health
|
||||
* document into the tab for as long as the tab lives, and the UI never
|
||||
* shows it. The hard case is a response that lands after the caller has
|
||||
* moved on — a fast scroll through the vault timeline changes the path far
|
||||
* more often than the network answers.
|
||||
*
|
||||
* The unit suite runs in `node` with no DOM, so the React hook cannot be
|
||||
* rendered here. That is why the lifecycle rule lives in
|
||||
* `createFencedBlobLoader` rather than inside the effect: an untested
|
||||
* revocation rule is exactly the kind that is wrong.
|
||||
*/
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
|
||||
|
||||
import { createFencedBlobLoader } from "@/hooks/use-fenced-object-url";
|
||||
import {
|
||||
__resetRecordFenceForTests,
|
||||
adoptRecordFenceState,
|
||||
} from "@/lib/api/record-fence";
|
||||
|
||||
const created: string[] = [];
|
||||
const revoked: string[] = [];
|
||||
let counter = 0;
|
||||
|
||||
/** A deferred blob response, so a load can be left in flight deliberately. */
|
||||
function pendingResponse() {
|
||||
let settle!: () => void;
|
||||
const gate = new Promise<void>((resolve) => {
|
||||
settle = resolve;
|
||||
});
|
||||
return {
|
||||
settle,
|
||||
fetch: vi.fn(async () => {
|
||||
await gate;
|
||||
return new Response(new Blob(["bytes"]));
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
created.length = 0;
|
||||
revoked.length = 0;
|
||||
counter = 0;
|
||||
__resetRecordFenceForTests();
|
||||
vi.stubGlobal("URL", {
|
||||
createObjectURL: vi.fn(() => {
|
||||
const url = `blob:mock/${counter++}`;
|
||||
created.push(url);
|
||||
return url;
|
||||
}),
|
||||
revokeObjectURL: vi.fn((url: string) => {
|
||||
revoked.push(url);
|
||||
}),
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
__resetRecordFenceForTests();
|
||||
});
|
||||
|
||||
describe("createFencedBlobLoader", () => {
|
||||
it("carries the record-session assertion on the request", async () => {
|
||||
adoptRecordFenceState({ epoch: 4, scope: "owner-1" });
|
||||
const fetchMock = vi.fn(async () => new Response(new Blob(["bytes"])));
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
await createFencedBlobLoader().load(
|
||||
"/api/documents/inbound/doc-1/thumbnail",
|
||||
);
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
const [, init] = fetchMock.mock.calls[0] as unknown as [
|
||||
string,
|
||||
RequestInit,
|
||||
];
|
||||
const headers = new Headers(init.headers);
|
||||
// The whole point: a browser-issued `<img src>` cannot send these, which is
|
||||
// why the bytes come through the transport instead.
|
||||
expect(headers.get("x-healthlog-record-epoch")).toBe("4");
|
||||
expect(headers.get("x-healthlog-record-scope")).toBe("owner-1");
|
||||
});
|
||||
|
||||
it("hands back an object URL once the blob lands", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => new Response(new Blob(["bytes"]))),
|
||||
);
|
||||
|
||||
const result = await createFencedBlobLoader().load("/api/x/y");
|
||||
|
||||
expect(result).toEqual({ kind: "loaded", url: "blob:mock/0" });
|
||||
expect(created).toEqual(["blob:mock/0"]);
|
||||
expect(revoked).toEqual([]);
|
||||
});
|
||||
|
||||
it("revokes the live URL on dispose", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => new Response(new Blob(["bytes"]))),
|
||||
);
|
||||
|
||||
const loader = createFencedBlobLoader();
|
||||
await loader.load("/api/x/y");
|
||||
loader.dispose();
|
||||
|
||||
expect(revoked).toEqual(["blob:mock/0"]);
|
||||
});
|
||||
|
||||
it("revokes a response that lands AFTER dispose instead of storing it", async () => {
|
||||
// The leak that has no visible symptom: the component unmounted, nothing
|
||||
// will ever render this, and without the check it stays alive for the life
|
||||
// of the tab.
|
||||
const { settle, fetch } = pendingResponse();
|
||||
vi.stubGlobal("fetch", fetch);
|
||||
|
||||
const loader = createFencedBlobLoader();
|
||||
const inFlight = loader.load("/api/x/y");
|
||||
loader.dispose();
|
||||
settle();
|
||||
|
||||
expect(await inFlight).toEqual({ kind: "superseded" });
|
||||
expect(created).toEqual(["blob:mock/0"]);
|
||||
expect(revoked).toEqual(["blob:mock/0"]);
|
||||
});
|
||||
|
||||
it("revokes a response superseded by a newer load", async () => {
|
||||
const first = pendingResponse();
|
||||
vi.stubGlobal("fetch", first.fetch);
|
||||
|
||||
const loader = createFencedBlobLoader();
|
||||
const stale = loader.load("/api/x/1");
|
||||
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => new Response(new Blob(["newer"]))),
|
||||
);
|
||||
const fresh = await loader.load("/api/x/2");
|
||||
first.settle();
|
||||
|
||||
expect(await stale).toEqual({ kind: "superseded" });
|
||||
expect(fresh).toEqual({ kind: "loaded", url: "blob:mock/0" });
|
||||
// The superseded one was revoked; the live one was not.
|
||||
expect(revoked).toEqual(["blob:mock/1"]);
|
||||
expect(created).toEqual(["blob:mock/0", "blob:mock/1"]);
|
||||
});
|
||||
|
||||
it("revokes the previous URL when the same loader loads again", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => new Response(new Blob(["bytes"]))),
|
||||
);
|
||||
|
||||
const loader = createFencedBlobLoader();
|
||||
await loader.load("/api/x/1");
|
||||
await loader.load("/api/x/2");
|
||||
|
||||
expect(created).toEqual(["blob:mock/0", "blob:mock/1"]);
|
||||
expect(revoked).toEqual(["blob:mock/0"]);
|
||||
});
|
||||
|
||||
it("reports a refusal as failed rather than leaving the caller loading", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => new Response("no", { status: 403 })),
|
||||
);
|
||||
|
||||
const result = await createFencedBlobLoader().load("/api/x/y");
|
||||
|
||||
expect(result).toEqual({ kind: "failed" });
|
||||
// Nothing was created, so nothing needs revoking.
|
||||
expect(created).toEqual([]);
|
||||
expect(revoked).toEqual([]);
|
||||
});
|
||||
|
||||
it("reports a transport-level throw as failed too", async () => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => {
|
||||
throw new TypeError("offline");
|
||||
}),
|
||||
);
|
||||
|
||||
expect(await createFencedBlobLoader().load("/api/x/y")).toEqual({
|
||||
kind: "failed",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,138 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useState } from "react";
|
||||
|
||||
import { apiFetchRaw } from "@/lib/api/api-fetch";
|
||||
|
||||
/**
|
||||
* v1.37.0 — reach a record-fenced route from the DOM without a headerless
|
||||
* request.
|
||||
*
|
||||
* `<img src>`, `<iframe src>` and `<a href download>` are issued by the browser
|
||||
* itself. They never pass through `src/lib/api/api-fetch.ts`, so they carry no
|
||||
* record-session assertion — and once a session has entered or left a shared
|
||||
* record the fence refuses an unasserted request with 403
|
||||
* `sharing.access.denied`, permanently, because nothing about those three will
|
||||
* ever start sending a header. The visible symptom is a thumbnail that stops
|
||||
* rendering for good after somebody visits a shared record.
|
||||
*
|
||||
* So the bytes are fetched through the app transport, which attaches the
|
||||
* assertion and validates the echo, and the DOM is handed an object URL
|
||||
* instead. The shape is not new here: `document-ai-transport.ts` already
|
||||
* reaches `/api/documents/inbound/{id}/original` this way to build a `File` for
|
||||
* in-browser OCR. `src/__tests__/record-fence-headerless-transport-guard.test.ts`
|
||||
* is what keeps a future `src="/api/…"` from reintroducing the class.
|
||||
*
|
||||
* ## Why the loader is separate from the hook
|
||||
*
|
||||
* Revocation is the part worth being careful about: getting it wrong leaks a
|
||||
* decrypted health document into the tab's memory for as long as the tab lives,
|
||||
* and nothing in the UI ever shows it. The unit suite runs in `node` with no
|
||||
* DOM and no testing-library, so a hook cannot be rendered here — and an
|
||||
* untested revocation rule is exactly the kind that is wrong. The rule
|
||||
* therefore lives in {@link createFencedBlobLoader}, which is a plain object
|
||||
* with `load` and `dispose`, fully exercised in
|
||||
* `src/hooks/__tests__/use-fenced-object-url.test.ts`. The hook below is the
|
||||
* thin React lifecycle shell over it.
|
||||
*/
|
||||
export interface FencedObjectUrl {
|
||||
/** The object URL, or null while loading, disabled, or failed. */
|
||||
url: string | null;
|
||||
/** The fetch finished and did not produce bytes. Render the fallback. */
|
||||
failed: boolean;
|
||||
}
|
||||
|
||||
/** What one load produced. */
|
||||
export type FencedBlobResult =
|
||||
{ kind: "loaded"; url: string } | { kind: "failed" } | { kind: "superseded" };
|
||||
|
||||
/**
|
||||
* A one-slot object-URL holder.
|
||||
*
|
||||
* Holds at most one live URL. Loading again revokes the previous one; a
|
||||
* response that lands after `dispose()` or after a newer `load()` is revoked
|
||||
* immediately rather than stored, which is the case a fast scroll through the
|
||||
* vault timeline produces constantly — the path changes far more often than the
|
||||
* network answers.
|
||||
*/
|
||||
export function createFencedBlobLoader() {
|
||||
let current: string | null = null;
|
||||
let generation = 0;
|
||||
let disposed = false;
|
||||
|
||||
const release = () => {
|
||||
if (current !== null) {
|
||||
URL.revokeObjectURL(current);
|
||||
current = null;
|
||||
}
|
||||
};
|
||||
|
||||
return {
|
||||
async load(path: string): Promise<FencedBlobResult> {
|
||||
const mine = ++generation;
|
||||
release();
|
||||
let url: string | null = null;
|
||||
try {
|
||||
const res = await apiFetchRaw(path);
|
||||
if (!res.ok) throw new Error(`HTTP ${res.status}`);
|
||||
const blob = await res.blob();
|
||||
url = URL.createObjectURL(blob);
|
||||
} catch {
|
||||
// Every failure reads the same to the caller — refused, offline, gone,
|
||||
// or a context mismatch the transport discarded. The caller renders its
|
||||
// fallback either way, and the fence's own reconciliation handles the
|
||||
// one case that is recoverable.
|
||||
return disposed || mine !== generation
|
||||
? { kind: "superseded" }
|
||||
: { kind: "failed" };
|
||||
}
|
||||
if (disposed || mine !== generation) {
|
||||
// Nobody is going to render this. Revoke it here or it lives as long
|
||||
// as the tab does.
|
||||
URL.revokeObjectURL(url);
|
||||
return { kind: "superseded" };
|
||||
}
|
||||
current = url;
|
||||
return { kind: "loaded", url };
|
||||
},
|
||||
dispose() {
|
||||
disposed = true;
|
||||
generation += 1;
|
||||
release();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export function useFencedObjectUrl(path: string | null): FencedObjectUrl {
|
||||
// The resolved path is stored ALONGSIDE the result, and the return value is
|
||||
// derived by comparing it against the current one. That is what keeps this
|
||||
// free of a synchronous `setState` inside the effect: while the path has just
|
||||
// changed and the effect has not run yet, the answer is "loading" because the
|
||||
// stored result belongs to a different path — not because something reset it.
|
||||
// Without that, a fast scroll through the vault timeline would paint one
|
||||
// card's thumbnail on the next card for a frame.
|
||||
const [resolved, setResolved] = useState<{
|
||||
path: string | null;
|
||||
url: string | null;
|
||||
failed: boolean;
|
||||
}>({ path: null, url: null, failed: false });
|
||||
|
||||
useEffect(() => {
|
||||
if (path === null) return;
|
||||
const loader = createFencedBlobLoader();
|
||||
void loader.load(path).then((result) => {
|
||||
if (result.kind === "superseded") return;
|
||||
setResolved({
|
||||
path,
|
||||
url: result.kind === "loaded" ? result.url : null,
|
||||
failed: result.kind === "failed",
|
||||
});
|
||||
});
|
||||
return () => loader.dispose();
|
||||
}, [path]);
|
||||
|
||||
if (path === null || resolved.path !== path) {
|
||||
return { url: null, failed: false };
|
||||
}
|
||||
return { url: resolved.url, failed: resolved.failed };
|
||||
}
|
||||
Reference in new issue
Block a user