diff --git a/CHANGELOG.md b/CHANGELOG.md index 732ac6186..db9d51039 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,30 @@ # Changelog +## [Unreleased] + +### Fixed + +- **On a self-host served over plain HTTP, picking a file to upload did + nothing at all.** No document appeared, no progress bar, no error — the + file picker closed and the page sat there, and the only way to get an + upload through was to reach the app over HTTPS. Browsers withhold a + handful of APIs from pages served over plain HTTP, and one of them is the + call the upload used to label each transfer so a retry could not store the + same file twice. That call is not a security measure here and nothing in + an upload needs it: the file travels the same way either way, and the + encryption happens on the server. It is now built from a source that plain + HTTP does allow, so uploads work over HTTP on a LAN, a VPN or Tailscale + exactly as they do over HTTPS. Passkeys, web push and installing the app + to a home screen still need HTTPS; browsers withhold those outright and no + application can hand them back. +- **The same missing call broke every save on a plain-HTTP host, not only + uploads.** Recording a measurement, editing a medication, restoring a + backup — anything that wrote — failed from the moment the app started + labelling writes this way in v1.37.18. Those all work again. +- **The import guide link under Settings → Import went the long way round.** + Both the CSV and the JSON card now link straight to the guide instead of + through a redirect. + ## [1.38.8] — 2026-09-03 The heart-rate-variability tile now works for accounts whose HRV comes diff --git a/src/__tests__/client-random-id-guard.test.ts b/src/__tests__/client-random-id-guard.test.ts new file mode 100644 index 000000000..fc1c9491a --- /dev/null +++ b/src/__tests__/client-random-id-guard.test.ts @@ -0,0 +1,130 @@ +/** + * Browser-side code does not call `crypto.randomUUID()` directly. + * + * `randomUUID` is `[SecureContext]` in the Web Cryptography IDL, so it is + * absent on a plain-HTTP origin — the LAN self-host case. A direct call + * there throws `TypeError: crypto.randomUUID is not a function`, and the + * two places it was reached from were both silent: the document-vault + * upload threw inside a file-input change handler (a picker that appeared + * to do nothing), and the client transport threw while assembling the + * default `Idempotency-Key` for every `apiPost` / `apiPatch`. + * + * `randomId()` in `src/lib/random-id.ts` covers both contexts. This guard + * keeps the class from coming back one call site at a time. + * + * Server-side files are exempt: Node's global `crypto` carries + * `randomUUID` unconditionally, and `node:crypto` imports are unrelated. + * The exemption is an explicit list, not a pattern, so a new browser file + * cannot join it by accident. + * + * Limit, stated so the next reader does not over-trust it: the matcher is + * textual. `const f = crypto["randomUUID"]` or a destructured + * `const { randomUUID } = crypto` would slip it. + */ +import { readdirSync, readFileSync, statSync } from "node:fs"; +import { join, relative, sep } from "node:path"; +import { describe, expect, it } from "vitest"; + +const ROOT = join(__dirname, "..", ".."); +const SRC = join(ROOT, "src"); + +/** + * Pruned on purpose: the generated Prisma client, vendored dependencies, and + * the suites themselves are not the tree under audit. The walk descends into + * dot-prefixed directories, so `src/app/.well-known` stays inside the sweep — + * `fs.globSync` would drop it. + */ +const SKIP_DIRS = new Set([ + "__tests__", + "__mocks__", + "node_modules", + ".next", + "generated", +]); + +/** + * Files that legitimately call the platform API directly, each because it + * runs on the server where the property always exists. + */ +const SERVER_SIDE_EXEMPT = new Set([ + // Next middleware — runs on the edge runtime, never in the page. + "src/proxy.ts", + // The one module allowed to reach for the platform call. + "src/lib/random-id.ts", +]); + +/** A direct call, tolerant of whitespace between the parts. */ +const DIRECT_CALL = /\bcrypto\s*\.\s*randomUUID\s*\(/; + +/** + * Comments are prose, not calls. Without this the guard fires on the very + * doc comments that explain why the call was removed — a guard that its own + * explanation trips is one the next reader deletes. + */ +function stripComments(source: string): string { + return source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/[^\n]*/g, ""); +} + +/** Every `.ts(x)` under `src/`, as a repo-relative POSIX path. */ +function sourceFiles(dir: string = SRC, out: string[] = []): string[] { + for (const name of readdirSync(dir)) { + if (SKIP_DIRS.has(name)) continue; + const p = join(dir, name); + if (statSync(p).isDirectory()) { + sourceFiles(p, out); + } else if ( + (name.endsWith(".ts") || name.endsWith(".tsx")) && + !name.endsWith(".test.ts") && + !name.endsWith(".test.tsx") + ) { + out.push(relative(ROOT, p).split(sep).join("/")); + } + } + return out.sort(); +} + +describe("client-side randomUUID guard", () => { + it("sweeps a non-trivial number of files", () => { + // Without this the suite would pass by matching nothing at all. + expect(sourceFiles().length).toBeGreaterThan(500); + }); + + it("finds no direct crypto.randomUUID() outside the exempt set", () => { + const offenders = sourceFiles().filter( + (file) => + !SERVER_SIDE_EXEMPT.has(file) && + DIRECT_CALL.test(stripComments(readFileSync(join(ROOT, file), "utf8"))), + ); + + expect(offenders).toEqual([]); + }); + + it("still matches the call it is meant to catch", () => { + // The matcher's own positive control — a guard that cannot fire is + // indistinguishable from a clean tree. + expect(DIRECT_CALL.test('headers.set("k", crypto.randomUUID());')).toBe( + true, + ); + expect(DIRECT_CALL.test("const id = crypto . randomUUID ()")).toBe(true); + expect(DIRECT_CALL.test("const id = randomId();")).toBe(false); + // …and does not fire on prose about the call. + expect( + DIRECT_CALL.test( + stripComments("/**\n * it did so with crypto.randomUUID().\n */"), + ), + ).toBe(false); + expect( + DIRECT_CALL.test(stripComments("// const id = crypto.randomUUID();")), + ).toBe(false); + }); + + it("keeps every exempt file real and still calling it", () => { + for (const file of SERVER_SIDE_EXEMPT) { + const source = stripComments(readFileSync(join(ROOT, file), "utf8")); + expect( + DIRECT_CALL.test(source), + `${file} no longer needs the exemption`, + ).toBe(true); + } + }); +}); diff --git a/src/components/admin/backups-section.tsx b/src/components/admin/backups-section.tsx index 6f2c0f1d6..613f6799d 100644 --- a/src/components/admin/backups-section.tsx +++ b/src/components/admin/backups-section.tsx @@ -47,6 +47,7 @@ import { SettingsCardHeader } from "@/components/settings/_card-header"; import { ListRow } from "@/components/ui/list-row"; import { useFormatters, useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; +import { randomId } from "@/lib/random-id"; import type { BackupRow, BackupsList } from "@/types/backups"; import type { BackupScheduleStatus } from "@/lib/jobs/backup-schedule-status"; import type { @@ -781,7 +782,7 @@ export function BackupsSection() { // Idempotency-Key prevents a double-click from re-running the // destructive transaction. Include the row id so two different // backups can both be restored independently in the same minute. - const idempotencyKey = `restore-${row.id}-${crypto.randomUUID()}`; + const idempotencyKey = `restore-${row.id}-${randomId()}`; return apiPost<{ restored: true; skipped?: RestoreSkipSummary }>( `/api/admin/backups/${row.id}/restore`, { confirm: "RESTORE" }, diff --git a/src/components/admin/danger-zone-section.tsx b/src/components/admin/danger-zone-section.tsx index 7548a514c..3ad0bc056 100644 --- a/src/components/admin/danger-zone-section.tsx +++ b/src/components/admin/danger-zone-section.tsx @@ -22,6 +22,7 @@ import { SettingsCard } from "@/components/settings/settings-card"; import { SettingsCardHeader } from "@/components/settings/_card-header"; import { useTranslations } from "@/lib/i18n/context"; import { apiDelete } from "@/lib/api/api-fetch"; +import { randomId } from "@/lib/random-id"; /** * The literal an admin must type to arm the wipe. Mirrors the Backups @@ -46,7 +47,7 @@ export function DangerZoneSection() { mutationFn: async () => { // Idempotency-Key prevents a double-submit from re-running the // destructive transaction, matching the Backups restore path. - const idempotencyKey = `admin-data-wipe-${crypto.randomUUID()}`; + const idempotencyKey = `admin-data-wipe-${randomId()}`; // The server returns one count per table that actually held rows. It // used to return nine fixed names, which is how nobody noticed the // wipe had stopped covering the schema: the result line read the same diff --git a/src/components/cycle/use-cycle.ts b/src/components/cycle/use-cycle.ts index 0e8b4fd22..d041dabec 100644 --- a/src/components/cycle/use-cycle.ts +++ b/src/components/cycle/use-cycle.ts @@ -20,6 +20,7 @@ import { apiPost, } from "@/lib/api/api-fetch"; import { useTranslations } from "@/lib/i18n/context"; +import { randomId } from "@/lib/random-id"; import { toastWrittenOutcome } from "@/components/outcome/outcome-toast"; import { @@ -225,7 +226,7 @@ export function useCycleInsights() { /** A fresh idempotency key per write attempt (matches the iOS Outbox shape). */ function idempotencyKey(): string { - return crypto.randomUUID(); + return randomId(); } /** diff --git a/src/components/documents/__tests__/use-document-upload-insecure-context.test.ts b/src/components/documents/__tests__/use-document-upload-insecure-context.test.ts new file mode 100644 index 000000000..bb7925998 --- /dev/null +++ b/src/components/documents/__tests__/use-document-upload-insecure-context.test.ts @@ -0,0 +1,143 @@ +/** + * The vault upload on a plain-HTTP origin — the reported failure. + * + * A self-hoster on plain HTTP over a LAN picked a file and nothing + * happened: no queue row, no request, no error toast. The upload minted an + * `Idempotency-Key` (and its local queue id) with `crypto.randomUUID()`, + * which a non-secure context does not expose — measured in Chromium on + * `http://`: `isSecureContext` false, `crypto.randomUUID` + * undefined, `crypto.getRandomValues` still a function. The call threw + * synchronously inside the file-input change handler, so the throw had + * nowhere to surface and the picker looked inert. + * + * Watched red: restoring `crypto.randomUUID()` in `uploadViaXhr` fails + * "sends the multipart POST" with the plain-HTTP TypeError, and the + * secure-context case stays green — which is the whole shape of the bug. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { uploadViaXhr } from "../use-document-upload"; + +const UUID_V4 = + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; + +/** + * Bound BEFORE any stubbing: `vi.stubGlobal("crypto", …)` replaces + * `globalThis.crypto`, so a stub that delegated through the global would + * call itself. + */ +const realGetRandomValues = globalThis.crypto.getRandomValues.bind( + globalThis.crypto, +); + +/** + * The `crypto` a plain-HTTP origin actually hands the page: real + * `getRandomValues`, no `randomUUID`, no `subtle`. + */ +function insecureCrypto(): Crypto { + return { getRandomValues: realGetRandomValues } as Crypto; +} + +interface SentRequest { + method: string; + url: string; + headers: Record; + body: FormData | null; +} + +const sent: SentRequest[] = []; + +/** Minimal XHR double that records the request and resolves with a 201. */ +class FakeXhr { + status = 0; + responseText = ""; + upload = { addEventListener: () => {} }; + private listeners = new Map void>(); + private request: SentRequest = { + method: "", + url: "", + headers: {}, + body: null, + }; + + open(method: string, url: string): void { + this.request.method = method; + this.request.url = url; + } + + setRequestHeader(name: string, value: string): void { + this.request.headers[name] = value; + } + + addEventListener(type: string, listener: () => void): void { + this.listeners.set(type, listener); + } + + send(body: FormData): void { + this.request.body = body; + sent.push(this.request); + this.status = 201; + this.responseText = JSON.stringify({ + data: { id: "doc-1", fileName: "scan.pdf" }, + error: null, + }); + this.listeners.get("load")?.(); + } +} + +beforeEach(() => { + sent.length = 0; + vi.stubGlobal("XMLHttpRequest", FakeXhr); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +function pdf(): File { + return new File([new Uint8Array([1, 2, 3])], "scan.pdf", { + type: "application/pdf", + }); +} + +describe("vault upload on a plain-HTTP origin", () => { + beforeEach(() => { + vi.stubGlobal("crypto", insecureCrypto()); + }); + + it("has no crypto.randomUUID to call — the control", () => { + expect(typeof (crypto as Crypto).randomUUID).toBe("undefined"); + }); + + it("sends the multipart POST", async () => { + const result = await uploadViaXhr(pdf(), {}, () => {}); + + expect(sent).toHaveLength(1); + expect(sent[0]!.method).toBe("POST"); + expect(sent[0]!.url).toBe("/api/documents/inbound"); + expect(sent[0]!.body?.get("file")).toBeInstanceOf(File); + expect(result.ok).toBe(true); + }); + + it("still carries a v4 Idempotency-Key", async () => { + await uploadViaXhr(pdf(), {}, () => {}); + + expect(sent[0]!.headers["Idempotency-Key"]).toMatch(UUID_V4); + }); + + it("still forwards the episode link on a deep-link upload", async () => { + await uploadViaXhr(pdf(), { episodeId: "ep-1" }, () => {}); + + expect(sent[0]!.body?.get("episodeIds")).toBe("ep-1"); + }); +}); + +describe("vault upload in a secure context", () => { + it("is unchanged — a v4 Idempotency-Key and one POST", async () => { + await uploadViaXhr(pdf(), {}, () => {}); + + expect(sent).toHaveLength(1); + expect(sent[0]!.headers["Idempotency-Key"]).toMatch(UUID_V4); + expect(typeof globalThis.crypto.randomUUID).toBe("function"); + }); +}); diff --git a/src/components/documents/use-document-upload.ts b/src/components/documents/use-document-upload.ts index ee6f5c1d2..d58919ef2 100644 --- a/src/components/documents/use-document-upload.ts +++ b/src/components/documents/use-document-upload.ts @@ -23,6 +23,7 @@ import { useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; import { useTranslations } from "@/lib/i18n/context"; +import { randomId } from "@/lib/random-id"; import { invalidateKeys, queryKeys } from "@/lib/query-keys"; import { parseUploadResponse, @@ -103,8 +104,14 @@ async function downscaleImage(file: File): Promise { } } -/** One multipart POST via XHR, reporting upload progress. */ -function uploadViaXhr( +/** + * One multipart POST via XHR, reporting upload progress. + * + * Exported for the unit test: this function mints the `Idempotency-Key`, + * and it did so with `crypto.randomUUID()`, which a plain-HTTP origin does + * not expose. The test drives it with that property absent. + */ +export function uploadViaXhr( file: File, options: EnqueueOptions, onProgress: (fraction: number) => void, @@ -114,7 +121,7 @@ function uploadViaXhr( xhr.open("POST", "/api/documents/inbound"); // Retry-safe by construction: the server honours Idempotency-Key, so a // network blip that DID land server-side cannot double-store on retry. - xhr.setRequestHeader("Idempotency-Key", crypto.randomUUID()); + xhr.setRequestHeader("Idempotency-Key", randomId()); xhr.upload.addEventListener("progress", (event) => { if (event.lengthComputable && event.total > 0) { onProgress(event.loaded / event.total); @@ -263,7 +270,7 @@ export function useDocumentUpload( if (files.length === 0) return; const fresh: UploadQueueItem[] = []; for (const file of files) { - const localId = crypto.randomUUID(); + const localId = randomId(); // Client pre-flight against the server's configured cap: an obvious // giant (a CD image) fails instantly and locally — friendly reason, // no wasted transfer. The server re-enforces regardless. diff --git a/src/components/insights/coach-panel/coach-conversation.tsx b/src/components/insights/coach-panel/coach-conversation.tsx index d712b793f..d4cb5b493 100644 --- a/src/components/insights/coach-panel/coach-conversation.tsx +++ b/src/components/insights/coach-panel/coach-conversation.tsx @@ -16,6 +16,7 @@ import { Button } from "@/components/ui/button"; import { cn } from "@/lib/utils"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; +import { randomId } from "@/lib/random-id"; import { apiDelete, apiFetchRaw, apiGet } from "@/lib/api/api-fetch"; import type { CoachScope } from "@/lib/ai/coach/types"; import type { CoachLaunchScope } from "@/lib/insights/coach-launch-context"; @@ -528,7 +529,7 @@ export function CoachConversation({ try { response = await apiFetchRaw("/api/documents/inbound", { method: "POST", - headers: { "Idempotency-Key": crypto.randomUUID() }, + headers: { "Idempotency-Key": randomId() }, body: form, }); } catch { diff --git a/src/components/settings/import-panel/__tests__/import-docs-link.test.ts b/src/components/settings/import-panel/__tests__/import-docs-link.test.ts index 876a9998f..fd97fce62 100644 --- a/src/components/settings/import-panel/__tests__/import-docs-link.test.ts +++ b/src/components/settings/import-panel/__tests__/import-docs-link.test.ts @@ -3,28 +3,45 @@ * * The app serves no `/docs` tree, so the old internal * `/docs/integrations/data-import` href 404'd for every user who - * clicked it. The guide lives at `docs.healthlog.dev`; the link goes - * through `INTEGRATION_DOCS_BASE` so the host lives in exactly one - * place (the same constant every Settings → Integrations card uses). + * clicked it. That is the property this suite exists to hold. + * + * It used to assert `INTEGRATION_DOCS_BASE` and `data-import` by name. + * Both were how the link happened to be built, not what it had to be: + * the guide is not an integration runbook, it lives under `/guides/`, + * and `${INTEGRATION_DOCS_BASE}/data-import` only resolved through a + * redirect. Pinning the spelling would have made the correction fail a + * green suite, so the assertions now name the guarantee — an absolute + * `docs.healthlog.dev` URL, no internal path, new-tab hygiene — and + * `IMPORT_GUIDE_URL` owns the address in one place for both cards. * * Watched red: restoring `href="/docs/integrations/data-import"` in - * either card fails the matching assertion below. + * either card fails the internal-path assertion; dropping `rel` fails + * the hygiene one. */ import { readFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; +import { IMPORT_GUIDE_URL } from "../constants"; + const CARDS = ["csv-import-card.tsx", "json-import-card.tsx"] as const; describe("import-card docs links", () => { + it("resolves to an absolute docs-site guide, not a redirect stop", () => { + expect(IMPORT_GUIDE_URL).toMatch( + /^https:\/\/docs\.healthlog\.dev\/guides\/[a-z0-9-]+\/$/, + ); + }); + for (const card of CARDS) { it(`${card} links the external docs site, never an internal /docs path`, () => { const source = readFileSync(join(__dirname, "..", card), "utf8"); + expect(source).not.toContain('href="/docs'); - expect(source).toContain("INTEGRATION_DOCS_BASE"); - expect(source).toContain("data-import"); + expect(source).toContain("IMPORT_GUIDE_URL"); // External link hygiene for a new-tab docs jump. expect(source).toContain('rel="noopener noreferrer"'); + expect(source).toContain('target="_blank"'); }); } }); diff --git a/src/components/settings/import-panel/constants.ts b/src/components/settings/import-panel/constants.ts index f283f7e82..e89bc5088 100644 --- a/src/components/settings/import-panel/constants.ts +++ b/src/components/settings/import-panel/constants.ts @@ -4,3 +4,14 @@ * before it ever reaches the route and feeds the live character counter. */ export const MAX_PASTE_CHARS = 16 * 1024 * 1024; + +/** + * The CSV / JSON import guide on the public docs site. + * + * Both import cards used `${INTEGRATION_DOCS_BASE}/data-import`, which is + * the wrong tree — the guide is not an integration runbook, and that path + * now only resolves through a redirect. This is the canonical address, so + * the link goes straight to the page. Kept here rather than in each card + * because both cards point at the same guide. + */ +export const IMPORT_GUIDE_URL = "https://docs.healthlog.dev/guides/import-csv/"; diff --git a/src/components/settings/import-panel/csv-import-card.tsx b/src/components/settings/import-panel/csv-import-card.tsx index 520f0a20d..2db0131bc 100644 --- a/src/components/settings/import-panel/csv-import-card.tsx +++ b/src/components/settings/import-panel/csv-import-card.tsx @@ -14,10 +14,9 @@ import { SettingsCardActions } from "@/components/settings/_card-actions"; import { Label } from "@/components/ui/label"; import { Textarea } from "@/components/ui/textarea"; import { useTranslations } from "@/lib/i18n/context"; -import { INTEGRATION_DOCS_BASE } from "@/components/settings/integrations/setup-guide-link"; import { apiFetchRaw } from "@/lib/api/api-fetch"; import { ImportCardShell } from "./import-card-shell"; -import { MAX_PASTE_CHARS } from "./constants"; +import { IMPORT_GUIDE_URL, MAX_PASTE_CHARS } from "./constants"; import { EXAMPLE_CSV } from "./import-examples"; import { CsvImportResultView, @@ -154,7 +153,7 @@ export function CsvImportCard() { {/* The import guide lives on the external docs site — the app itself serves no /docs tree, so an internal link 404s. */} ` — a plain-HTTP LAN self-host, the reporter's setup: + * + * isSecureContext false + * crypto object + * crypto.randomUUID undefined → "TypeError: … is not a function" + * crypto.subtle undefined + * crypto.getRandomValues function + * + * `insecureCrypto()` reproduces exactly that: `getRandomValues` present, + * `randomUUID` and `subtle` absent. + * + * Watched red: with the fallback branch removed — `randomId` reduced to a + * bare `return crypto.randomUUID()` — every insecure-context case below + * fails with the same TypeError the plain-HTTP host raises. + */ +import { afterEach, describe, expect, it, vi } from "vitest"; + +const UUID_V4 = + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; + +/** + * Bound BEFORE any stubbing: `vi.stubGlobal("crypto", …)` replaces + * `globalThis.crypto`, so a stub that delegated through the global would + * call itself. + */ +const realGetRandomValues = globalThis.crypto.getRandomValues.bind( + globalThis.crypto, +); + +/** + * The `crypto` a plain-HTTP origin actually hands the page: real + * `getRandomValues`, no `randomUUID`, no `subtle`. + */ +function insecureCrypto(): Crypto { + return { getRandomValues: realGetRandomValues } as Crypto; +} + +afterEach(() => { + vi.unstubAllGlobals(); + vi.resetModules(); +}); + +/** Fresh import per case — the module must not cache a context decision. */ +async function loadRandomId() { + vi.resetModules(); + return (await import("../random-id")).randomId; +} + +describe("randomId in a secure context", () => { + it("delegates to the platform randomUUID", async () => { + const randomUUID = vi.fn( + () => "11111111-2222-4333-8444-555555555555" as const, + ); + vi.stubGlobal("crypto", { ...globalThis.crypto, randomUUID }); + + const randomId = await loadRandomId(); + + expect(randomId()).toBe("11111111-2222-4333-8444-555555555555"); + expect(randomUUID).toHaveBeenCalledTimes(1); + }); +}); + +describe("randomId on a plain-HTTP origin", () => { + it("returns a v4 UUID without randomUUID present", async () => { + vi.stubGlobal("crypto", insecureCrypto()); + const randomId = await loadRandomId(); + + expect(randomId()).toMatch(UUID_V4); + }); + + it("does not throw where crypto.randomUUID would", async () => { + vi.stubGlobal("crypto", insecureCrypto()); + const randomId = await loadRandomId(); + + // The control: the call the app used to make is genuinely unavailable. + expect(() => (crypto as Crypto).randomUUID()).toThrow(TypeError); + expect(() => randomId()).not.toThrow(); + }); + + it("draws from getRandomValues, not a clock or Math.random", async () => { + const getRandomValues = vi.fn(realGetRandomValues); + vi.stubGlobal("crypto", { getRandomValues } as unknown as Crypto); + const randomId = await loadRandomId(); + + randomId(); + + expect(getRandomValues).toHaveBeenCalledTimes(1); + expect(getRandomValues.mock.calls[0]![0]).toBeInstanceOf(Uint8Array); + }); + + it("does not collide across a batch the size of a bulk upload", async () => { + vi.stubGlobal("crypto", insecureCrypto()); + const randomId = await loadRandomId(); + + const ids = new Set(Array.from({ length: 5_000 }, () => randomId())); + + expect(ids.size).toBe(5_000); + }); +}); diff --git a/src/lib/api/__tests__/api-fetch-insecure-context.test.ts b/src/lib/api/__tests__/api-fetch-insecure-context.test.ts new file mode 100644 index 000000000..cde2c95c0 --- /dev/null +++ b/src/lib/api/__tests__/api-fetch-insecure-context.test.ts @@ -0,0 +1,113 @@ +/** + * Every client write still works on a plain-HTTP origin. + * + * `apiPost` / `apiPatch` mint a default `Idempotency-Key`. That key came + * from `crypto.randomUUID()`, which the Web Cryptography IDL marks + * `[SecureContext]` — absent on `http://`, verified in Chromium. + * So on a LAN self-host served over plain HTTP the header line threw + * before `fetch` was ever reached, and all seventy-odd `apiPost` / + * `apiPatch` call sites failed with a raw TypeError. + * + * Watched red: restoring `crypto.randomUUID()` in + * `withDefaultIdempotencyKey` fails every case here. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const UUID_V4 = + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; + +const fetchMock = vi.fn(); + +/** + * Bound BEFORE any stubbing: `vi.stubGlobal("crypto", …)` replaces + * `globalThis.crypto`, so a stub that delegated through the global would + * call itself. + */ +const realGetRandomValues = globalThis.crypto.getRandomValues.bind( + globalThis.crypto, +); + +/** + * The `crypto` a plain-HTTP origin actually hands the page: real + * `getRandomValues`, no `randomUUID`, no `subtle`. + */ +function insecureCrypto(): Crypto { + return { getRandomValues: realGetRandomValues } as Crypto; +} + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +function idempotencyKeyOf(call: unknown[]): string | null { + return new Headers((call[1] as RequestInit | undefined)?.headers).get( + "Idempotency-Key", + ); +} + +beforeEach(() => { + vi.resetModules(); + vi.stubGlobal("fetch", fetchMock); + vi.stubGlobal("crypto", insecureCrypto()); +}); + +afterEach(() => { + fetchMock.mockReset(); + vi.unstubAllGlobals(); +}); + +describe("client writes on a plain-HTTP origin", () => { + it("has no crypto.randomUUID to call — the control", () => { + expect(typeof (crypto as Crypto).randomUUID).toBe("undefined"); + }); + + it("apiPost still sends, with a v4 Idempotency-Key", async () => { + fetchMock.mockResolvedValueOnce(jsonResponse({ data: { id: "m1" } })); + const { apiPost } = await import("../api-fetch"); + + await expect(apiPost("/api/measurements", { value: 80 })).resolves.toEqual({ + id: "m1", + }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(idempotencyKeyOf(fetchMock.mock.calls[0]!)).toMatch(UUID_V4); + }); + + it("apiPatch still sends, with a v4 Idempotency-Key", async () => { + fetchMock.mockResolvedValueOnce(jsonResponse({ data: { ok: true } })); + const { apiPatch } = await import("../api-fetch"); + + await apiPatch("/api/auth/me", { locale: "de" }); + + expect(idempotencyKeyOf(fetchMock.mock.calls[0]!)).toMatch(UUID_V4); + }); + + it("gives each write its own key", async () => { + // A fresh Response per call: a body can only be read once. + fetchMock.mockImplementation(() => jsonResponse({ data: null })); + const { apiPost } = await import("../api-fetch"); + + await apiPost("/api/measurements", { value: 80 }); + await apiPost("/api/measurements", { value: 81 }); + + const first = idempotencyKeyOf(fetchMock.mock.calls[0]!); + const second = idempotencyKeyOf(fetchMock.mock.calls[1]!); + expect(first).not.toBe(second); + }); + + it("still lets a caller's own natural key win", async () => { + fetchMock.mockResolvedValueOnce(jsonResponse({ data: null })); + const { apiPost } = await import("../api-fetch"); + + await apiPost( + "/api/admin/backups/b1/restore", + { confirm: "RESTORE" }, + { headers: { "Idempotency-Key": "restore-b1-fixed" } }, + ); + + expect(idempotencyKeyOf(fetchMock.mock.calls[0]!)).toBe("restore-b1-fixed"); + }); +}); diff --git a/src/lib/api/api-fetch.ts b/src/lib/api/api-fetch.ts index 80e1b6500..5d6dfcc31 100644 --- a/src/lib/api/api-fetch.ts +++ b/src/lib/api/api-fetch.ts @@ -49,6 +49,7 @@ * long-lived responses ride this path with their own signals. */ +import { randomId } from "@/lib/random-id"; import { readError } from "./read-error"; import { validateResponseContext, @@ -209,7 +210,7 @@ export function apiGet( function withDefaultIdempotencyKey(init: RequestInit): RequestInit { const headers = new Headers(init.headers); if (!headers.has("Idempotency-Key")) { - headers.set("Idempotency-Key", crypto.randomUUID()); + headers.set("Idempotency-Key", randomId()); } return { ...init, headers }; } diff --git a/src/lib/query-keys/record-session-transition.ts b/src/lib/query-keys/record-session-transition.ts index 9d9fd9406..c971883c3 100644 --- a/src/lib/query-keys/record-session-transition.ts +++ b/src/lib/query-keys/record-session-transition.ts @@ -4,6 +4,7 @@ import { RECORD_SCOPE_STORAGE_KEY, setRecordScope, } from "@/lib/query-keys/record-scope"; +import { randomId } from "@/lib/random-id"; /** * A browser-wide hold around a server-side record-session switch. @@ -58,13 +59,6 @@ function notify(): void { for (const listener of listeners) listener(); } -function makeId(): string { - if (typeof crypto !== "undefined" && "randomUUID" in crypto) { - return crypto.randomUUID(); - } - return `${Date.now()}-${Math.random().toString(36).slice(2)}`; -} - function parseWire(value: string | null): WireTransition | null { if (value === null) return null; try { @@ -162,7 +156,7 @@ function ensureTransport(): void { // cost. if (!isForeignScopeWrite(transition)) return; publish({ - id: makeId(), + id: randomId(), phase: "resolving", expectedScope, at: Date.now(), @@ -249,7 +243,7 @@ export function beginRecordSessionTransition( ): string { seed(); ensureTransport(); - const id = makeId(); + const id = randomId(); publish({ id, phase: "blocking", expectedScope, at: Date.now() }); return id; } @@ -296,7 +290,7 @@ export function holdForRecordSessionReconcile(): void { seed(); ensureTransport(); if (transition.phase === "resolving") return; - publish({ id: makeId(), phase: "resolving", at: Date.now() }); + publish({ id: randomId(), phase: "resolving", at: Date.now() }); } /** Release only the `/me` response that confirms the committed record. */ @@ -310,7 +304,7 @@ export function settleRecordSessionTransition( ) { return; } - const id = transition.id ?? makeId(); + const id = transition.id ?? randomId(); publish({ id, phase: "ready", at: Date.now() }); } @@ -322,7 +316,7 @@ export function settleRecordSessionTransition( */ export function settleRefusedRecordSessionTransition(): void { if (transition.phase !== "resolving") return; - const id = transition.id ?? makeId(); + const id = transition.id ?? randomId(); publish({ id, phase: "ready", at: Date.now() }); } diff --git a/src/lib/random-id.ts b/src/lib/random-id.ts new file mode 100644 index 000000000..94aee8762 --- /dev/null +++ b/src/lib/random-id.ts @@ -0,0 +1,55 @@ +/** + * A random v4 UUID that also works on a plain-HTTP origin. + * + * `crypto.randomUUID()` is marked `[SecureContext]` in the Web Cryptography + * IDL, so on a LAN self-host served over plain HTTP the property is simply + * absent and the call throws `TypeError: crypto.randomUUID is not a + * function`. Measured in Chromium on `http://`: `isSecureContext` + * false, `crypto.randomUUID` and `crypto.subtle` undefined — but + * `crypto.getRandomValues` still a function, because it carries no + * `[SecureContext]` marker. + * + * That threw where nobody caught it. The document-vault upload minted a + * queue id and an `Idempotency-Key` this way inside the file-input change + * handler, so picking a file on a plain-HTTP host did nothing at all: no + * queue row, no request, no error. Since the client transport started + * minting a default `Idempotency-Key`, every `apiPost` / `apiPatch` in the + * app failed the same way on such a host. + * + * So the id is built from `getRandomValues` whenever `randomUUID` is out of + * reach. Same 122 bits of entropy from the same CSPRNG, same RFC 4122 + * version-4 layout, no secure context required. Nothing in an upload needs + * one: the bytes go out over multipart XHR and every cryptographic + * operation happens on the server. + */ + +/** Hex lookup for the byte→string step; built once. */ +const HEX = Array.from({ length: 256 }, (_, i) => + i.toString(16).padStart(2, "0"), +); + +/** + * A fresh RFC 4122 version-4 UUID. + * + * Prefers the platform's own `randomUUID` and falls back to the + * always-available `getRandomValues`. Use this instead of + * `crypto.randomUUID()` in anything that runs in the browser — the + * `client-random-id-guard` test fails a direct call. + */ +export function randomId(): string { + if (typeof crypto.randomUUID === "function") return crypto.randomUUID(); + + const bytes = crypto.getRandomValues(new Uint8Array(16)); + // RFC 4122 §4.4: version nibble to 4, variant bits to 0b10. + bytes[6] = (bytes[6]! & 0x0f) | 0x40; + bytes[8] = (bytes[8]! & 0x3f) | 0x80; + + const hex = Array.from(bytes, (b) => HEX[b]!).join(""); + return [ + hex.slice(0, 8), + hex.slice(8, 12), + hex.slice(12, 16), + hex.slice(16, 20), + hex.slice(20), + ].join("-"); +}