From e593080d8ac63a596fe16f598e2d61ed83a26b22 Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 15:58:38 +0200 Subject: [PATCH 01/13] feat(push): wake a closed browser, over Web Push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server registered wake tokens and woke nobody: `Waker`'s only implementation was `Silent`. A messenger that does not wake the phone is not a messenger, and this was the first thing between the deployment and somebody able to use it. The roadmap described FCM and APNs and called the missing half "the part that requires secrets". That was true and it was not the hard part. Device-side registration needs a Tauri plugin that does not exist, therefore Kotlin and Swift, and none of it compiles or runs here — no NDK, no macOS host, no device. Writing it would have produced what this repository refuses elsewhere: integration code that has never been executed and looks like a feature. Web Push removed that wall for one reason. The wake-up carries nothing, so there is no payload to encrypt, so the whole content-encryption half of Web Push — RFC 8291, aes128gcm, the `p256dh` and `auth` secrets — is unused. What is left is one ES256 signature. It needed no migration for the addresses either: without a payload the only thing worth keeping is the endpoint, so `Address { provider, token }` holds a subscription unchanged. One variable turns it on, `VAPID_SUBJECT`, and there is no private key to supply: the pair belongs to the server and is created on first start, the shape `log_key` has had since the transparency log. Unset, the waker stays `Silent`, the key route answers 503 and the client hides the control — the second of the three limits in `0011_push.sql`, still the behaviour to preserve first. Verified against the real thing, which is what separates this from a hope: Chrome subscribed through fcm.googleapis.com, the server signed and pushed, Google accepted, and the worker showed "New message" with the tab closed. The test that proves it is committed `#[ignore]` with the command to replay it. Six more run against a fake service and check what no real one can be asked to: that the body on the wire is empty, that the token verifies under the advertised key, that a 410 drops the subscription and a 500 does not. Two things this cost elsewhere, both stated in the roadmap: - The server has an outbound HTTP client now, which it never had. reqwest with rustls, no cookie store, no redirect following — a push endpoint answering with a redirect is not one to follow carrying a bearer token. - There is a service worker, and `notifications.ts` argued against one. The objection was a cache of the application shell served by the server the desktop build exists to stop trusting. This one caches nothing, registers no fetch handler, and `push.test.ts` asserts that rather than trusting the comment. Found while using it: two toggles a second apart left the switch claiming this browser would be woken with nothing subscribed, and the reverse after. Subscribing and unsubscribing both reach the push service and finish in an order nobody chose. The displayed state is now read back from the browser instead of inferred from the call, and toggles are chained so only one is in flight. No FCM, no APNs: the packaged mobile application is still only notified while it is open. `Vapid::wake` matches on the provider name, so a second emitter lands beside it without touching the call site. --- Cargo.lock | 1 + README.md | 9 +- apps/web/public/sw.js | 82 +++++ apps/web/src/App.tsx | 12 + apps/web/src/components/Notices.tsx | 104 ++++++- apps/web/src/lib/api.ts | 38 +++ apps/web/src/lib/notifications.ts | 23 +- apps/web/src/lib/push.test.ts | 109 +++++++ apps/web/src/lib/push.ts | 200 ++++++++++++ apps/web/src/lib/session.ts | 35 +++ crates/server/Cargo.toml | 9 + crates/server/migrations/0020_vapid.sql | 40 +++ crates/server/src/lib.rs | 39 ++- crates/server/src/main.rs | 10 +- crates/server/src/push.rs | 244 +++++++++++++++ crates/server/src/routes.rs | 25 ++ crates/server/src/vapid.rs | 249 +++++++++++++++ crates/server/tests/common/mod.rs | 26 ++ crates/server/tests/webpush.rs | 394 ++++++++++++++++++++++++ deploy/.env.example | 19 ++ deploy/docker-compose.yml | 5 + docs/DEPLOY.md | 45 ++- docs/ROADMAP.md | 108 ++++--- 23 files changed, 1771 insertions(+), 55 deletions(-) create mode 100644 apps/web/public/sw.js create mode 100644 apps/web/src/lib/push.test.ts create mode 100644 apps/web/src/lib/push.ts create mode 100644 crates/server/migrations/0020_vapid.sql create mode 100644 crates/server/src/vapid.rs create mode 100644 crates/server/tests/webpush.rs diff --git a/Cargo.lock b/Cargo.lock index 2adf9753..54a88193 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4939,6 +4939,7 @@ dependencies = [ "futures-util", "hex", "hmac", + "p256", "rand_core 0.6.4", "reqwest 0.12.28", "serde", diff --git a/README.md b/README.md index 692ebec7..2f3e39eb 100644 --- a/README.md +++ b/README.md @@ -43,10 +43,11 @@ not their equal and does not try to be. ## What does not work -- **Push notifications are half-built.** The server records tokens and decides who to wake, - and then sends nothing. There is no FCM or APNs provider, no configuration, no device-side - token registration and no user-facing setting. It is inert without configuration, and a - self-hosted deployment that talks to neither Apple nor Google stays fully functional. +- **Push reaches browsers, not the packaged mobile app.** Web Push works end to end — a browser + subscribes, the server signs a VAPID token, a notification arrives with the tab closed — and it + is off until a deployment sets `VAPID_SUBJECT`. FCM and APNs are not written: device-side + registration needs a Tauri plugin that does not exist, so the Tauri build is only notified while + it is open. The wake-up carries no text, no sender and no group id. - **Biometric unlock has never been executed.** The code exists; not one line of it has run. There is no Android NDK and no physical device on the development machine, so even the compilation of its dependency is unconfirmed. diff --git a/apps/web/public/sw.js b/apps/web/public/sw.js new file mode 100644 index 00000000..74733dfc --- /dev/null +++ b/apps/web/public/sw.js @@ -0,0 +1,82 @@ +/** + * The service worker, and what it deliberately is not. + * + * # Why this file exists at all, when the project refused one + * + * `src/lib/notifications.ts` refuses a service worker in as many words: "one would be a cache of + * the application shell served by the same server the desktop build exists to stop trusting". That + * objection is about **caching**. A worker that caches the shell keeps a copy of the application + * alive across visits, so a server that served a hostile bundle once keeps its victim even after + * it is fixed — which is a real and serious thing to refuse. + * + * This worker caches nothing. It registers no `fetch` handler, opens no `Cache`, keeps no + * precache manifest, and intercepts no request. Every load of the page comes from the network + * exactly as it did before this file existed, and deleting it changes nothing except that + * notifications stop arriving. It cannot serve a stale application because it cannot serve an + * application. + * + * The reason a worker is needed at all is that the Push API has no other delivery point: a push + * message wakes the *worker*, not the page, and there is no version of Web Push that reaches a + * document directly. + * + * # Why the text is a constant + * + * The worker cannot decrypt. The MLS keys live in a WASM module inside the page, in memory the + * worker has no access to, and moving them here would mean handing the decryption keys to a + * context that outlives every tab. So the notification says that something arrived, and nothing + * about what: the same answer iOS forces on every messenger, arrived at here on purpose rather + * than by constraint. + * + * That is also the third of the three limits in `migrations/0011_push.sql`: the wake-up carries + * no text, no sender and no group id. There is nothing here to display even if this file wanted + * to. + */ + +// Kept in step with `NOTICE_TITLE` and `NOTICE_BODY_ONE` in `src/lib/notifications.ts`. Duplicated +// rather than imported: a service worker is its own module graph, served as a plain file so that +// what is deployed is what can be read, and a build step to share two strings would cost more +// clarity than it saves. `push.test.ts` pins them against their source. +const TITLE = "Whispee"; +const BODY = "New message"; + +self.addEventListener("push", (event) => { + // `waitUntil` or the worker may be killed before the notification is shown. Browsers also + // require that a push handler show *something*: staying silent gets the subscription revoked + // after a few offences, and on some browsers displays a "this site was updated in the + // background" notice instead — worse than ours, and not ours to write. + event.waitUntil( + self.registration.showNotification(TITLE, { + body: BODY, + // The collapse key. Ten messages while the phone is in a pocket are one notification, not + // ten — the page does the same with `tag: conversation`, except this side does not know + // which conversation, so everything collapses into one. + tag: "whispee-wake", + // No `renotify`: the point of collapsing is not to buzz again for each one. + silent: false, + }), + ); +}); + +self.addEventListener("notificationclick", (event) => { + event.notification.close(); + + // Focus a tab that is already open before opening another. Somebody who clicks a notification + // wants the conversation they were already in, not a second copy of the application signing in + // from scratch. + event.waitUntil( + (async () => { + const clients = await self.clients.matchAll({ + type: "window", + includeUncontrolled: true, + }); + + for (const client of clients) { + if ("focus" in client) return client.focus(); + } + + // No deep link, and it is not an oversight: the wake-up does not say which conversation, + // so the honest destination is the application's front door. + return self.clients.openWindow("/"); + })(), + ); +}); diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 8c4f22be..01023978 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -353,6 +353,13 @@ function Frame({ // timer: retrying on a schedule keeps hammering a server that is down, and the two moments // that actually change the answer — a reconnection, a resume — are reported right here. void session.flushOutbox().then(bump); + + // And the wake address, on the same two moments and for a related reason: a push + // subscription rotates without warning, and a browser that renewed its own would otherwise + // be reachable at an address this server does not have — a phone that stops waking, with + // nothing anywhere to say why. Does nothing when this browser is not subscribed, so it + // never turns the feature on by itself. + void session.replayWaking(); }); const lost = () => setOffline(true); @@ -472,6 +479,11 @@ function Frame({ }); title.current ??= countUnreadInTitle(); + // Once per session, beside the notifier that handles the other half of the same job: this one + // covers the tab being closed, that one covers it being open. The address is only re-sent, so + // a browser that never subscribed stays unsubscribed. + void session.replayWaking(); + const notices = notifier.current; const counter = title.current; diff --git a/apps/web/src/components/Notices.tsx b/apps/web/src/components/Notices.tsx index fc950955..b1e2c7eb 100644 --- a/apps/web/src/components/Notices.tsx +++ b/apps/web/src/components/Notices.tsx @@ -25,15 +25,17 @@ * What that does not solve is the switch staying live while permission is denied — it is still * a recorded preference, and hiding it would lose the setting rather than explain it. */ -import { useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { DISCLOSE_NAME_COPY, notificationPermission, requestNotificationPermission, } from "@/lib/notifications"; +import { PUSH_DISCLOSURE_COPY, pushEnabled, pushSupported } from "@/lib/push"; import { useReport } from "@/state/report"; import { useBump, useSession } from "@/state/SessionProvider"; +import { Banner } from "@/ui/Banner"; import { Button } from "@/ui/Button"; import { Field } from "@/ui/Field"; import { Panel } from "@/ui/Panel"; @@ -45,6 +47,16 @@ export function NoticeSettings() { const report = useReport(); const [permission, setPermission] = useState(notificationPermission()); const [named, setNamed] = useState(session.discloseConversationName); + // Read from the browser rather than from the session, the way `Recovery.tsx` re-reads its + // factors from the server: the subscription is the state, and nothing of ours records it. It + // can also have gone away without this application being told — a browser drops a subscription + // when site data is cleared. + const [waking, setWaking] = useState(false); + const [busy, setBusy] = useState(false); + + useEffect(() => { + void pushEnabled().then(setWaking); + }, []); // Called from a click and from nowhere else. Nothing in this component runs it on mount, and // that is the whole reason the request lives behind a button rather than in an effect. @@ -67,6 +79,63 @@ export function NoticeSettings() { }); }; + // Async, unlike the switch above it, because both directions talk to the browser and to the + // server. `busy` rather than an optimistic flip: a subscription that failed to register would + // otherwise leave a switch saying the phone will wake when it will not. + /** + * The tail of the chain of toggles, so that only one is ever in flight. + * + * `busy` disables the switch while one runs, and that is not enough on its own: subscribing and + * unsubscribing both reach the browser's push service, and two of them started a second apart + * finish in an order nobody chose. Observed, both ways round — a switch left saying this browser + * would be woken with nothing subscribed, and the reverse. Chaining makes the order the one the + * clicks were in, which is the only order a person can reason about. + */ + const chain = useRef>(Promise.resolve()); + + const toggleWaking = (value: boolean) => { + setBusy(true); + + const change = chain.current + // A failed toggle must not stop the next one: the chain is about ordering, not about + // carrying an error forward. The `.catch` below still reports this one. + .catch(() => undefined) + .then(() => (value ? session.enableWaking() : session.disableWaking().then(() => true))); + + chain.current = change; + + change + .then(async (done) => { + if (value && !done) { + // Not a failure: `Api.vapidPublicKey` answers null on a 503, which is this deployment + // saying it does not do push. Saying so is better than a switch that flips back with no + // explanation. + report.error("This server does not send wake-ups."); + } + + // **Read back rather than trust the call.** Turning it off and on again inside a second + // leaves the browser's own unsubscribe still running while the new subscription is being + // made, and the second can lose to the first: the switch then says this browser will be + // woken while nothing is subscribed. Asking the browser what is true costs one call and + // makes that class of lie impossible — the subscription is the state, so it is the only + // thing worth displaying. + const actual = await pushEnabled(); + setWaking(actual); + + // Only claimed when it is true. A report that says "this browser will be woken" while the + // read-back disagrees would be the same lie one line further down. + if (actual === value) { + report.done(value ? "This browser will be woken." : "This browser will not be woken."); + } + }) + .catch((e: unknown) => { + report.error(e instanceof Error ? e.message : String(e)); + }) + .finally(() => { + setBusy(false); + }); + }; + return ( Allow notifications )} + {/* + * Below the permission cascade, because a wake-up that cannot show a notification is a + * wake-up for nothing — and above the disclosure switch, because that one refines what a + * notification says while this one decides whether there is one at all. + * + * The banner comes before the control, as on the recovery and vault screens: what this + * gives up is stated in the present tense, where somebody deciding will read it, and not + * in a hint under a switch they have already flipped. + */} + {pushSupported() ? ( + <> + + {PUSH_DISCLOSURE_COPY} + + + + {(control) => ( + + )} + + + ) : null} + {(control) => ( { + return this.request("POST", "/v1/push/token", { provider, token }); + } + + /** + * Drops this device's wake address. + * + * The row goes rather than gaining a disabled flag: what is not stored cannot leak with a + * database later. + */ + forgetPushToken(): Promise { + return this.request("POST", "/v1/push/forget", {}); + } + + /** + * The key a browser must subscribe against, or `null` when this deployment does not do push. + * + * Unsigned, and it has to be: a client asks before it has anything to subscribe. `null` on 503 + * rather than a throw — a deployment without push is not an error, it is a deployment offering + * one fewer thing, and the screen hides the control the way it does for calls. + */ + static async vapidPublicKey(): Promise { + const response = await fetch(`${BASE_URL}/v1/push/vapid`); + + if (response.status === 503) return null; + if (!response.ok) throw new ApiError(response.status, await response.text()); + + const body = (await response.json()) as { key: string }; + return body.key; + } + /** Stops or resumes broadcasting presence. Reciprocal: opting out means ceasing to see. */ setPresenceOptout(optout: boolean): Promise { return this.request("POST", "/v1/presence/optout", { optout }); diff --git a/apps/web/src/lib/notifications.ts b/apps/web/src/lib/notifications.ts index b50c6462..72de834e 100644 --- a/apps/web/src/lib/notifications.ts +++ b/apps/web/src/lib/notifications.ts @@ -311,12 +311,23 @@ export function createNotifier({ }); } catch { // `new Notification()` throws `TypeError: Illegal constructor` in the Android Chrome tab, - // where notifications exist only through a service worker's registration. There is no - // service worker here — one would be a cache of the application shell served by the same - // server the desktop build exists to stop trusting — so on that browser this feature is - // simply absent. Swallowed rather than reported: the caller has no repair to offer, and - // an error banner for "your browser cannot do this" on every message would be worse than - // the silence. + // where notifications exist only through a service worker's registration. Swallowed + // rather than reported: the caller has no repair to offer, and an error banner for "your + // browser cannot do this" on every message would be worse than the silence. + // + // **This used to say there was no service worker here, and that a worker would be a cache + // of the application shell served by the same server the desktop build exists to stop + // trusting.** There is one now — `public/sw.js` — and the sentence needed amending rather + // than deleting, because the objection it made is still right about the thing it names. + // That worker caches nothing: no `fetch` handler, no `Cache`, no precache manifest, and + // `push.test.ts` asserts the absence rather than trusting the comment. It exists because + // the Push API has no other delivery point — a push message wakes the worker, never a + // document — and it cannot serve a stale application because it cannot serve one at all. + // + // What that does **not** fix is the path this `catch` is on: the worker only runs for a + // push, so a tab open on Android Chrome still has no notification to show. The feature is + // absent there exactly as before, and `lib/push.ts` covers the other case — the tab + // closed — on the browsers that offer Web Push. return; } diff --git a/apps/web/src/lib/push.test.ts b/apps/web/src/lib/push.test.ts new file mode 100644 index 00000000..a96f0b14 --- /dev/null +++ b/apps/web/src/lib/push.test.ts @@ -0,0 +1,109 @@ +/** + * What can be tested about Web Push without a browser. + * + * `node --test` has no `navigator`, no service worker and no `PushManager`, so the subscription + * path itself is exercised by hand in a real browser — the procedure is in `docs/DEPLOY.md`, and + * it is what separates this feature from a hope. What is here is the part that is pure and that + * fails silently in production if it is wrong: the key decoding, the capability check, and the + * two strings the service worker cannot import. + */ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { test } from "node:test"; + +import { NOTICE_BODY_ONE, NOTICE_TITLE } from "./notifications.ts"; +import { PROVIDER, decodeApplicationServerKey, pushSupported } from "./push.ts"; + +/** + * The provider name is a wire value shared with the server. + * + * `push::WEB_PUSH` is the other half. They are two constants in two languages and nothing but this + * assertion connects them: a rename on one side alone produces a subscription the emitter skips — + * silently, because skipping an unknown provider is exactly what it is meant to do for a token + * whose provider has not landed yet. + */ +test("the provider name matches the one the server files subscriptions under", () => { + const source = readFileSync(new URL("../../../../crates/server/src/push.rs", import.meta.url), "utf8"); + + assert.match( + source, + new RegExp(`pub const WEB_PUSH: &str = "${PROVIDER}";`), + "the client and the server disagree on the provider name", + ); +}); + +/** + * The worker's copy is duplicated rather than imported, and this is what keeps the copy honest. + * + * A service worker is its own module graph, served as a plain file so what is deployed is what can + * be read. That costs two literals. Left unchecked they drift, and the drift is invisible: the + * notification simply starts saying something the rest of the application does not. + */ +test("the service worker shows the same words the application does", () => { + const worker = readFileSync(new URL("../../public/sw.js", import.meta.url), "utf8"); + + assert.match(worker, new RegExp(`const TITLE = "${NOTICE_TITLE}";`)); + assert.match(worker, new RegExp(`const BODY = "${NOTICE_BODY_ONE}";`)); +}); + +/** + * **The property that matters about the worker**, and it is an absence. + * + * `notifications.ts` refused a service worker because one would cache the application shell served + * by the server the desktop build exists to stop trusting. This worker is allowed to exist because + * it caches nothing. That is not a promise in a comment: a `fetch` handler or a `caches` call is + * what would turn it into the thing that was refused, so their absence is asserted. + */ +test("the service worker intercepts nothing and caches nothing", () => { + const worker = readFileSync(new URL("../../public/sw.js", import.meta.url), "utf8"); + const code = worker.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, ""); + + assert.doesNotMatch(code, /addEventListener\(\s*["']fetch["']/, "it would serve a stale bundle"); + assert.doesNotMatch(code, /caches\b/, "it would keep a copy of the application"); +}); + +test("a base64url key decodes to the sixty-five bytes of an uncompressed point", () => { + // A real P-256 public key as the server advertises it: 0x04 then two 32-byte coordinates. + const raw = new Uint8Array(65); + raw[0] = 0x04; + for (let index = 1; index < raw.length; index += 1) raw[index] = index; + + const base64url = Buffer.from(raw) + .toString("base64") + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); + + assert.deepEqual([...decodeApplicationServerKey(base64url)], [...raw]); +}); + +/** + * The padding is restored before decoding, and that is the whole reason this function exists + * rather than a bare `atob`. + * + * base64url as this protocol writes it is unpadded, `atob` demands padding, and the failure is an + * `InvalidCharacterError` raised inside the browser that names neither the value nor the caller. + */ +test("an unpadded key is decoded rather than refused", () => { + // Three bytes encode to four characters with no padding; two bytes need one `=`, one needs two. + assert.deepEqual([...decodeApplicationServerKey("AQID")], [1, 2, 3]); + assert.deepEqual([...decodeApplicationServerKey("AQI")], [1, 2]); + assert.deepEqual([...decodeApplicationServerKey("AQ")], [1]); +}); + +/** The two characters base64url replaces are the ones a raw key is most likely to contain. */ +test("the url alphabet is translated back", () => { + assert.deepEqual([...decodeApplicationServerKey("-_8")], [251, 255]); +}); + +/** + * Under `node --test` there is no `navigator` and no `PushManager`, so the capability check must + * answer no rather than throw. + * + * That is not a concession to the harness: it is the same answer a browser without push gives, + * and the screen hides the control on it. A `ReferenceError` here would take the settings screen + * down on exactly those browsers. + */ +test("push reports itself unsupported where the browser offers nothing", () => { + assert.equal(pushSupported(), false); +}); diff --git a/apps/web/src/lib/push.ts b/apps/web/src/lib/push.ts new file mode 100644 index 00000000..9b5ac4c1 --- /dev/null +++ b/apps/web/src/lib/push.ts @@ -0,0 +1,200 @@ +/** + * Web Push, from the browser's side. + * + * # What this gets you, and what it costs + * + * A message arriving while the tab is closed wakes the browser, which shows "New message" and + * nothing else. That is the whole feature. `notifications.ts` already handles the case where the + * tab is open, and keeps handling it — the two do not overlap, because a push handler only runs + * when no page is there to. + * + * The cost is two things, and both belong on the screen before the switch rather than in a + * document afterwards: + * + * 1. **The browser's push service learns the rhythm.** Chrome subscribes through Google, Firefox + * through Mozilla. That service sees a wake-up arrive for this browser every time a message + * does, and it can tie that to an IP address. The content stays encrypted; the timing does not. + * 2. **The server learns who to wake, which sealed sender was built to remove.** A server that + * chooses whom to wake gains a targeted activity trigger: ceasing to wake four members of five + * makes the next post attributable to the fifth. Nothing cryptographic answers this — see + * `docs/ROADMAP.md`, which says so at more length. + * + * # Why there is no stored setting + * + * The subscription itself is the state. `pushManager.getSubscription()` answers "is this browser + * subscribed" without anything of ours being written down, which is both one fewer thing to keep + * in step and the answer to something the roadmap asks for: a token has to be re-registered at + * every start, because it rotates without warning. Re-registering is just sending back whatever + * `getSubscription()` returns, so the replay and the read are the same operation. + * + * It is also per browser and not per account. `signal-sync.ts` gives the test — a fact about the + * *machine* rather than about the *account* — which is why this is never synchronised between + * devices, the same reason `locale` is not. + * + * # Why no payload + * + * Because the wake-up carries nothing, the whole content-encryption half of Web Push (RFC 8291) + * is unused: the `p256dh` and `auth` secrets a subscription carries are never read here and never + * sent anywhere. See `crates/server/src/vapid.rs` for the same observation from the other end. + */ +/** + * What this module needs from the server, and nothing more. + * + * A structural port rather than an import of `Api`, for the reason `notifications.ts` gives about + * every browser object it touches: `api.ts` uses constructor parameter properties, which + * `node --test` cannot strip, so importing it would make this module untestable. Naming the two + * methods used is also a shorter statement of what waking a browser can reach than a class with + * fifty. + */ +export interface PushApi { + setPushToken(provider: string, token: string): Promise; + forgetPushToken(): Promise; +} + +/** The provider name the server files this subscription under. Must match `push::WEB_PUSH`. */ +export const PROVIDER = "webpush"; + +/** + * Copy for the settings screen, stated before the choice — the same discipline as + * `DISCLOSE_NAME_COPY` and the vault screen, and exported from beside the behaviour so the + * sentence and the code cannot drift apart. + */ +export const PUSH_DISCLOSURE_COPY = + "Waking this browser means two things leave. Your browser's push service — Google for Chrome, " + + "Mozilla for Firefox — learns each time a message arrives for you, and can tie that to your " + + "address. And this server learns which devices to wake, which is exactly what it was arranged " + + "not to know: a server that stops waking four members of five can tell who wrote the next " + + "message. Nothing in the message itself is disclosed — the notification says a message " + + "arrived and never what it says or who sent it."; + +/** + * Is Web Push usable here at all? + * + * Three conditions, and the third is the one that surprises people: a secure context. Service + * workers and the Push API are both refused over plain http, `localhost` excepted. + */ +export function pushSupported(): boolean { + return ( + typeof navigator !== "undefined" && + "serviceWorker" in navigator && + typeof window !== "undefined" && + "PushManager" in window && + window.isSecureContext + ); +} + +/** + * Decodes the server's key into the form `subscribe` demands. + * + * The key travels as base64url because that is how it is written everywhere in this protocol, and + * arrives as a `BufferSource` because that is what the browser takes. Exported for its test: an + * error here produces `InvalidCharacterError` from deep inside the browser, which names nothing. + */ +export function decodeApplicationServerKey(base64url: string): Uint8Array { + const padded = base64url.replace(/-/g, "+").replace(/_/g, "/"); + const binary = atob(padded.padEnd(padded.length + ((4 - (padded.length % 4)) % 4), "=")); + + return Uint8Array.from(binary, (character) => character.charCodeAt(0)); +} + +/** + * Registers the worker, or `null` where it cannot be. + * + * Scoped to the root because that is where the file is served from and where the notification's + * click has to land. Failure is a `null`, not a throw: an unsupported browser and a blocked + * registration are the same thing to every caller here — this feature is absent — and neither is + * worth an error dialog on a path the user did not ask for. + */ +async function worker(): Promise { + if (!pushSupported()) return null; + + try { + return await navigator.serviceWorker.register("/sw.js", { scope: "/" }); + } catch (error) { + console.warn("service worker not registered", error); + return null; + } +} + +/** Is this browser subscribed right now? The subscription is the state; nothing else is read. */ +export async function pushEnabled(): Promise { + const registration = await worker(); + if (!registration) return false; + + return (await registration.pushManager.getSubscription()) !== null; +} + +/** + * Subscribes this browser and hands the endpoint to the server. + * + * Returns `false` when the deployment does not do push — `Api.vapidPublicKey` answers `null` on a + * 503 — so the caller can say "this server does not offer that" rather than "it failed". + * + * **Notification permission is not requested here.** It belongs to a click, and `Notices.tsx` + * already owns that; `subscribe` with `userVisibleOnly` would raise the prompt itself, from + * whatever code path happened to call it. Asked for from a settings screen it is a question; + * raised from a replay after a reconnection it is an ambush. + */ +export async function enablePush(api: PushApi, key: string | null): Promise { + const registration = await worker(); + if (!registration) return false; + + // `null` is this deployment answering 503 on the key route: it does not do push. Fetched by the + // caller rather than here, because the route is unsigned and `Api` exposes it as a static — + // and because this module stays free of `api.ts`, which it cannot import. See `PushApi`. + if (key === null) return false; + + const subscription = + (await registration.pushManager.getSubscription()) ?? + (await registration.pushManager.subscribe({ + // Required by every browser that implements this, and it is not a formality: it is the + // promise that every wake-up produces something the user sees. A silent push is what a + // tracker would want, and the worker keeps that promise by always showing a notification. + userVisibleOnly: true, + applicationServerKey: decodeApplicationServerKey(key), + })); + + await api.setPushToken(PROVIDER, subscription.endpoint); + return true; +} + +/** + * Unsubscribes, and tells the server to forget the address. + * + * Both halves, in that order, and neither is enough alone: dropping the local subscription while + * the server keeps the endpoint leaves it pushing into a void until the service reports it gone, + * and forgetting it server-side while the browser stays subscribed leaves a live subscription + * nobody uses. + */ +export async function disablePush(api: PushApi): Promise { + const registration = await worker(); + const subscription = await registration?.pushManager.getSubscription(); + + await subscription?.unsubscribe(); + await api.forgetPushToken(); +} + +/** + * Re-sends the endpoint this browser already holds, if any. + * + * Called at every start and after every reconnection, which is what the roadmap asks for: a push + * address rotates without warning, and a browser that re-subscribes on its own would otherwise be + * reachable at an address the server does not have. Doing nothing when there is no subscription + * is the point — this must never turn the feature on by itself. + * + * Silent on failure. It runs on a path nobody asked for; a toast here would report a problem the + * user did not cause and cannot act on. + */ +export async function replayPushToken(api: PushApi): Promise { + try { + if (!pushSupported()) return; + + const registration = await navigator.serviceWorker.getRegistration("/"); + const subscription = await registration?.pushManager.getSubscription(); + if (!subscription) return; + + await api.setPushToken(PROVIDER, subscription.endpoint); + } catch (error) { + console.warn("wake address not re-registered", error); + } +} diff --git a/apps/web/src/lib/session.ts b/apps/web/src/lib/session.ts index 91354593..5ca1edab 100644 --- a/apps/web/src/lib/session.ts +++ b/apps/web/src/lib/session.ts @@ -17,6 +17,7 @@ import { type AttachmentRef, downloadAndDecrypt, encryptAndUpload } from "./atta import * as content from "./content"; import * as envelope from "./envelope"; import { expiryOf, prune } from "./expiry.ts"; +import { disablePush, enablePush, replayPushToken } from "./push.ts"; import { type Cached, decodeHistory } from "./history"; import { PINNED_LOG_KEY } from "./pinning"; import * as derive from "./conversation-view.ts"; @@ -1180,6 +1181,40 @@ export class Session { return enablePasskeyRecovery(this.api, this.accountId, this.handle, this.account.exportSeed()); } + /** + * Subscribes this browser to wake-ups, and hands the address to the server. + * + * `false` means the deployment does not do push, not that something failed — the screen says so + * rather than flipping a switch back with no explanation. + * + * Per browser, deliberately, and therefore not a preference: it is a fact about this machine, + * which is the test `signal-sync.ts` states for what does and does not sync between an account's + * devices. Nothing here is stored or announced. + */ + async enableWaking(): Promise { + // The key is read here rather than inside `enablePush`: the route is unsigned, `Api` exposes + // it as a static, and `lib/push.ts` deliberately imports nothing from `api.ts` so that it can + // be tested without a browser. + return enablePush(this.api, await Api.vapidPublicKey()); + } + + /** Unsubscribes this browser and drops the address the server holds. */ + disableWaking(): Promise { + return disablePush(this.api); + } + + /** + * Re-sends the wake address this browser already holds. + * + * At every start and after every reconnection: a push address rotates without warning, and a + * browser that renewed its subscription on its own would otherwise be reachable at an address + * the server does not have. Does nothing when there is no subscription — this must never turn + * waking on by itself. + */ + replayWaking(): Promise { + return replayPushToken(this.api); + } + /** Removes a recovery factor. Removing one that is not there is a success. */ async forgetRecovery(kind: RecoveryKind): Promise { await this.api.forgetRecovery(kind); diff --git a/crates/server/Cargo.toml b/crates/server/Cargo.toml index d97d70a1..484bf7b4 100644 --- a/crates/server/Cargo.toml +++ b/crates/server/Cargo.toml @@ -22,6 +22,15 @@ thiserror.workspace = true rand_core.workspace = true transparency = { path = "../transparency" } hmac.workspace = true +# P-256, for the VAPID tokens that authenticate this deployment to a push service. `ecdsa` only: +# no key agreement is needed here, because a wake-up carries no payload and therefore nothing to +# encrypt — see `crate::vapid` for how much of Web Push that removes. +p256 = { version = "0.13", features = ["ecdsa"] } +# **The server's first outbound HTTP client**, and it stays optional in the sense that matters: no +# request is made unless a deployment turns push on. `rustls` rather than the platform TLS, like +# sqlx, so the binary depends on no system OpenSSL; no cookie store, no redirect following — a +# push service that answers with a redirect is not one to follow with a bearer token attached. +reqwest = { version = "0.12", default-features = false, features = ["rustls-tls"] } tokio = { version = "1", features = ["macros", "rt-multi-thread", "signal", "sync", "time"] } # Already in the tree through axum; declared here because the broadcast hub uses them # directly. diff --git a/crates/server/migrations/0020_vapid.sql b/crates/server/migrations/0020_vapid.sql new file mode 100644 index 00000000..42c587e1 --- /dev/null +++ b/crates/server/migrations/0020_vapid.sql @@ -0,0 +1,40 @@ +-- The deployment's VAPID key pair, for Web Push. +-- +-- # Why the server generates it instead of being handed it +-- +-- Because the alternative is worse in a way that is easy to miss. A `VAPID_PRIVATE_KEY` variable +-- means every operator has to produce a P-256 key in the one encoding this server accepts, which +-- in practice means an `openssl` incantation copied from somewhere, and a private key travelling +-- through a shell history and an environment file on its way in. It also means a deployment that +-- rotates the key by accident silently invalidates every subscription it ever handed out, with no +-- error anywhere — the push services simply start refusing. +-- +-- `log_key` in `0006_transparency.sql` already solved this shape: a key the server needs, creates +-- once, and never twice. The same table, for the same reason. +-- +-- # What is stored, and what is not +-- +-- The private scalar, thirty-two bytes. The public half is derived from it on demand rather than +-- stored beside it: two copies of one key pair is one copy that can be wrong, and the derivation +-- costs a multiplication on a path that runs once per push service per few hours. +-- +-- # This table existing does not turn push on +-- +-- The key is created on every start, like the log's, because a key that appears only once +-- configuration is present is a key that appears on a path nobody tested. What turns push on is +-- `VAPID_SUBJECT`: with no subject the waker stays `Silent` and this row is unused. That is the +-- second of the three limits written into `0011_push.sql` — inert without configuration — and it +-- is enforced in `crate::push`, not here. +CREATE TABLE vapid_key ( + id BOOLEAN PRIMARY KEY DEFAULT TRUE, + signing_key BYTEA NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + + -- One row, always. The same guard `log_key` carries: two keys would mean two identities + -- offered to the same push service, and subscriptions minted under the older one would start + -- being refused with nothing to say why. + CONSTRAINT vapid_key_is_singleton CHECK (id IS TRUE), + -- A P-256 private scalar. A wrong length here is a key that cannot sign, and finding that out + -- at the first wake-up means finding it out on a path nobody is watching. + CONSTRAINT vapid_signing_key_is_p256 CHECK (octet_length(signing_key) = 32) +); diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index 343b4bd5..9b6b8d23 100644 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -26,6 +26,7 @@ pub mod routes; pub mod storage; pub mod stream; pub mod throttle; +pub mod vapid; use std::sync::Arc; @@ -74,6 +75,13 @@ pub struct AppState { /// that talks to neither Apple nor Google must stay fully functional. See `push` for what /// this wake-up costs in metadata. pub push: Arc, + /// The VAPID public key this deployment advertises, when push is on. + /// + /// `None` is what an unconfigured deployment carries, and the route that serves it answers + /// 503 rather than 404 — the distinction `call.rs` already draws: the client reads it as "this + /// deployment does not offer that" and hides the control, instead of retrying something no + /// retry fixes. + pub push_public_key: Option, /// Where calls are relayed, if anywhere. /// /// Empty by default, and for the reason the waker above is silent by default: a deployment @@ -106,6 +114,15 @@ impl FromRef for Arc { } } +/// Cloned as an `Option` rather than reached through the pool: it is read on a route that +/// runs once per client start-up, and a query there would be a query for a value that cannot +/// change while the process lives. +impl FromRef for Option { + fn from_ref(state: &AppState) -> Self { + state.push_public_key.clone() + } +} + impl FromRef for Arc { fn from_ref(state: &AppState) -> Self { state.media.clone() @@ -655,6 +672,21 @@ pub fn app_with_waker( pool: PgPool, limits: throttle::Limits, push: Arc, +) -> axum::Router { + // No advertised key: a test substituting a waker is checking what this server *sends*, and a + // deployment that means to be subscribed to goes through `app_with_push`. + app_with_push(pool, limits, push::Configured { waker: push, public_key: None }) +} + +/// The application, with whatever `push::from_environment` settled on. +/// +/// The waker and the key it advertises arrive together, because a deployment that advertises one +/// key and signs with another mints subscriptions that are refused later — see +/// [`push::Configured`]. +pub fn app_with_push( + pool: PgPool, + limits: throttle::Limits, + push: push::Configured, ) -> axum::Router { use tower_http::limit::RequestBodyLimitLayer; use tower_http::trace::TraceLayer; @@ -670,9 +702,10 @@ pub fn app_with_waker( writes: Arc::new(limits.writes), recovery: Arc::new(limits.recovery), storage: Arc::new(limits.storage), - // The default is `Silent`, set by `app_with`: wiring up Apple or Google requires secrets - // a deployment must provide knowingly, after reading what the wake-up leaks. - push, + // The default is `Silent`, set by `app_with`: waking a browser requires a deployment to + // name a contact knowingly, after reading what the wake-up leaks. + push: push.waker, + push_public_key: push.public_key, // Read from the environment rather than passed in, unlike the waker: there is nothing to // substitute here. The tests that matter check what this server *sends* — a token's // contents, a refusal when unconfigured — and both are reachable without a media server. diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 2c074982..8a2a5c32 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -16,6 +16,12 @@ async fn main() -> Result<(), Box> { .parse()?; let pool = server::connect(&database_url).await?; + + // Read after the pool exists, because the key it may load lives in the database. Absent + // configuration this is `Silent` and no advertised key, which is the behaviour of a + // deployment that talks to no push service and must stay fully functional — see `push`. + let push = server::push::from_environment(&pool).await; + let listener = tokio::net::TcpListener::bind(addr).await?; tracing::info!(%addr, "delivery service listening"); @@ -23,7 +29,9 @@ async fn main() -> Result<(), Box> { // `into_make_service_with_connect_info` rather than the bare service: without it, the rate // limit's `ConnectInfo` extractor fails and **every** open route returns an internal error. // A one-line omission takes the whole signup path down. - axum::serve(listener, server::app(pool).into_make_service_with_connect_info::()) + let app = server::app_with_push(pool, server::throttle::Limits::from_environment(), push); + + axum::serve(listener, app.into_make_service_with_connect_info::()) .with_graceful_shutdown(async { let _ = tokio::signal::ctrl_c().await; }) diff --git a/crates/server/src/push.rs b/crates/server/src/push.rs index a3f1231b..2e5e6e3b 100644 --- a/crates/server/src/push.rs +++ b/crates/server/src/push.rs @@ -63,6 +63,250 @@ impl Waker for Silent { } } +/// What a deployment ended up with: something to wake devices, and the key to advertise. +/// +/// The two travel together because they must agree. A client subscribes against the public key it +/// is given and the push service binds the subscription to it; a deployment advertising one key +/// and signing with another produces subscriptions that are refused later, on a path nobody +/// watches. Handing both out of one function makes disagreeing impossible. +pub struct Configured { + pub waker: Arc, + /// `None` when push is off. The route that serves it answers 503 in that case — the same + /// distinction calls make, and for the same reason: a client reads it and hides the control + /// instead of offering a subscription that would never be woken. + pub public_key: Option, +} + +/// Reads the configuration, and is content to find none. +/// +/// # What turns push on +/// +/// `VAPID_SUBJECT`, and nothing else. It is the contact a push service is told to reach if this +/// deployment misbehaves — RFC 8292 wants a `mailto:` or an `https:` URL — and it doubles as the +/// switch because there is nothing else a deployment must supply: the key pair is the server's +/// own, created on first start (`migrations/0020_vapid.sql`). One variable, one meaning. +/// +/// Unset, this returns [`Silent`] and no public key. That is the second of the three limits in +/// `migrations/0011_push.sql`: a deployment that talks to nobody records tokens, sends nothing, +/// and stays fully functional. `without_a_provider_nothing_is_sent` pins the behaviour. +pub async fn from_environment(pool: &PgPool) -> Configured { + let subject = std::env::var("VAPID_SUBJECT").ok().filter(|value| !value.is_empty()); + + let Some(subject) = subject else { + return Configured { waker: Arc::new(Silent), public_key: None }; + }; + + // A subject that is neither of the two forms RFC 8292 allows is refused here rather than at + // the first wake-up: services reject the token, and the symptom would be a feature that + // registers subscriptions and silently never delivers. + if !subject.starts_with("mailto:") && !subject.starts_with("https://") { + tracing::warn!("VAPID_SUBJECT must be a mailto: or https: URL; push stays off"); + return Configured { waker: Arc::new(Silent), public_key: None }; + } + + match crate::vapid::Key::ensure(pool).await { + Ok(key) => { + let public_key = key.public_key(); + tracing::info!(%subject, "web push enabled"); + Configured { + waker: Arc::new(Vapid::new(pool.clone(), key, subject)), + public_key: Some(public_key), + } + } + Err(error) => { + // Refusing to start would take a working messenger down over a feature that is + // optional by design. Off, loudly, is the honest answer. + tracing::warn!(?error, "cannot load the VAPID key; push stays off"); + Configured { waker: Arc::new(Silent), public_key: None } + } + } +} + +/// How long a push service may hold a wake-up for a device that is offline. +/// +/// Four hours. A wake-up is worth delivering late — somebody opening their phone at lunch should +/// learn that a message arrived at breakfast — but not indefinitely: past a point the application +/// will have polled and the notification would announce something already read. The header is +/// mandatory in RFC 8030; omitting it means the service picks, and the services do not agree. +const WAKE_TTL_SECONDS: u32 = 4 * 3600; + +/// The provider name a Web Push subscription is stored under. +/// +/// A string rather than an enum, because that is what the column holds and what the client sends. +/// The match in [`Vapid::wake`] is what gives it meaning, and an unknown value is skipped rather +/// than guessed at — a token whose provider this build does not know is a token for a provider +/// somebody is in the middle of adding. +pub const WEB_PUSH: &str = "webpush"; + +/// Wakes browsers, over Web Push. +/// +/// # Why this holds a pool +/// +/// Because a subscription dies without telling anybody. A browser drops it when the user clears +/// site data or the profile moves, and the only signal is the push service answering `404` or +/// `410` on the next attempt. Nothing else in this server would ever remove that row, so the +/// table would grow with every browser that ever subscribed and every wake-up would carry a +/// growing tail of requests that cannot succeed. Cleaning up needs the database, so the waker +/// holds it. [`Silent`] holds nothing and stays what it was. +/// +/// # Why the trait did not change +/// +/// [`Waker::wake`] is synchronous and this work is not. It could have become async — and then +/// every implementation, including the one that does nothing, would carry a boxed future for the +/// benefit of one of them. It is already called from inside a `tokio::spawn` (see +/// [`wake_detached`]), so spawning again here is a task inside a task and costs nothing anybody +/// can measure. The signature that cannot carry content stays exactly as it was, which is the +/// property worth protecting. +pub struct Vapid { + pool: PgPool, + key: crate::vapid::Key, + /// The contact a push service is told to reach if this deployment misbehaves. RFC 8292 asks + /// for a `mailto:` or an `https:` URL, and it is what turns push on: with no subject + /// configured, `Silent` is used instead and this type is never built. + subject: String, + http: reqwest::Client, + /// One signed token per push service, reused until it nears expiry. + /// + /// A `Mutex` over a small map rather than anything cleverer: it is held for the length of a + /// clone, contended by at most a handful of wake-ups, and the alternative — signing per + /// address — turns a constant cost into one that grows with the size of the room. + tokens: std::sync::Mutex>, +} + +impl Vapid { + pub fn new(pool: PgPool, key: crate::vapid::Key, subject: String) -> Self { + Self { + pool, + key, + subject, + // No cookie store and no redirect following. A push endpoint that answers with a + // redirect is not one to follow carrying a bearer token: the token is signed for the + // origin that was asked, and following would present it to another. + http: reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .timeout(std::time::Duration::from_secs(10)) + .build() + .expect("a client with no TLS backend would fail here, at start-up"), + tokens: std::sync::Mutex::new(std::collections::HashMap::new()), + } + } + + /// The token for one push service, minting a new one only when the held one is running out. + fn token_for(&self, audience: &str) -> String { + let mut tokens = self.tokens.lock().unwrap_or_else(|error| error.into_inner()); + + if let Some(held) = tokens.get(audience) + && held.usable() + { + return held.token.clone(); + } + + let fresh = crate::vapid::Cached::mint(&self.key, audience, &self.subject); + let token = fresh.token.clone(); + tokens.insert(audience.to_owned(), fresh); + token + } + + /// Pushes to one endpoint, and reports whether the subscription is gone for good. + async fn push_one(&self, endpoint: &str) -> Outcome { + let Some(audience) = crate::vapid::audience_of(endpoint) else { + // Not a URL this server will sign a credential for. Stored rather than sent, which + // means it came from a client that sent something odd — dropping it is the only + // action that ends the situation. + tracing::warn!("a stored subscription is not an http(s) endpoint; dropping it"); + return Outcome::Gone; + }; + + let request = self + .http + .post(endpoint) + .header( + "Authorization", + format!("vapid t={}, k={}", self.token_for(&audience), self.key.public_key()), + ) + .header("TTL", WAKE_TTL_SECONDS.to_string()) + // **No body, and this is the line to read twice.** Everything else in Web Push — the + // `Content-Encoding: aes128gcm`, the two subscription secrets, the whole of RFC 8291 — + // exists to carry one past a service that must not read it. There is nothing to carry: + // the wake-up says "wake up". `Content-Length: 0` is explicit so that a proxy inserting + // a body would be the one lying, not this. + .header("Content-Length", "0"); + + match request.send().await { + // 404 and 410 both mean the browser threw this subscription away. Every other status + // is this deployment's problem or the service's, and the row stays. + Ok(response) if response.status() == 404 || response.status() == 410 => Outcome::Gone, + Ok(response) if response.status().is_success() => Outcome::Delivered, + Ok(response) => { + tracing::warn!(status = %response.status(), "push service refused a wake-up"); + Outcome::Failed + } + Err(error) => { + tracing::warn!(?error, "cannot reach a push service"); + Outcome::Failed + } + } + } +} + +/// What one attempt settled. +#[derive(Debug, PartialEq, Eq)] +enum Outcome { + Delivered, + /// The subscription no longer exists. The row goes. + Gone, + /// Something else. The row stays: a service that is down comes back, and dropping a live + /// subscription over a bad afternoon would silence a device for good. + Failed, +} + +impl Waker for Vapid { + fn wake(&self, addresses: Vec
) { + // The pool, the key and the client are all cheap to clone or already behind an `Arc`; + // what cannot be cloned is `self`, so the work moves into a task that owns what it needs. + // See the type's own note on why the trait stayed synchronous. + let waker = Vapid { + pool: self.pool.clone(), + key: self.key.clone(), + subject: self.subject.clone(), + http: self.http.clone(), + // Deliberately not shared with the parent. The task lives for one wake-up, so at most + // one signature per service is minted and thrown away — measurably nothing, against a + // `Mutex` shared across tasks for the lifetime of the process. + tokens: std::sync::Mutex::new(std::collections::HashMap::new()), + }; + + tokio::spawn(async move { + for address in addresses { + if address.provider != WEB_PUSH { + // Neither an error nor a silence: a row for a provider this build cannot speak + // to is what an FCM or APNs token looks like before its provider lands. + tracing::debug!(provider = %address.provider, "no provider for this token"); + continue; + } + + if waker.push_one(&address.token).await == Outcome::Gone + && let Err(error) = forget_token(&waker.pool, &address.token).await + { + tracing::warn!(?error, "cannot drop a dead subscription"); + } + } + }); + } +} + +/// Drops a subscription the push service says no longer exists. +/// +/// By token and not by device: that is all a wake-up carries. A device that re-subscribes writes a +/// new row through [`register`] anyway, so there is nothing to preserve here. +async fn forget_token(pool: &PgPool, token: &str) -> sqlx::Result<()> { + sqlx::query("DELETE FROM push_tokens WHERE token = $1") + .bind(token) + .execute(pool) + .await + .map(|_| ()) +} + /// Records or replaces a device's token. /// /// Replacement is the rule: providers rotate their tokens without warning, and keeping the old diff --git a/crates/server/src/routes.rs b/crates/server/src/routes.rs index 4757c116..a654fba6 100644 --- a/crates/server/src/routes.rs +++ b/crates/server/src/routes.rs @@ -77,6 +77,7 @@ pub fn public_router(state: AppState) -> Router { .route("/v1/accounts", post(create_account)) .route("/v1/devices", post(register_device)) .route("/v1/pairings/{pairing_id}", post(deposit_pairing).get(claim_pairing)) + .route("/v1/push/vapid", get(vapid_public_key)) .route("/v1/recovery/claim", post(claim_recovery)) .with_state(state) } @@ -2409,6 +2410,30 @@ async fn forget_push_token(State(pool): State, signed: Signed) -> ApiRes Ok(()) } +/// The VAPID public key a browser needs in order to subscribe. +/// +/// # Why this is a route and not a build-time value +/// +/// Because a key baked into the client bundle is a key that needs a rebuild to change, and this +/// project has already been caught by exactly that shape: `VITE_LOG_PUBKEY` was documented as +/// "printed by the server on first boot" while nothing printed it. A subscription is bound to the +/// key it was created under, so the one place that cannot disagree with the signer is the signer. +/// +/// # Why it is open +/// +/// It is a public key, and it is needed before anything is subscribed. It carries no information +/// about the accounts on this deployment — only that it has push turned on, which the presence of +/// the setting reveals anyway. +/// +/// 503 rather than 404 when push is off, the distinction `call_token` already draws: a client +/// reads it as "this deployment does not offer that", hides the control, and does not retry. +async fn vapid_public_key( + State(public_key): State>, +) -> ApiResult> { + let key = public_key.ok_or(ApiError::Unavailable)?; + Ok(Json(serde_json::json!({ "key": key }))) +} + async fn post_envelope( State(pool): State, State(hub): State>, diff --git a/crates/server/src/vapid.rs b/crates/server/src/vapid.rs new file mode 100644 index 00000000..ae3f59a7 --- /dev/null +++ b/crates/server/src/vapid.rs @@ -0,0 +1,249 @@ +//! VAPID: how this server proves to a push service that it is the one the browser subscribed to. +//! +//! # What this is, in one paragraph +//! +//! A browser subscribes and gets back an endpoint URL belonging to its own vendor — Google for +//! Chrome, Mozilla for Firefox. Anybody who learns that URL could push to it, so RFC 8292 has the +//! application server sign a short-lived JWT with a P-256 key and send the matching public key +//! alongside. The subscription was minted against that same public key, so the service can check +//! that the sender is the party the browser agreed to hear from. That is the whole mechanism. +//! +//! # Why there is no payload encryption in this file +//! +//! Because there is no payload. RFC 8291 — `aes128gcm`, the `p256dh` and `auth` secrets, the +//! whole content-encryption half of Web Push — exists to carry a body past a service that must +//! not read it. This server has no body to carry: the wake-up says "wake up" and nothing else, +//! which is the third of the three limits in `migrations/0011_push.sql` and the property +//! `push::the_wake_up_only_carries_addresses` freezes. +//! +//! So a subscription here is one string, the endpoint, and it fits `push::Address` unchanged. It +//! is worth noticing how much of the specification that removes, and worth not quietly adding it +//! back: a payload would need the two subscription secrets, a migration to hold them, and an +//! encryption path — to send a preview to a lock screen, which is what this project exists not to +//! do. +//! +//! # What a push service still learns +//! +//! When this deployment wakes a device, and how often. That is irreducible and is stated in +//! `crate::push`; nothing in this file improves it. + +use std::time::{SystemTime, UNIX_EPOCH}; + +use base64::Engine as _; +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use p256::ecdsa::{Signature, SigningKey, signature::Signer}; +use sqlx::PgPool; + +/// How long a signed token stays valid. +/// +/// RFC 8292 caps this at twenty-four hours and services enforce it. Well below that on purpose: +/// the token is a bearer credential for pushing to every subscription of one service, and it +/// travels to a third party on every wake-up. Twelve hours would halve the signatures and double +/// the window a captured token is useful for, which is the wrong side of that trade for something +/// this cheap to mint. +const TOKEN_TTL_SECONDS: u64 = 3600; + +/// Re-sign once a token is this close to expiring. +/// +/// Without the margin, a token minted at the edge of validity is refused by a service whose clock +/// runs slightly ahead — and the failure appears as a `401` on a wake-up nobody is watching. +const REFRESH_MARGIN_SECONDS: u64 = 300; + +/// The deployment's key pair. +#[derive(Clone)] +pub struct Key { + signing: SigningKey, +} + +impl Key { + /// Loads the key, creating it on first start. + /// + /// `ON CONFLICT DO NOTHING` rather than a read-then-write, for the reason `log:: + /// ensure_signing_key` gives: two processes starting together would otherwise mint two keys, + /// and a subscription is bound to the key it was created under — half of them would start + /// being refused by the push service, with nothing in the logs to connect the two facts. + pub async fn ensure(pool: &PgPool) -> sqlx::Result { + let fresh = SigningKey::random(&mut rand_core::OsRng); + + sqlx::query("INSERT INTO vapid_key (id, signing_key) VALUES (TRUE, $1) ON CONFLICT DO NOTHING") + .bind(fresh.to_bytes().as_slice()) + .execute(pool) + .await?; + + let (stored,): (Vec,) = + sqlx::query_as("SELECT signing_key FROM vapid_key WHERE id = TRUE") + .fetch_one(pool) + .await?; + + let signing = SigningKey::from_slice(&stored).expect("vapid_signing_key_is_p256 constraint"); + + Ok(Self { signing }) + } + + /// The public half, as a push service expects it: uncompressed SEC1, base64url, unpadded. + /// + /// This is also exactly what a browser wants for `applicationServerKey`, which is why the + /// client reads it from this server rather than carrying a build-time copy: a key baked into + /// a bundle is a key that needs a rebuild to change, and one of those has already caught this + /// project out. + pub fn public_key(&self) -> String { + URL_SAFE_NO_PAD.encode(self.signing.verifying_key().to_encoded_point(false).as_bytes()) + } + + /// Signs a token for one push service. + /// + /// The audience is the service's origin and not the endpoint: RFC 8292 says so, and it is what + /// makes one signature serve every subscription of one vendor instead of one per device. + pub fn token(&self, audience: &str, subject: &str) -> String { + let header = serde_json::json!({ "typ": "JWT", "alg": "ES256" }); + let claims = serde_json::json!({ + "aud": audience, + "exp": seconds_now() + TOKEN_TTL_SECONDS, + "sub": subject, + }); + + let signing_input = format!("{}.{}", encode_part(&header), encode_part(&claims)); + + // `Signature` is the fixed-width r‖s form, sixty-four bytes, which is what JWS ES256 is + // defined over. The DER encoding the same crate can produce is what X.509 uses and what a + // push service rejects — the two are easy to confuse and only one of them is ever right + // here. + let signature: Signature = self.signing.sign(signing_input.as_bytes()); + + format!("{signing_input}.{}", URL_SAFE_NO_PAD.encode(signature.to_bytes())) + } +} + +/// A token, and when it stops being worth reusing. +#[derive(Clone)] +pub struct Cached { + pub token: String, + expires_at: u64, +} + +impl Cached { + pub fn mint(key: &Key, audience: &str, subject: &str) -> Self { + Self { + token: key.token(audience, subject), + expires_at: seconds_now() + TOKEN_TTL_SECONDS, + } + } + + /// Whether this token can still be sent. + /// + /// Cached per service rather than per wake-up: a group of twenty devices on one vendor is one + /// signature, not twenty. An ECDSA signature is cheap, but doing it per address turns a + /// constant cost into one that grows with the size of the room. + pub fn usable(&self) -> bool { + self.expires_at > seconds_now() + REFRESH_MARGIN_SECONDS + } +} + +/// The scheme and authority of an endpoint — the `aud` a token is signed for. +/// +/// Returns `None` for anything that is not an absolute http(s) URL. That is a refusal rather than +/// a fallback: the audience ends up inside a signed credential, and guessing it wrong produces a +/// token that authenticates this deployment to a service it did not mean to talk to. +pub fn audience_of(endpoint: &str) -> Option { + let (scheme, rest) = endpoint.split_once("://")?; + if scheme != "https" && scheme != "http" { + return None; + } + + let authority = rest.split(['/', '?', '#']).next()?; + if authority.is_empty() { + return None; + } + + Some(format!("{scheme}://{authority}")) +} + +fn encode_part(value: &serde_json::Value) -> String { + URL_SAFE_NO_PAD.encode(serde_json::to_vec(value).expect("a JSON object always serialises")) +} + +fn seconds_now() -> u64 { + SystemTime::now().duration_since(UNIX_EPOCH).map(|since| since.as_secs()).unwrap_or(0) +} + +#[cfg(test)] +mod tests { + use super::*; + use p256::ecdsa::VerifyingKey; + use p256::ecdsa::signature::Verifier; + + fn key() -> Key { + Key { signing: SigningKey::random(&mut rand_core::OsRng) } + } + + /// A push service verifies the token the way this test does, and it is the whole contract. + #[test] + fn the_token_verifies_under_the_advertised_public_key() { + let key = key(); + let token = key.token("https://fcm.googleapis.com", "mailto:ops@example.test"); + + let mut parts = token.rsplitn(2, '.'); + let signature = parts.next().expect("a signature"); + let signing_input = parts.next().expect("a header and a payload"); + + // The public key is taken from the advertised form rather than from the key object: what + // the service checks against is the string in the `k=` parameter, so that is what has to + // work. Verifying against `key.signing.verifying_key()` would pass even if `public_key` + // encoded it wrongly. + let advertised = URL_SAFE_NO_PAD.decode(key.public_key()).expect("base64url"); + let verifying = VerifyingKey::from_sec1_bytes(&advertised).expect("an uncompressed point"); + let signature = Signature::from_slice(&URL_SAFE_NO_PAD.decode(signature).expect("base64url")) + .expect("sixty-four bytes"); + + verifying.verify(signing_input.as_bytes(), &signature).expect("the service would refuse it"); + } + + /// The claims are the three RFC 8292 requires, and the expiry is inside the cap services + /// enforce. + #[test] + fn the_claims_say_who_this_is_for_and_for_how_long() { + let token = key().token("https://updates.push.services.mozilla.com", "mailto:ops@example.test"); + + let payload = token.split('.').nth(1).expect("a payload"); + let claims: serde_json::Value = + serde_json::from_slice(&URL_SAFE_NO_PAD.decode(payload).expect("base64url")) + .expect("json"); + + assert_eq!(claims["aud"], "https://updates.push.services.mozilla.com"); + assert_eq!(claims["sub"], "mailto:ops@example.test"); + + let exp = claims["exp"].as_u64().expect("a number"); + assert!(exp > seconds_now(), "already expired when minted"); + assert!(exp <= seconds_now() + 86_400, "past the twenty-four hours RFC 8292 allows"); + } + + /// **The one that matters for cost.** The audience is the origin, so every subscription of one + /// vendor shares a token; keying it on the endpoint would sign once per device. + #[test] + fn every_endpoint_of_one_service_shares_an_audience() { + let one = audience_of("https://fcm.googleapis.com/fcm/send/abc?x=1"); + let other = audience_of("https://fcm.googleapis.com/fcm/send/zzz"); + + assert_eq!(one.as_deref(), Some("https://fcm.googleapis.com")); + assert_eq!(one, other); + } + + /// Anything that is not an absolute http(s) URL has no audience, and gets no token. + #[test] + fn a_thing_that_is_not_a_url_is_refused_rather_than_guessed() { + assert_eq!(audience_of("fcm.googleapis.com/send/abc"), None); + assert_eq!(audience_of("javascript://evil/#"), None); + assert_eq!(audience_of("https://"), None); + assert_eq!(audience_of(""), None); + } + + /// A fresh token is usable, and one about to expire is not. + #[test] + fn a_token_is_reused_until_it_nears_its_expiry() { + let fresh = Cached::mint(&key(), "https://example.test", "mailto:ops@example.test"); + assert!(fresh.usable()); + + let stale = Cached { token: String::new(), expires_at: seconds_now() + 1 }; + assert!(!stale.usable(), "a token this close to expiry would be refused by a fast clock"); + } +} diff --git a/crates/server/tests/common/mod.rs b/crates/server/tests/common/mod.rs index 26324680..f6a5553c 100644 --- a/crates/server/tests/common/mod.rs +++ b/crates/server/tests/common/mod.rs @@ -181,6 +181,32 @@ pub async fn start_with_waker() -> (TestServer, std::sync::Arc) { (TestServer { base_url: format!("http://{addr}"), pool }, spy) } +/// A server wired to a real push emitter, rather than to a spy. +/// +/// Distinct from [`start_with_waker`] on purpose: that one observes what the server *decides* to +/// send, this one observes what it actually puts on the wire. Both are needed — the first proves +/// the right devices are chosen, the second proves the request is one a push service would +/// accept and that it carries nothing. +pub async fn start_with_push(push: server::push::Configured) -> TestServer { + let pool = pool().await; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let app = server::app_with_push(pool.clone(), Limits::off(), push) + .into_make_service_with_connect_info::(); + + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + TestServer { base_url: format!("http://{addr}"), pool } +} + +/// The pool, for a test that has to build something needing one before the server exists. +pub async fn test_pool() -> PgPool { + pool().await +} + async fn start_with(pool: PgPool, limits: Limits) -> TestServer { let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let addr = listener.local_addr().unwrap(); diff --git a/crates/server/tests/webpush.rs b/crates/server/tests/webpush.rs new file mode 100644 index 00000000..f5488d26 --- /dev/null +++ b/crates/server/tests/webpush.rs @@ -0,0 +1,394 @@ +//! What this server actually puts on the wire when it wakes a browser. +//! +//! # Why a fake push service and not a mock +//! +//! Because the property worth checking is not "a function was called" but "the bytes leaving this +//! process are ones a push service would accept, and they carry nothing". A mock of our own +//! `Waker` would assert the first and be blind to the second — and the second is the whole +//! feature. So the double is a real HTTP server, on a real socket, reached by a real client, +//! built from the recipe `common::start_with` already uses. +//! +//! It is also what makes the endpoint override unnecessary: a Web Push subscription **is** a URL, +//! stored as the token, so pointing the server at this double is just registering it as a +//! subscription. Nothing in production code exists for the benefit of these tests. +//! +//! # What no test here can establish +//! +//! That Google and Mozilla accept these tokens. This checks the token against the specification +//! and against the public key advertised beside it; the remaining risk is a service disagreeing +//! with our reading of RFC 8292, and only a real subscription settles that. `docs/ROADMAP.md` +//! says so rather than leaving it implied. + +mod common; + +use std::sync::{Arc, Mutex}; + +use base64::Engine as _; +use base64::engine::general_purpose::{STANDARD as BASE64_STANDARD, URL_SAFE_NO_PAD}; +use common::{Device, unique}; +use server::push::{Configured, Vapid, WEB_PUSH}; + +/// One request as the push service saw it. +#[derive(Clone)] +struct Received { + authorization: String, + ttl: String, + body: Vec, + path: String, +} + +/// A push service that records what it is sent and answers with the status it was told to. +struct FakeService { + origin: String, + seen: Arc>>, +} + +impl FakeService { + async fn start(status: u16) -> Self { + use axum::extract::{Path, State}; + use axum::http::HeaderMap; + use axum::routing::post; + + let seen: Arc>> = Arc::new(Mutex::new(Vec::new())); + + let app = axum::Router::new() + .route( + "/push/{id}", + post( + |State((seen, status)): State<(Arc>>, u16)>, + Path(id): Path, + headers: HeaderMap, + body: axum::body::Bytes| async move { + let header = |name: &str| { + headers + .get(name) + .and_then(|value| value.to_str().ok()) + .unwrap_or_default() + .to_owned() + }; + + seen.lock().unwrap().push(Received { + authorization: header("authorization"), + ttl: header("ttl"), + body: body.to_vec(), + path: format!("/push/{id}"), + }); + + axum::http::StatusCode::from_u16(status).unwrap() + }, + ), + ) + .with_state((seen.clone(), status)); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + Self { origin: format!("http://{addr}"), seen } + } + + fn endpoint(&self, id: &str) -> String { + format!("{}/push/{id}", self.origin) + } + + /// Waits for a request, because the wake-up is detached and nothing on the HTTP response + /// depends on it. Same shape as `WakerSpy::wait_for`, and for the same reason. + async fn wait_for_one(&self) -> Option { + for _ in 0..100 { + if let Some(first) = self.seen.lock().unwrap().first() { + return Some(first.clone()); + } + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + None + } + + fn count(&self) -> usize { + self.seen.lock().unwrap().len() + } +} + +const SUBJECT: &str = "mailto:ops@example.test"; + +/// Stands up a server that really emits Web Push, and hands back the advertised key. +async fn server_with_web_push(pool: sqlx::PgPool) -> (common::TestServer, String) { + let key = server::vapid::Key::ensure(&pool).await.unwrap(); + let public_key = key.public_key(); + + let push = Configured { + waker: Arc::new(Vapid::new(pool.clone(), key, SUBJECT.into())), + public_key: Some(public_key.clone()), + }; + + (common::start_with_push(push).await, public_key) +} + +/// Registers a device, puts it in a group with a sender, and subscribes it to `endpoint`. +async fn subscribed_pair(server: &common::TestServer, endpoint: &str) -> (Device, Vec) { + let alice = Device::register(server, &unique("alice")).await; + let bob = Device::register(server, &unique("bob")).await; + + let group_id = unique("group").into_bytes(); + alice + .post( + &format!("/v1/groups/{}/members", hex::encode(&group_id)), + serde_json::json!({ "device_ids": [alice.id, bob.id] }), + ) + .await; + + let registered = bob + .post( + "/v1/push/token", + serde_json::json!({ "provider": WEB_PUSH, "token": endpoint }), + ) + .await; + assert_eq!(registered.status(), 200); + + (alice, group_id) +} + +async fn post_a_message(sender: &Device, group_id: &[u8]) { + sender + .post( + &format!("/v1/groups/{}/envelopes", hex::encode(group_id)), + serde_json::json!({ "payload": BASE64_STANDARD.encode([7u8]) }), + ) + .await; +} + +/// **The test the whole feature rests on.** +/// +/// Three things at once, because they are one claim: the request is authenticated the way RFC +/// 8292 defines, the token verifies under the key this deployment advertises, and the body is +/// empty. The third is checked at the wire rather than inferred from `Waker`'s signature — the +/// signature stops a *parameter* being added, not a body being written here. +#[tokio::test] +async fn the_wake_up_is_signed_for_the_service_and_carries_nothing() { + let pool = common::test_pool().await; + let service = FakeService::start(201).await; + let (server, advertised) = server_with_web_push(pool).await; + + let (alice, group_id) = subscribed_pair(&server, &service.endpoint("abc")).await; + post_a_message(&alice, &group_id).await; + + let seen = service.wait_for_one().await.expect("the service was never called"); + + assert!(seen.body.is_empty(), "a wake-up must carry nothing, got {} bytes", seen.body.len()); + assert_eq!(seen.path, "/push/abc", "the endpoint's own path must be preserved"); + assert_eq!(seen.ttl, "14400"); + + // `vapid t=, k=` — the two parameters RFC 8292 defines. + let rest = seen.authorization.strip_prefix("vapid t=").expect("a vapid authorization"); + let (jwt, key_part) = rest.split_once(", k=").expect("both parameters"); + + assert_eq!( + key_part, advertised, + "the key sent to the service is not the one clients are told to subscribe against" + ); + + // Verified exactly as the service would: against the advertised key, not against anything + // this process kept a handle on. + use p256::ecdsa::signature::Verifier; + let (signing_input, signature) = jwt.rsplit_once('.').expect("a signature"); + let verifying = p256::ecdsa::VerifyingKey::from_sec1_bytes( + &URL_SAFE_NO_PAD.decode(key_part).expect("base64url"), + ) + .expect("an uncompressed point"); + let signature = p256::ecdsa::Signature::from_slice( + &URL_SAFE_NO_PAD.decode(signature).expect("base64url"), + ) + .expect("sixty-four bytes"); + verifying + .verify(signing_input.as_bytes(), &signature) + .expect("the push service would refuse this token"); + + let claims: serde_json::Value = serde_json::from_slice( + &URL_SAFE_NO_PAD + .decode(signing_input.split('.').nth(1).expect("a payload")) + .expect("base64url"), + ) + .expect("json"); + + assert_eq!(claims["aud"], service.origin, "signed for a different service than the one called"); + assert_eq!(claims["sub"], SUBJECT); +} + +/// A subscription the service says is gone is removed, rather than retried forever. +/// +/// Nothing else in this server would ever delete that row: a browser drops a subscription without +/// telling anybody, and `410` is the only notice there is. Left in place, every future wake-up +/// would carry a growing tail of requests that cannot succeed. +#[tokio::test] +async fn a_subscription_the_service_has_dropped_is_forgotten() { + let pool = common::test_pool().await; + let service = FakeService::start(410).await; + let (server, _) = server_with_web_push(pool).await; + + let endpoint = service.endpoint("gone"); + let (alice, group_id) = subscribed_pair(&server, &endpoint).await; + post_a_message(&alice, &group_id).await; + + service.wait_for_one().await.expect("the service was never called"); + + // The delete happens after the response, so it is worth a few attempts rather than one read. + for _ in 0..100 { + let (rows,): (i64,) = + sqlx::query_as("SELECT count(*)::bigint FROM push_tokens WHERE token = $1") + .bind(&endpoint) + .fetch_one(&server.pool) + .await + .unwrap(); + + if rows == 0 { + return; + } + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + + panic!("a subscription the service reported gone is still stored"); +} + +/// A service having a bad day keeps its subscriptions. +/// +/// The opposite policy is tempting and wrong: dropping a live subscription over one `500` would +/// silence a device permanently, and the device has no way to know it should re-subscribe. +#[tokio::test] +async fn a_failing_service_does_not_cost_the_subscription() { + let pool = common::test_pool().await; + let service = FakeService::start(500).await; + let (server, _) = server_with_web_push(pool).await; + + let endpoint = service.endpoint("flaky"); + let (alice, group_id) = subscribed_pair(&server, &endpoint).await; + post_a_message(&alice, &group_id).await; + + service.wait_for_one().await.expect("the service was never called"); + tokio::time::sleep(std::time::Duration::from_millis(200)).await; + + let (rows,): (i64,) = + sqlx::query_as("SELECT count(*)::bigint FROM push_tokens WHERE token = $1") + .bind(&endpoint) + .fetch_one(&server.pool) + .await + .unwrap(); + + assert_eq!(rows, 1, "a transient failure must not unsubscribe a device"); +} + +/// A token for a provider this build cannot speak to is skipped, not attempted. +/// +/// That is what an FCM or APNs token looks like today: the column accepts any provider name — the +/// server has nothing to decide there — and the emitter simply has no way to reach it yet. +#[tokio::test] +async fn a_token_for_another_provider_is_left_alone() { + let pool = common::test_pool().await; + let service = FakeService::start(201).await; + let (server, _) = server_with_web_push(pool).await; + + let alice = Device::register(&server, &unique("alice")).await; + let bob = Device::register(&server, &unique("bob")).await; + let group_id = unique("group").into_bytes(); + alice + .post( + &format!("/v1/groups/{}/members", hex::encode(&group_id)), + serde_json::json!({ "device_ids": [alice.id, bob.id] }), + ) + .await; + + // An endpoint that would work, under a provider name that is not this one. + bob.post( + "/v1/push/token", + serde_json::json!({ "provider": "fcm", "token": service.endpoint("nope") }), + ) + .await; + + post_a_message(&alice, &group_id).await; + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + + assert_eq!(service.count(), 0, "a webpush request was sent for an fcm token"); +} + +/// The advertised key is served to clients, and it is the one that signs. +/// +/// Checked through the route rather than through the struct: a client subscribes against what the +/// route returns, so a deployment whose route disagreed with its signer would mint subscriptions +/// that are refused later, on a path nobody watches. +#[tokio::test] +async fn the_route_serves_the_key_that_signs() { + let pool = common::test_pool().await; + let (server, advertised) = server_with_web_push(pool).await; + + let response = reqwest::get(format!("{}/v1/push/vapid", server.base_url)).await.unwrap(); + assert_eq!(response.status(), 200); + + let body: serde_json::Value = response.json().await.unwrap(); + assert_eq!(body["key"], advertised); + + // And it is a point, not a string that happens to be there. + let raw = URL_SAFE_NO_PAD.decode(body["key"].as_str().unwrap()).unwrap(); + assert_eq!(raw.len(), 65, "an uncompressed P-256 point is sixty-five bytes"); + assert_eq!(raw[0], 0x04, "not in uncompressed form; a browser would refuse it"); +} + +/// With push off, the route says the deployment does not offer this — not that it is missing. +/// +/// 503 and not 404, the distinction calls already draw: the client hides the control instead of +/// retrying something no retry fixes. +#[tokio::test] +async fn the_key_route_is_unavailable_when_push_is_off() { + let server = common::start().await; + + let response = reqwest::get(format!("{}/v1/push/vapid", server.base_url)).await.unwrap(); + + assert_eq!(response.status(), 503); +} + +/// **The one thing the double above cannot settle: does a real push service accept our token?** +/// +/// Ignored by default, because it needs a live subscription and a network. Run it with an endpoint +/// taken from a browser — `pushManager.subscribe()` in the console, or the settings switch — and +/// watch the notification arrive: +/// +/// ```sh +/// WHISPEE_REAL_ENDPOINT='https://fcm.googleapis.com/fcm/send/…' \ +/// cargo test --release -p server --test webpush -- --ignored --nocapture +/// ``` +/// +/// A refusal shows up two ways, and both are the point of running it: the subscription is dropped +/// here if the service answered `404`/`410`, and a `401` or `403` is logged by the emitter. Success +/// is a notification on the screen, which no assertion in this file can reach. +#[tokio::test] +#[ignore = "needs a live subscription in WHISPEE_REAL_ENDPOINT and a network"] +async fn a_real_push_service_accepts_the_token() { + let Ok(endpoint) = std::env::var("WHISPEE_REAL_ENDPOINT") else { + panic!("set WHISPEE_REAL_ENDPOINT to a subscription endpoint from a browser"); + }; + + let pool = common::test_pool().await; + let (server, advertised) = server_with_web_push(pool).await; + eprintln!("advertised key: {advertised}"); + eprintln!("pushing to: {endpoint}"); + + let (alice, group_id) = subscribed_pair(&server, &endpoint).await; + post_a_message(&alice, &group_id).await; + + // Long enough for the round trip to Google or Mozilla, which is not a loopback. + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + + let (rows,): (i64,) = + sqlx::query_as("SELECT count(*)::bigint FROM push_tokens WHERE token = $1") + .bind(&endpoint) + .fetch_one(&server.pool) + .await + .unwrap(); + + assert_eq!( + rows, 1, + "the push service reported this subscription gone — the endpoint is stale, or it was \ + minted against a different key than the one this deployment advertises" + ); + + eprintln!("the service did not reject the subscription; a notification should be on screen"); +} diff --git a/deploy/.env.example b/deploy/.env.example index 9e635c49..0f8251b8 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -48,6 +48,25 @@ ACCOUNT_STORAGE_BYTES= RUST_LOG=server=info,tower_http=info +# ----------------------------------------------------------------------------- web push +# +# The contact a push service is told to reach if this deployment misbehaves — a `mailto:` or an +# `https:` URL, as RFC 8292 requires. **Setting it is what turns waking on**, and it is the only +# thing to set: the VAPID key pair belongs to the server and is created on its first start, so +# there is no private key to generate, paste, or lose. +# +# Left empty, this deployment records subscriptions and wakes nobody. That is not a degraded mode: +# a self-hosted deployment that talks to no push service stays fully functional, and the client +# hides the control rather than offering a subscription that would never be woken. +# +# **Read what it costs before setting it.** Two things leave. The browser's push service — Google +# for Chrome, Mozilla for Firefox — learns each time a message arrives for one of your users, and +# can tie that to an address. And this server learns which devices to wake, which is what sealed +# sender was arranged to remove: a server that stops waking four members of five can tell who +# wrote the next message. Nothing cryptographic answers that. The wake-up itself carries no text, +# no sender and no group id. +VAPID_SUBJECT= + # ----------------------------------------------------------------------------- calls # # All empty, and a deployment that leaves them empty keeps a fully working messenger: the call diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 2df060b0..679f6eac 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -50,6 +50,11 @@ services: # them, and the comment above it explains what breaking that costs. ALLOWED_ORIGINS: https://${WHISPEE_DOMAIN:?set WHISPEE_DOMAIN in deploy/.env} ACCOUNT_STORAGE_BYTES: ${ACCOUNT_STORAGE_BYTES:-} + # Web Push. Empty here, and a deployment that leaves it empty wakes nobody: tokens still + # register, the emitter stays `Silent`, and the client hides the control because the key + # route answers 503. One variable, because the key pair is the server's own — see + # `migrations/0020_vapid.sql` for why it is not handed in. + VAPID_SUBJECT: ${VAPID_SUBJECT:-} # Calls. Empty here, and a deployment that leaves them empty keeps a fully working # messenger — the call route answers 503 and the client hides the button. Read # `crates/server/src/call.rs` before setting them: a call leaks more than a message does. diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index 93af257f..2f8235ac 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -120,6 +120,44 @@ deployment that rebuilds often and forgets its volume re-issues every time — a counts those: fifty certificates a week per domain, after which the domain is unusable for a week. +## Turning on push, and what it costs + +One variable, `VAPID_SUBJECT` — a `mailto:` or `https:` URL a push service can use to reach +whoever runs this deployment. There is no private key to generate: the pair belongs to the server +and is created on its first start. + +```sh +$EDITOR .env # VAPID_SUBJECT=mailto:ops@example.test +docker compose up -d +``` + +Left empty, subscriptions still register and nobody is woken — the key route answers 503 and the +client hides the control. A deployment that wants to talk to no push service keeps a fully working +messenger. + +**What it discloses, and the settings screen says this before offering the switch.** The browser's +push service — Google for Chrome, Mozilla for Firefox — learns each time a message arrives for one +of your users and can tie that to an address. And this server learns which devices to wake, which +is what sealed sender was arranged to remove: a server that stops waking four members of five can +tell who wrote the next message. Nothing cryptographic answers that. The wake-up itself carries no +text, no sender and no group id. + +### Checking it actually works + +The suites check the token against RFC 8292 and against the key advertised beside it, against a +fake push service. What they cannot check is that Google and Mozilla agree with that reading, so +one pass through a browser is part of standing a deployment up rather than optional: + +1. Open the deployment, create an account, allow notifications, then turn on **Wake this browser + when a message arrives** in Settings → Notifications. +2. Close every tab of the site. +3. From another account — a second browser profile does — send a message. +4. A notification saying "New message" should appear. Clicking it opens the application. + +If nothing arrives, `docker compose logs server | grep -i push` is where the refusal appears: a +`401` means the service rejected the token, a `403` usually means the subscription was minted +against a different key than the one now advertised. + ## Calls are not set up here `MEDIA_URL` and the four variables beside it are for a deployment that already runs a media @@ -137,10 +175,9 @@ is encrypted under a key derived from the MLS epoch, which is never sent anywher Written here rather than discovered later. -- **No push notifications.** `crates/server/src/push.rs` registers tokens and its default `Waker` - is `Silent`: it sends nothing. Neither provider exists, and the server has no outbound HTTP - client. On mobile this means the application is only notified while it is open — see - [`./ROADMAP.md`](./ROADMAP.md) for what is missing and what push would cost sealed sender. +- **No push to the packaged mobile application.** Web Push covers the web client and an Android + browser; FCM and APNs do not exist, so the Tauri build is only notified while it is open. See + [`./ROADMAP.md`](./ROADMAP.md) for why, and for what push costs sealed sender. - **No health endpoint.** The server exposes no route for a load balancer or an uptime check to call; `docker compose ps` and the logs are what there is. Adding one means adding a route, and it has not been done. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 9c9afb54..b9378c24 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -35,6 +35,7 @@ Everything in this list is implemented and has tests, unless the row says otherw | Desktop application | Tauri 2, interface packaged in the binary | | Reproducible signed releases | `scripts/release.sh` and `scripts/verify-release.sh` | | Mobile adaptation | Navigation, safe areas, keyboard, touch targets, lifecycle, offline state, native storage, QR pairing | +| Push notifications | Web Push, off until a deployment sets `VAPID_SUBJECT`; the wake-up carries nothing. Browsers only — no FCM, no APNs | The mobile work was carried out as seven numbered workstreams. Six landed. What each one left behind is in the next section, because "landed" and "verified on a real device" are not the @@ -53,48 +54,83 @@ These are finished features whose last mile could not be exercised on the develo | Mobile builds | Only ever built in CI, never locally — no Android NDK and no macOS host here | | Mobile builds in CI | `test.yml` runs the suites and the WebAssembly check on every pull request; `android.yml` and `ios.yml` stay manual or `main`-only, so no mobile artefact is built on a PR | -## Push notifications — half-built, and stopping there is the decision - -**What exists, server-side, and works:** token registration and replacement -(`crates/server/src/push.rs`, `migrations/0011_push.sql`), the logic that decides which devices -to wake after an envelope is posted, and a `Waker` trait whose default implementation, `Silent`, -sends nothing. - -`Silent` is not a stub. It is the behaviour of a deployment that has configured no provider, -and that deployment must stay **fully functional**: tokens register, nothing is sent, the -application keeps working exactly as it does today. Anyone wiring a real provider in must -preserve that first. - -**What is missing, precisely — all of it the part that requires secrets:** - -1. **An FCM provider.** HTTP v1, therefore OAuth2 with a service account, therefore an RS256 - JWT. -2. **An APNs provider.** ES256 JWT, `content-available: 1`. -3. **The configuration that wires them in.** Absent by default, or the second of the three - limits written into `migrations/0011_push.sql` falls. -4. **Device-side token registration.** The token comes from the operating system through a - Tauri plugin that has to be integrated, and it **changes without warning** — so registration - must be replayed at every start, not only when the feature is switched on. -5. **The user-facing setting.** Enabling it belongs to the user, and the screen has to say what - it discloses before offering the switch — as the vault screen does, for the same reason. - -The server also has **no outbound HTTP client** today. Adding one is a dependency and a new -network surface on a service that had none. - -**None of this is written, deliberately.** Integration code that has never been executed would -look like a feature where there is none, and a half-wired provider is the kind of thing that -appears to work in review and fails in the hands of the person relying on it. +## Push notifications — Web Push works, FCM and APNs do not + +**What works, end to end:** a browser subscribes from the settings screen, the server signs a +VAPID token per push service and sends an empty wake-up, and the service worker shows a +notification with the tab closed. `crates/server/src/vapid.rs` holds the ES256 half, +`crates/server/src/push.rs` the emitter, `apps/web/src/lib/push.ts` and `apps/web/public/sw.js` +the browser half. + +**What turns it on is one variable, `VAPID_SUBJECT`** — the contact a push service is told to +reach. There is no private key to supply: the pair is the server's own, created on first start +(`migrations/0020_vapid.sql`), the same shape `log_key` has had since the transparency log. Unset, +the waker is `Silent`, the key route answers 503, and the client hides the control. That is the +second of the three limits in `migrations/0011_push.sql`, and it is still the behaviour to +preserve first. + +### Why Web Push and not FCM + +The roadmap used to describe FCM and APNs, and said the missing part was "all of it the part that +requires secrets". That was true and it was not the hard part. The hard part is device-side +registration: it needs a Tauri plugin that does not exist, therefore Kotlin and Swift, and none of +it compiles or runs on the development machine — no NDK, no macOS host, no physical device. +Writing it would have produced exactly what this document refuses elsewhere: integration code that +has never been executed and looks like a feature. + +Web Push removed that wall for one specific reason. **The wake-up carries nothing**, so there is +no payload to encrypt, so the whole content-encryption half of Web Push — RFC 8291, `aes128gcm`, +the `p256dh` and `auth` subscription secrets — is unused. What is left is one ES256 signature. + +It also needed no migration for the addresses: without a payload the only thing worth keeping is +the endpoint URL, so `push::Address { provider, token }` holds a subscription unchanged. + +### What it cost elsewhere + +**The server has an outbound HTTP client now**, which it had never had. `reqwest` with rustls, no +cookie store, no redirect following — a push endpoint that answers with a redirect is not one to +follow carrying a bearer token. No request is made unless a deployment sets the variable. + +**There is a service worker**, and `notifications.ts` used to argue against one. The objection was +that a worker would cache the application shell served by the server the desktop build exists to +stop trusting. This one caches nothing — no `fetch` handler, no `Cache`, no precache manifest — +and `push.test.ts` asserts that rather than trusting the comment. It exists because a push message +wakes a worker and never a document. + +### What is still missing + +- **FCM and APNs.** The packaged mobile application is not covered; the web client and an Android + browser are. `Vapid::wake` matches on the provider name, so a second emitter lands beside it + without touching the call site, but neither is written and the wall described above has not + moved. +- **iOS needs the site installed to the home screen** before it will subscribe at all, and even + then a notification there can never show content — the service extension is a separate Swift + process while the keys live in a WASM module inside the webview. +- **The notification is generic.** "New message", and nothing else. The worker cannot decrypt: the + MLS keys are in the page's memory, not the worker's, and moving them would hand the decryption + keys to a context that outlives every tab. Same constraint iOS imposes, arrived at on purpose. +- **No test can establish that Google and Mozilla accept these tokens.** `tests/webpush.rs` checks + the token against the specification and against the public key advertised beside it, using a + fake push service that verifies the signature and asserts the body is empty. The residual risk + is a service disagreeing with our reading of RFC 8292, and only a real subscription settles it — + which is why the browser pass in `docs/DEPLOY.md` is part of the procedure rather than optional. ### What push costs, and it is not the tokens -Push **degrades sealed sender** — not through its tokens, but through its existence. A server -that chooses *whom* to wake gains a targeted activity trigger: ceasing to wake four members out -of five makes subsequent posts attributable to the fifth. Sealed sender protects against a -server that observes, not against a server that **paces**. Nothing cryptographic answers this. +Unchanged by any of the above, and the reason the feature stays optional. Push **degrades sealed +sender** — not through its tokens, but through its existence. A server that chooses *whom* to wake +gains a targeted activity trigger: ceasing to wake four members out of five makes subsequent posts +attributable to the fifth. Sealed sender protects against a server that observes, not against a +server that **paces**. Nothing cryptographic answers this. + +A second party learns something too, and the settings screen says so before offering the switch: +the browser's push service — Google for Chrome, Mozilla for Firefox — sees a wake-up arrive every +time a message does, and can tie that to an address. The content stays encrypted; the timing does +not. That is the price of the feature, and it is the reason it must stay strictly optional, inert without configuration, and empty — the wake-up carries no text, no sender and no group id, so -neither Apple, nor Google, nor the lock screen learns who writes to whom. +neither the push service nor the lock screen learns who writes to whom. ## Biometric unlock — written, never executed From b0848a6866ca76e8c0418c02adc53a97e303e14a Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 16:11:08 +0200 Subject: [PATCH 02/13] refactor(ui): settings open over the application, not instead of it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings were a route that replaced the centre column, so opening them made the whole window claim you had gone somewhere: the conversation you were reading disappeared to show a theme picker, and coming back meant navigating rather than closing. They are an interruption of the application, not a place in it, and now they say so — a dialog over whatever was on screen, with the rail and the thread still behind it. **The URL is kept, and that is the part worth defending.** `#/settings/notifications` still deep-links, the back gesture still steps out of a section before leaving, and `RouteAnnouncer` still has something to announce. A modal driven by local state would have been shorter and would have thrown all of that away. So `open` is derived from the route and closing navigates: one source of truth for whether settings are showing. `Dialog` gains `size="panel"` rather than a second modal being written beside it. That file exists for four things Radix does and every hand-rolled modal omits — the focus trap, `inert` on the rest, the scroll lock, focus restored on close — and a settings modal built next to it would have omitted the same four invisibly. `panel` changes layout only: wider, taller, no padding of its own, and no close button of its own because the content already carries one. A second cross would have been off-screen and still in the tab order, which is the mirror image of the hover-only control this project refuses. Checked in a browser: opened over a conversation, Escape closed it and landed back in that conversation, and a direct link to a section opened the dialog with the focus inside it. --- apps/web/src/app/SettingsScreen.tsx | 41 +++++++++++++++++- apps/web/src/app/Shell.tsx | 9 +++- apps/web/src/ui/Dialog.tsx | 64 ++++++++++++++++++++++++----- 3 files changed, 100 insertions(+), 14 deletions(-) diff --git a/apps/web/src/app/SettingsScreen.tsx b/apps/web/src/app/SettingsScreen.tsx index df792d4e..a12e2364 100644 --- a/apps/web/src/app/SettingsScreen.tsx +++ b/apps/web/src/app/SettingsScreen.tsx @@ -12,6 +12,7 @@ import { useTheme } from "@/lib/theme"; import { useOcclusion } from "@/lib/viewport"; import { Icon } from "@/ui/Icon"; import { IconButton } from "@/ui/IconButton"; +import { Dialog } from "@/ui/Dialog"; import { Panel } from "@/ui/Panel"; import { cn } from "@/ui/cn"; import type { SettingsSection } from "@/routes/route"; @@ -207,6 +208,44 @@ function Navigation({ section }: { section: SettingsSection | null }) { ); } +/** + * Settings, over whatever was on screen. + * + * # Why a modal and not a route that replaces the centre + * + * Because settings are not a place in the application, they are an interruption of it. Rendering + * them into the centre column made the whole window claim you had gone somewhere — the + * conversation you were reading disappeared to show you a theme picker, and coming back meant + * navigating rather than closing. + * + * # The URL is kept, and that is the part worth defending + * + * `#/settings/notifications` still works, still deep-links, and the back gesture still steps out + * of a section into the list before leaving. A modal driven by local state would have been + * simpler and would have thrown all of that away: the route is what makes a setting something you + * can send somebody, and what makes `RouteAnnouncer` able to say where the reader has arrived. + * + * So `open` is derived from the route rather than held here, and closing navigates rather than + * flipping a boolean. There is exactly one source of truth for whether settings are showing. + */ +export function SettingsDialog({ section }: { section: SettingsSection | null }) { + return ( + { + if (!next) history.back(); + }} + size="panel" + title={section === null ? "Settings" : `Settings, ${TITLES[section]}`} + > + + + ); +} + export function SettingsScreen({ section }: { section: SettingsSection | null }) { const duo = useDuo(); const occlusion = useOcclusion(); @@ -216,7 +255,7 @@ export function SettingsScreen({ section }: { section: SettingsSection | null }) const showNavigation = duo || section === null; return ( -
+
{showNavigation && ( // No `border-r`. The shell no longer divides anything with a hairline — panes are // separated by the gutter of ground between them — and a rule drawn here would be the diff --git a/apps/web/src/app/Shell.tsx b/apps/web/src/app/Shell.tsx index ae5a0ab8..53790623 100644 --- a/apps/web/src/app/Shell.tsx +++ b/apps/web/src/app/Shell.tsx @@ -13,7 +13,7 @@ import { DetailPanel } from "./DetailPanel"; import { EmptyCenter } from "./EmptyCenter"; import { NewConversation } from "./NewConversation"; import { Rail } from "./Rail"; -import { TITLES, SettingsScreen } from "./SettingsScreen"; +import { TITLES, SettingsDialog } from "./SettingsScreen"; import { RouteAnnouncer } from "./RouteAnnouncer"; import { useBinding } from "./Shortcuts"; import { ShortcutsHelp } from "./ShortcutsHelp"; @@ -223,7 +223,10 @@ export function Shell({ onLock, onForget }: { onLock: () => void; onForget: () = case "new": return ; case "settings": - return ; + // Settings are an overlay, so the centre keeps showing what settings were opened *over*. + // Arriving straight from a bookmark there is nothing behind, and the empty centre is the + // honest answer — the same thing `#/` shows. + return ; case "conversation": // A well-formed key that names nothing — a stale bookmark, or a thread this device has // not discovered yet. `parse` deliberately does not check existence, and redirecting @@ -272,6 +275,7 @@ export function Shell({ onLock, onForget }: { onLock: () => void; onForget: () = + {route.kind === "settings" && } ); } @@ -377,6 +381,7 @@ export function Shell({ onLock, onForget }: { onLock: () => void; onForget: () = + {route.kind === "settings" && } ); } diff --git a/apps/web/src/ui/Dialog.tsx b/apps/web/src/ui/Dialog.tsx index 6c9fd5ab..7d00c29e 100644 --- a/apps/web/src/ui/Dialog.tsx +++ b/apps/web/src/ui/Dialog.tsx @@ -46,6 +46,20 @@ import { useOverlayContainer } from "./Overlays.tsx"; * `variant="destructive"` action by its caller, because only the caller knows which of its * actions is the dangerous one. * + * # `size="panel"` is for a screen, not a question + * + * The default is a prompt: narrow, padded, one thing to answer. Settings are neither — ten + * sections in three groups, a list beside the section it opens — and cramming that into `max-w-md` + * would produce a column of truncated labels. + * + * It is a size on this component rather than a second modal elsewhere, because the four absences + * above are the reason this file exists. A settings modal built beside it would omit the same + * four, invisibly, and nobody would notice until a keyboard user tabbed out of it into a + * conversation they could not see. + * + * What `panel` changes is layout only: wider, taller, and no padding of its own — the content owns + * its own scrolling regions, which a prompt never needs. + * * What this does not solve: nothing here debounces. A dialog opened by a key that repeats, or by * two components at once, is a call-site problem — `open` is controlled, and whoever owns it * owns that. @@ -59,6 +73,7 @@ export function Dialog({ actions, children, tone = "default", + size = "prompt", }: { open: boolean; onOpenChange: (open: boolean) => void; @@ -72,6 +87,8 @@ export function Dialog({ actions?: ReactNode; children?: ReactNode; tone?: "default" | "danger"; + /** `prompt` asks one question; `panel` holds a screen. See the note above. */ + size?: "prompt" | "panel"; }): ReactElement { const container = useOverlayContainer(); @@ -100,18 +117,25 @@ export function Dialog({ "fixed left-1/2 top-1/2 z-(--z-index-overlay) -translate-x-1/2 -translate-y-1/2", // Never wider than the window minus a margin, never taller than it: a dialog that // overflows the viewport puts its actions off-screen, where they cannot be reached. - "w-[calc(100%-2rem)] max-w-md max-h-[calc(100dvh-2rem)] overflow-y-auto", + "w-[calc(100%-2rem)] max-h-[calc(100dvh-2rem)]", + size === "panel" + // Tall as well as wide, and a fixed height rather than a maximum: the content is two + // scrolling columns, and a box that shrinks to its shortest column would make the + // list jump every time a section with less in it is opened. + ? "max-w-3xl h-[calc(100dvh-2rem)] sm:h-[44rem] overflow-hidden flex flex-col" + : "max-w-md overflow-y-auto", // The portal is outside the layout, so the shell's insets do not reach it. "safe-sides", - "rounded-control border bg-(--color-surface-raised) p-pane shadow-overlay", + "rounded-control border bg-(--color-surface-raised) shadow-overlay", + size === "panel" ? null : "p-pane", tone === "danger" ? "border-(--color-danger)" : "border-(--color-border-strong)", )} > -
+
{description} @@ -135,13 +159,31 @@ export function Dialog({
{/* Escape and a click outside already close it; this is for the pointer user who - looks for a cross, and for the touch user who has neither. */} - - } className="-mr-snug -mt-snug" /> - + looks for a cross, and for the touch user who has neither. + + Not in a panel: the content carries its own close control there, and a second one + hidden off-screen would still be in the tab order — a button a keyboard user + reaches and cannot see, which is the mirror image of the hover-only control this + project refuses everywhere. */} + {size === "panel" ? null : ( + + } className="-mr-snug -mt-snug" /> + + )}
- {children === undefined ? null :
{children}
} + {children === undefined ? null : ( +
+ {children} +
+ )} {actions === undefined ? null : (
{actions}
From 39601f3b8c859404479f638f7ade5b4df9e20117 Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 16:12:54 +0200 Subject: [PATCH 03/13] docs(threat-model): push is no longer half-built, so stop saying it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §4 opened with "the server records tokens, decides who to wake, and sends nothing". That was true when it was written and stopped being true with Web Push, and a threat model describing a feature as inert while it delivers is worse than one that omits it: a reader checks what the project claims to do before checking what it does. What is written instead is what holds now — it works on browsers, only there, and only once a deployment sets `VAPID_SUBJECT`. The three limits that follow are re-pointed at the service that actually does the waking, and one of them gains the reason the wake-up is empty is now structural rather than a policy: with no payload there is nothing to encrypt, so RFC 8291 is unused and the subscription's secrets are never read. The limitations table needed no change. It described the cost, and the cost did not move — a server that chooses whom to wake still gains a targeted activity trigger, and no cryptography answers that. --- docs/THREAT-MODEL.md | 45 +++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/docs/THREAT-MODEL.md b/docs/THREAT-MODEL.md index ce603d0b..3d00fedd 100644 --- a/docs/THREAT-MODEL.md +++ b/docs/THREAT-MODEL.md @@ -167,7 +167,7 @@ Same position, now actively lying, withholding, injecting and delaying. meantime. The server-side membership filter narrows the window without closing it. - **serve hostile JavaScript**, on the web target, on every load. No browser policy fixes that — which is what the desktop application, with its interface inside the signed binary, exists for. -- **choose whom to wake**, if push is ever configured. See §4. +- **choose whom to wake**, wherever push is configured. See §4. - **keep an envelope forever.** There is no purge, and no proof of deletion for anything. ### 2.4 Another group member @@ -339,9 +339,15 @@ chair. Sharing a secret between the two would silently promote the weaker requir This is the one property the project knowingly trades away. The mobile execution plan recorded the decision and said it belonged in the documentation; it was never written down. It is written here. -Push is **half-built**: the server records tokens, decides who to wake, and sends nothing. There is -no FCM or APNs provider, no configuration, no device-side token registration, and no user-facing -setting. `Silent` is the default waker and it wakes nobody. +Push **works, over Web Push, and only there**. A browser subscribes from the settings screen, the +server signs a VAPID token per push service and sends an empty wake-up, and a service worker shows +a notification with the tab closed. There is no FCM and no APNs provider, so the packaged mobile +application is still only notified while it is open. + +It is off until a deployment sets `VAPID_SUBJECT`. Unset — which is the default, and the state of +every deployment that has not decided otherwise — `Silent` is the waker, it wakes nobody, and the +route serving the subscription key answers 503 so the client hides the control. That is not a +degraded mode: a deployment that talks to no push service keeps a fully working messenger. The degradation is not caused by the tokens. It is caused by the feature's **existence**: @@ -351,19 +357,29 @@ The degradation is not caused by the tokens. It is caused by the feature's **exi > cryptography answers this. That is the price of the feature. It is also why the feature is strictly optional and inert without -configuration: a self-hosted deployment that talks to neither Apple nor Google must stay fully -functional, and does. +configuration: a self-hosted deployment that talks to no push service must stay fully functional, +and does. + +**Turning it on is the user's decision as well as the operator's**, and the settings screen states +both halves of the cost — the push service learning the rhythm, and this server learning whom to +wake — above the switch rather than under it. Three further limits follow from push, and hold whenever it is configured: -- **the third party learns the rhythm.** For a sleeping phone to learn a message is waiting, - Google or Apple must wake it — and they can tie that device to an account. The content stays - encrypted; the activity metadata leaks, and that is irreducible, not a defect; +- **the third party learns the rhythm.** For a closed browser to learn a message is waiting, its + vendor's push service — Google for Chrome, Mozilla for Firefox — must wake it, and can tie that + browser to an address. The content stays encrypted; the activity metadata leaks, and that is + irreducible, not a defect; - **the wake-up carries nothing** — no text, no sender, no group id, because putting the message in - the notification would show it to the provider *and* to the lock screen; -- **on iOS, the notification will never show the content.** The service extension is a separate - Swift process; the keys live in a WASM module inside the webview. Fixing that would require - porting the crypto to native. + the notification would show it to the provider *and* to the lock screen. Over Web Push that is + not only a policy: with no payload there is nothing to encrypt, so the whole of RFC 8291 is + unused and the subscription's own secrets are never read. `tests/webpush.rs` asserts the empty + body at the wire; +- **the notification says only that something arrived.** The service worker cannot decrypt: the MLS + keys are in the page's memory, not the worker's, and moving them there would hand the decryption + keys to a context that outlives every tab. On iOS the same holds for a different reason — the + service extension is a separate Swift process — and fixing that would require porting the crypto + to native. --- @@ -378,6 +394,9 @@ What it buys is smaller in exact proportion: it fires only while the page is run tab, a killed process or a sleeping phone produces nothing, and no client-side work changes that. The honest statement is "you find out sooner while the application is open", not "you find out". +That gap is what §4's feature closes, on browsers, at the price §4 states — and closing it is what +makes the price worth restating on the screen that offers it. + What it does disclose is on the screen, not on the wire: that this application is installed, and that something arrived. The notice carries no sender, no group and no text. Naming the conversation is available and off by default, behind copy that says what lands on a lock screen From c73ed0abc89f3910a57f088c169796daabf287aa Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 16:32:54 +0200 Subject: [PATCH 04/13] test(push): ask a real push service, and say what it answered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suites check this server against RFC 8292 as we read it, using a fake service that verifies the signature and asserts the body is empty. A service disagreeing with that reading would pass every one of them. `a_real_push_service_accepts_the_token` is the other half, ignored by default because it needs a subscription minted by a real browser. It signs exactly as the emitter does and sends to the live endpoint, so the answer reaches the test instead of a `tracing::warn!`. Run once against Chrome's service: `201 Created`, and the notification appeared with every tab closed — server to FCM to service worker to screen. Mozilla has not been tried, and the roadmap says so rather than implying otherwise. `docs/DEPLOY.md` gains the command beside the browser pass, because it is what separates "the service refused us" from "the browser showed nothing". --- crates/server/tests/webpush.rs | 74 +++++++++++++++++----------------- docs/DEPLOY.md | 12 ++++++ docs/ROADMAP.md | 15 ++++--- 3 files changed, 60 insertions(+), 41 deletions(-) diff --git a/crates/server/tests/webpush.rs b/crates/server/tests/webpush.rs index f5488d26..2d166de2 100644 --- a/crates/server/tests/webpush.rs +++ b/crates/server/tests/webpush.rs @@ -345,50 +345,52 @@ async fn the_key_route_is_unavailable_when_push_is_off() { assert_eq!(response.status(), 503); } -/// **The one thing the double above cannot settle: does a real push service accept our token?** +/// **The one thing no double can settle: does a real push service accept these tokens?** /// -/// Ignored by default, because it needs a live subscription and a network. Run it with an endpoint -/// taken from a browser — `pushManager.subscribe()` in the console, or the settings switch — and -/// watch the notification arrive: +/// Ignored by default, because it needs a live subscription minted by a real browser and reaches +/// out to Google or Mozilla. It is the other half of `docs/DEPLOY.md`'s browser pass, and the +/// reason that pass exists: everything above checks this server against RFC 8292 as we read it, +/// and a service disagreeing with that reading would pass every one of them. /// -/// ```sh -/// WHISPEE_REAL_ENDPOINT='https://fcm.googleapis.com/fcm/send/…' \ -/// cargo test --release -p server --test webpush -- --ignored --nocapture -/// ``` +/// Take the endpoint from a browser that has subscribed — it is the `token` column of +/// `push_tokens`, or `pushManager.getSubscription().endpoint` in its console — and run: /// -/// A refusal shows up two ways, and both are the point of running it: the subscription is dropped -/// here if the service answered `404`/`410`, and a `401` or `403` is logged by the emitter. Success -/// is a notification on the screen, which no assertion in this file can reach. +/// WEBPUSH_ENDPOINT='https://fcm.googleapis.com/fcm/send/…' \ +/// cargo test -p server --release --test webpush -- --ignored --nocapture +/// +/// A pass means the service accepted the token and queued the wake-up. Whether a notification +/// then appears is the browser's half, and only a person looking at the screen can say. #[tokio::test] -#[ignore = "needs a live subscription in WHISPEE_REAL_ENDPOINT and a network"] +#[ignore = "needs a live subscription and talks to a real push service"] async fn a_real_push_service_accepts_the_token() { - let Ok(endpoint) = std::env::var("WHISPEE_REAL_ENDPOINT") else { - panic!("set WHISPEE_REAL_ENDPOINT to a subscription endpoint from a browser"); + let Ok(endpoint) = std::env::var("WEBPUSH_ENDPOINT") else { + panic!("set WEBPUSH_ENDPOINT to a subscription endpoint from a real browser"); }; let pool = common::test_pool().await; - let (server, advertised) = server_with_web_push(pool).await; - eprintln!("advertised key: {advertised}"); - eprintln!("pushing to: {endpoint}"); - - let (alice, group_id) = subscribed_pair(&server, &endpoint).await; - post_a_message(&alice, &group_id).await; - - // Long enough for the round trip to Google or Mozilla, which is not a loopback. - tokio::time::sleep(std::time::Duration::from_secs(3)).await; - - let (rows,): (i64,) = - sqlx::query_as("SELECT count(*)::bigint FROM push_tokens WHERE token = $1") - .bind(&endpoint) - .fetch_one(&server.pool) - .await - .unwrap(); + let key = server::vapid::Key::ensure(&pool).await.unwrap(); - assert_eq!( - rows, 1, - "the push service reported this subscription gone — the endpoint is stale, or it was \ - minted against a different key than the one this deployment advertises" + // Signed exactly as the emitter does, and sent by hand rather than through `Vapid::wake` so + // that the service's answer reaches this test instead of a `tracing::warn!`. + let audience = server::vapid::audience_of(&endpoint).expect("an http(s) endpoint"); + let token = key.token(&audience, SUBJECT); + + let response = reqwest::Client::new() + .post(&endpoint) + .header("Authorization", format!("vapid t={token}, k={}", key.public_key())) + .header("TTL", "60") + .header("Content-Length", "0") + .send() + .await + .expect("the push service was unreachable"); + + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + + println!("push service answered {status}: {body}"); + assert!( + status.is_success(), + "the service refused the wake-up: {status} {body} — \ + 401 means it rejected the token, 403 that the subscription was minted under another key" ); - - eprintln!("the service did not reject the subscription; a notification should be on screen"); } diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index 2f8235ac..433da8d7 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -158,6 +158,18 @@ If nothing arrives, `docker compose logs server | grep -i push` is where the ref `401` means the service rejected the token, a `403` usually means the subscription was minted against a different key than the one now advertised. +To separate "the service refused us" from "the browser showed nothing", take the endpoint — the +`token` column of `push_tokens`, or `pushManager.getSubscription().endpoint` in the browser's +console — and ask the service directly: + +```sh +WEBPUSH_ENDPOINT='https://fcm.googleapis.com/fcm/send/…' \ + cargo test -p server --release --test webpush -- --ignored --nocapture +``` + +It prints what the service answered. A `201` means the wake-up was accepted and queued, so +anything still missing is on the browser's side of the line. + ## Calls are not set up here `MEDIA_URL` and the four variables beside it are for a deployment that already runs a media diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index b9378c24..61f014e1 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -109,11 +109,16 @@ wakes a worker and never a document. - **The notification is generic.** "New message", and nothing else. The worker cannot decrypt: the MLS keys are in the page's memory, not the worker's, and moving them would hand the decryption keys to a context that outlives every tab. Same constraint iOS imposes, arrived at on purpose. -- **No test can establish that Google and Mozilla accept these tokens.** `tests/webpush.rs` checks - the token against the specification and against the public key advertised beside it, using a - fake push service that verifies the signature and asserts the body is empty. The residual risk - is a service disagreeing with our reading of RFC 8292, and only a real subscription settles it — - which is why the browser pass in `docs/DEPLOY.md` is part of the procedure rather than optional. +- **No automated test can establish that Google and Mozilla accept these tokens.** + `tests/webpush.rs` checks the token against the specification and against the public key + advertised beside it, using a fake push service that verifies the signature and asserts the body + is empty. A service disagreeing with our reading of RFC 8292 would pass every one of them. + + What closes that is `a_real_push_service_accepts_the_token`, ignored by default because it needs + a subscription minted by a real browser: it signs exactly as the emitter does and sends to the + live endpoint. Run once against Chrome's service, which answered `201 Created`, and the + notification appeared with every tab closed — server to FCM to service worker to screen. Mozilla + has not been tried. The command is in `docs/DEPLOY.md` beside the browser pass. ### What push costs, and it is not the tokens From c58f3e5402569518fca89c91ef105c61607c471e Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 16:33:08 +0200 Subject: [PATCH 05/13] feat(ui): errors float beside confirmations instead of shrinking the room MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A confirmation already landed well: portalled, animated, announced politely, expired by `report.ts` rather than by a timer inside the component. An error did not. It was a `Banner` with `rounded-none border-x-0 border-b-0` mounted as a flex child of the shell, so it took height from the conversation for as long as it stood, and up to four of those could stack. Both halves come through `ui/Toast.tsx` now. The lifetime does not move — an error still waits to be dismissed or replaced, because it usually means something is left to decide. Only the rendering does. # Radix, for one reason A message can now carry an action, and a button that appears unbidden has to be reachable by keyboard **without stealing focus**. That is a viewport in the tab order, a recall hotkey, and an announcement whose urgency matches the message — `type="foreground"` for an error, `"background"` for a confirmation, the same distinction `Banner` draws between `alert` and `status`. It also buys an exit. `Overlays.tsx` records that `useEntered` has no counterpart because Radix unmounts on close; a toast keeps its node through `data-state="closed"` and fades instead of blinking out. The expiry stays in `report.ts` and Radix's own `duration` is `Infinity`: two owners of one timer is one too many. # What the browser pass changed The action was wired nowhere, which would have shipped a path never executed. It is on the failing poll now: "Retry" does at once what the thirty-second timer would have done, and a second failure replaces the message rather than stacking. And Radix's `label` prop — documented as the accessible name, accepted by its types — reaches no attribute in 1.2.23: the rendered `
    ` carried `tabindex` and `class` and nothing else. `aria-label` is set directly, and checked in the browser rather than assumed. Verified by hand, since `node --test` has no DOM and no component in this repository is testable: the confirmation expires by itself, the error persists and is cleared by the next successful poll, one `
  1. ` after several failures and a retry, and the whole thing draws above the settings dialog — which matters more since settings became one. --- apps/web/package.json | 1 + apps/web/pnpm-lock.yaml | 36 ++++ apps/web/src/App.tsx | 45 +++-- apps/web/src/state/report.ts | 96 ++++++++--- apps/web/src/ui/Toast.tsx | 197 ++++++++++++++++------ docs/plans/2026-08-25-feedback-channel.md | 103 +++++++++++ 6 files changed, 388 insertions(+), 90 deletions(-) create mode 100644 docs/plans/2026-08-25-feedback-channel.md diff --git a/apps/web/package.json b/apps/web/package.json index d76a498f..6d5d9ce2 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -19,6 +19,7 @@ "@radix-ui/react-popover": "^1.1.23", "@radix-ui/react-slot": "^1.3.3", "@radix-ui/react-switch": "^1.3.7", + "@radix-ui/react-toast": "^1.2.23", "@radix-ui/react-tooltip": "^1.2.16", "@tauri-apps/api": "^2.11.1", "@tauri-apps/plugin-opener": "^2.5.4", diff --git a/apps/web/pnpm-lock.yaml b/apps/web/pnpm-lock.yaml index e8c5604e..a289da0f 100644 --- a/apps/web/pnpm-lock.yaml +++ b/apps/web/pnpm-lock.yaml @@ -26,6 +26,9 @@ importers: '@radix-ui/react-switch': specifier: ^1.3.7 version: 1.3.7(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + '@radix-ui/react-toast': + specifier: ^1.2.23 + version: 1.2.23(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) '@radix-ui/react-tooltip': specifier: ^1.2.16 version: 1.2.16(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) @@ -795,6 +798,19 @@ packages: '@types/react-dom': optional: true + '@radix-ui/react-toast@1.2.23': + resolution: {integrity: sha512-ofhyAsYaocRGOs/n0XWdUOSVzEAG6BfrMVM8z0c0kLEWY38w/0WuMFPTJP/HVaZPYkMvHZoKIIhNcjbTCBILPg==} + peerDependencies: + '@types/react': '*' + '@types/react-dom': '*' + react: ^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc + react-dom: ^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc + peerDependenciesMeta: + '@types/react': + optional: true + '@types/react-dom': + optional: true + '@radix-ui/react-tooltip@1.2.16': resolution: {integrity: sha512-6EamKFRRnlpdadndbZ6LMwycfwkwPte1B42hs6QA0gYhjaOKqW4PZ4pjaW9UrlDX5eVt/OjncE7BFTPL5nmZhg==} peerDependencies: @@ -3004,6 +3020,26 @@ snapshots: '@types/react': 19.2.18 '@types/react-dom': 19.2.4(@types/react@19.2.18) + '@radix-ui/react-toast@1.2.23(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8)': + dependencies: + '@radix-ui/primitive': 1.1.7 + '@radix-ui/react-collection': 1.1.15(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + '@radix-ui/react-compose-refs': 1.1.5(@types/react@19.2.18)(react@19.2.8) + '@radix-ui/react-context': 1.2.2(@types/react@19.2.18)(react@19.2.8) + '@radix-ui/react-dismissable-layer': 1.1.19(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + '@radix-ui/react-portal': 1.1.17(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + '@radix-ui/react-presence': 1.1.10(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + '@radix-ui/react-primitive': 2.1.10(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + '@radix-ui/react-use-callback-ref': 1.1.4(@types/react@19.2.18)(react@19.2.8) + '@radix-ui/react-use-controllable-state': 1.2.6(@types/react@19.2.18)(react@19.2.8) + '@radix-ui/react-use-layout-effect': 1.1.4(@types/react@19.2.18)(react@19.2.8) + '@radix-ui/react-visually-hidden': 1.2.11(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + react: 19.2.8 + react-dom: 19.2.8(react@19.2.8) + optionalDependencies: + '@types/react': 19.2.18 + '@types/react-dom': 19.2.4(@types/react@19.2.18) + '@radix-ui/react-tooltip@1.2.16(@types/react-dom@19.2.4(@types/react@19.2.18))(@types/react@19.2.18)(react-dom@19.2.8(react@19.2.8))(react@19.2.8)': dependencies: '@radix-ui/primitive': 1.1.7 diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 01023978..af5bfaab 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -238,7 +238,16 @@ function Boot() { } if (!store) { - return ; + // The sentence, not the message object: onboarding is a full screen shown before a session + // exists, so the toast viewport is not mounted behind it and its own `Banner` is the only + // surface there is. Nothing here floats over anything. + return ( + + ); } return ( @@ -447,7 +456,19 @@ function Frame({ dismissError(); bump(); } catch (e) { - if (!cancelled) report.error(e instanceof Error ? e.message : String(e)); + // The one place an action earns its keep. A poll that failed will be retried in thirty + // seconds anyway, and thirty seconds is a long time to sit in front of a sentence saying + // the connection is gone — especially when the cause was a laptop lid, and the fix is to + // ask again. The button does exactly what the timer would have done, sooner. + // + // Retrying calls `tick` again, so a second failure replaces this message with a new one + // and a success clears it through `dismissError` above. Nothing accumulates. + if (!cancelled) { + report.error(e instanceof Error ? e.message : String(e), { + label: "Retry", + run: () => void tick(), + }); + } } }; @@ -680,17 +701,15 @@ function Frame({ )} - {/* Always dismissible: an error you cannot wave away ends up part of the scenery. */} - {reported.error && ( - - {reported.error} - - )} - + {/* + * The error used to be a fourth full-bleed banner here, and it was the one that did not + * belong: the three above describe a state the reader is *in* — no network, a log that + * disagrees with itself, a session that would not restore — while an error describes + * something that just happened. A standing condition earns room in the layout; an event + * does not, and taking it left the conversation an inch shorter for as long as the error + * stood. It floats now, out of `ui/Toast.tsx`, and is still dismissible for the reason + * that always applied: an error you cannot wave away ends up part of the scenery. + */}
); diff --git a/apps/web/src/state/report.ts b/apps/web/src/state/report.ts index cde72132..4a91eb36 100644 --- a/apps/web/src/state/report.ts +++ b/apps/web/src/state/report.ts @@ -12,10 +12,14 @@ * * # Two surfaces, because the two have different lifetimes * - * **Errors go to a banner, and the banner is dismissible.** That rule is inherited verbatim from - * `App.tsx` and it is worth restating: an error you cannot wave away ends up part of the scenery, - * and scenery is not read. It stays until it is dismissed or replaced, because an error usually - * means something still has to be decided. + * **Errors stay until dismissed or replaced, and they are always dismissible.** That rule is + * inherited verbatim from `App.tsx` and it is worth restating: an error you cannot wave away ends + * up part of the scenery, and scenery is not read. It stays because an error usually means + * something still has to be decided. + * + * It used to be a full-bleed banner mounted in the shell's flex column, which shrank the + * conversation for as long as it stood. It floats now, beside the confirmations — the lifetime + * did not move, only the rendering. `ui/Toast.tsx` draws both. * * **Successes go to a toast, one at a time, for four seconds.** A success has already happened; * nothing is pending on the reader, so it expires on its own. One at a time rather than a stack: @@ -27,8 +31,8 @@ * * # What this module does not render * - * No DOM. It owns the state and the timer, and hands both out. `ui/Toast.tsx` and the shell draw - * the banner and the toast from `useReported()`. + * No DOM. It owns the state and the timer, and hands both out. `ui/Toast.tsx` draws both from + * `useReported()`. * * The contract that matters for them: **the expiry lives here, not in the component.** A toast * component that ran its own `setTimeout` would restart it on every re-render of its parent, and @@ -52,29 +56,64 @@ import { /** How long a confirmation stays up. Long enough to read a short sentence, short enough to ignore. */ export const TOAST_MS = 4000; -export interface Toast { - /** - * Distinguishes two toasts carrying the same text — "Copied" twice in a row is the ordinary - * case, and without this the second one would be indistinguishable from the first still hanging - * around. - */ +/** + * Something the reader can do about what just happened. + * + * Optional, and rare on purpose. A confirmation has nothing to offer — the thing already worked. + * A failure sometimes does: "Retry" on a send that did not go out is worth more than a sentence + * explaining that it did not. + * + * `run` is called on click and nothing else happens: dismissing afterwards is the caller's + * business, because only it knows whether the retry succeeded. An action that reported its own + * outcome would raise a second message on top of the first, and the first is what it replaced. + */ +export interface Action { + label: string; + run: () => void; +} + +/** + * One thing to say, whichever surface it lands on. + * + * # Why the error has an id now + * + * It did not, and that was an asymmetry with a cost. The id is what a React `key` uses to tell a + * replacement from the same message still hanging around — "Copied" twice in a row is the ordinary + * case for a confirmation, and two identical failures in a row is the ordinary case for a poll + * against a server that is down. Without it the second one silently reuses the first one's node + * and the entrance never replays, so nothing on screen says a new thing happened. + */ +export interface Message { id: number; message: string; + action?: Action; } +/** Kept as the old name for what a confirmation is, because that is what the callers call it. */ +export type Toast = Message; + /** What the shell needs in order to draw. */ export interface Reported { /** The standing error, or null. Survives until dismissed or replaced. */ - error: string | null; + error: Message | null; /** The confirmation currently on screen, or null. Expires by itself. */ - toast: Toast | null; + toast: Message | null; dismissError: () => void; + /** + * Takes a confirmation down before its time is up. + * + * It expires on its own, so nothing needs this to be correct — until the surface drawing it + * lets somebody swipe or close it. A dismissal the state does not hear about is a toast that + * reappears on the next render, which looks like a bug in the message rather than in the + * plumbing. + */ + dismissToast: () => void; } /** What everybody else needs in order to speak. */ export interface Report { - error: (message: string) => void; - done: (message: string) => void; + error: (message: string, action?: Action) => void; + done: (message: string, action?: Action) => void; } /** @@ -88,8 +127,8 @@ const ReportContext = createContext(null); const ReportedContext = createContext(null); export function ReportProvider({ children }: { children: ReactNode }) { - const [error, setError] = useState(null); - const [toast, setToast] = useState(null); + const [error, setError] = useState(null); + const [toast, setToast] = useState(null); /** * Monotonic, and never reset. It only has to be unique within one run of the application; the * alternative — a timestamp — collides for two toasts raised in the same millisecond, which is @@ -100,13 +139,18 @@ export function ReportProvider({ children }: { children: ReactNode }) { const report = useMemo( () => ({ - error: (message) => setError(message), - done: (message) => { + // No timer, and that is the whole difference between the two. An error waits to be + // dismissed or replaced; see this module's header for why. + error: (message, action) => { + nextId.current += 1; + setError({ id: nextId.current, message, action }); + }, + done: (message, action) => { // Cleared before rescheduling: without this, the first toast's timer would still be // running and would take the replacement down early. clearTimeout(expiry.current); nextId.current += 1; - setToast({ id: nextId.current, message }); + setToast({ id: nextId.current, message, action }); expiry.current = setTimeout(() => setToast(null), TOAST_MS); }, }), @@ -118,9 +162,15 @@ export function ReportProvider({ children }: { children: ReactNode }) { useEffect(() => () => clearTimeout(expiry.current), []); const dismissError = useCallback(() => setError(null), []); + const dismissToast = useCallback(() => { + // The timer goes with it: left running it would fire against a toast that is already gone, + // which is harmless today and would not be if this ever cleared something newer. + clearTimeout(expiry.current); + setToast(null); + }, []); const reported = useMemo( - () => ({ error, toast, dismissError }), - [error, toast, dismissError], + () => ({ error, toast, dismissError, dismissToast }), + [error, toast, dismissError, dismissToast], ); return createElement( diff --git a/apps/web/src/ui/Toast.tsx b/apps/web/src/ui/Toast.tsx index 00233626..572ae7e3 100644 --- a/apps/web/src/ui/Toast.tsx +++ b/apps/web/src/ui/Toast.tsx @@ -1,48 +1,57 @@ +import * as RadixToast from "@radix-ui/react-toast"; import { createPortal } from "react-dom"; import type { ReactElement } from "react"; +import type { Message } from "../state/report.ts"; import { useReported } from "../state/report.ts"; +import { Button } from "./Button.tsx"; import { cn } from "./cn.ts"; -import { useEntered, useOverlayContainer } from "./Overlays.tsx"; +import { Icon } from "./Icon.tsx"; +import { IconButton } from "./IconButton.tsx"; +import { useOverlayContainer } from "./Overlays.tsx"; /** - * The confirmation that an action worked. + * What just happened, said in one place. * - * # This file owns no state and no timer, and that is the contract + * # Both halves come here now, and that is the change * - * `state/report.ts` holds both. It says so in its own header and the reason is worth repeating - * here, at the place that would get it wrong: a component running its own `setTimeout` restarts - * it on every re-render of its parent. In a thread receiving messages that means the timer never - * expires and a four-second confirmation stays on screen until the conversation goes quiet. + * A confirmation always did. An error went to a `Banner` mounted as a flex child of the shell — + * full-bleed, corners squared off through a `className`, and **shrinking the conversation to make + * room for itself**. Up to four of those could stack. Errors float here instead; the three + * remaining banners in `App.tsx` stay where they are because they describe standing conditions + * (offline, an inconsistent key log) rather than events, and a standing condition belongs in the + * layout. * - * So this reads `useReported().toast` and draws it. When the field turns null, the toast is over. - * Nothing here schedules anything. + * # This file still owns no state and no timer * - * # `key={toast.id}` + * `state/report.ts` holds both, and the reason is worth repeating at the place that would get it + * wrong: a component running its own `setTimeout` restarts it on every re-render of its parent. In + * a thread receiving messages that means the timer never expires and a four-second confirmation + * stays until the conversation goes quiet. * - * One toast at a time, and a new one replaces the old. Without the key React sees the same - * component in the same position and merely swaps the text — the entrance never replays, and two - * confirmations in a row look like one that changed its mind. The id exists in `report.ts` - * precisely so that "Copied" following "Copied" is still visibly a second event. + * Radix has a `duration` of its own, so it is set to `Infinity` on both roots — not because + * nothing should expire, but because **two owners of one expiry is one owner too many**. What + * closes a toast is `report.ts` letting go of it. * - * # The live region outlives the toast + * # Why Radix rather than the portal this file used to be * - * `role="status"` with `aria-live="polite"` is on the *container*, which is mounted for as long - * as the shell is, empty or not. A live region that appears at the same moment as its content is - * unreliable — several screen readers only announce changes to a region they were already - * observing, so a region that mounts with its message announces nothing. Mounting it empty and - * filling it later is what makes the announcement happen. + * One reason: a toast can now carry a button, and a button that appears unbidden has to be + * reachable by keyboard **without stealing focus**. That is not a ` + + )} + + + } size="sm" className="shrink-0" /> + + ); } diff --git a/docs/plans/2026-08-25-feedback-channel.md b/docs/plans/2026-08-25-feedback-channel.md new file mode 100644 index 00000000..13db2f63 --- /dev/null +++ b/docs/plans/2026-08-25-feedback-channel.md @@ -0,0 +1,103 @@ +# One floating channel for what just happened + +## Context + +Successes already land well: `ui/Toast.tsx` portals a small card into `#overlays`, animates it in, +announces it politely, and expires it from `state/report.ts` rather than from a timer inside the +component. Errors do not. They render as a `Banner` with `rounded-none border-x-0 border-b-0` at +`App.tsx:683` — a full-bleed strip that is a **flex child of the shell**, so it shrinks the +conversation to make room for itself. Up to four of those can stack (`offline`, `fallback`, the +key-log warning, and this one), each squaring off its own corners through a `className` that +`Banner.tsx:52-77` already calls a workaround pinned in place rather than fixed. + +Two decisions frame the work, both taken deliberately: + +- **An error stays until it is dismissed or replaced.** `report.ts:13-26` argues it and the + argument holds — an error usually means something is still to be decided. Only the rendering + moves; the lifetime does not. +- **A message may carry an action.** "Retry" on a failed send is worth more than a sentence about + the failure. That is a new interactive surface inside a live region, which is the part that + needs care rather than the part that needs code. + +## What is built + +### Radix Toast replaces the hand-rolled portal + +`@radix-ui/react-toast` (1.2.23, peer-compatible with React 19) joins the seven Radix packages +already in `apps/web/package.json`. It is chosen for one reason: a toast that carries a button has +to be **reachable by keyboard after appearing unbidden, without stealing focus**, and Radix +implements that pattern — a recall hotkey, the viewport in the tab order, and `type` driving +whether the announcement interrupts. + +That mapping is the existing rule, kept: `type="foreground"` for errors (assertive, interrupts) and +`type="background"` for successes (polite). It is the same distinction `Banner.tsx:132` draws +between `alert` and `status`, and `docs/ACCESSIBILITY.md:121-127` states. + +What is gained beyond the button: an **exit** animation. `Overlays.tsx:110` records that +`useEntered` has none because Radix unmounts on close; Radix Toast keeps the node through +`data-state="closed"`, so a dismissed message fades instead of vanishing. + +What is given up, and must be carried over rather than lost: + +- the timer stays in `report.ts`, never in the component (`Toast.tsx:10-18`) — Radix's own + `duration` is set to `Infinity` so that it never competes for ownership of expiry; +- the viewport still portals into `#overlays` with `useOverlayContainer()`, and still carries + `safe-bottom safe-sides` itself (`Overlays.tsx:21-30`); +- `z-(--z-index-toast)` stays above dialogs, for the reason `index.css:241` gives: an action taken + *inside* a confirmation still has to be able to report that it worked. + +### `state/report.ts` gains an id and an optional action + +`Toast` already has an `id`; the error does not, so two identical errors are indistinguishable and +nothing can key a re-entrance. Both become the same shape — `{ id, message, action? }` — with +`action = { label: string, run: () => void }`. + +`Report.error(message, action?)` and `Report.done(message, action?)`. Every one of the 68 existing +call sites keeps working unchanged, because the parameter is optional. + +**The one coupling that must not break**: `App.tsx:445-451` calls `dismissError()` when a poll +succeeds, so a passing incident does not leave a red strip on screen forever. The poll fails every +thirty seconds while a server is down (`POLL_MS`, `App.tsx:71`), and `Rail.tsx:769-799` can raise +three errors in a row. Nothing in `report.ts` coalesces; that stays true, and the replacement +behaviour — a second error overwrites the first — is what keeps a burst from becoming a wall. + +### `App.tsx` loses one banner, keeps three + +Only `reported.error` moves out of the flex column. `offline`, `fallback` and the key-log warning +are **standing conditions**, not events: they describe a state the user is in, and belong in the +layout. Removing them is not part of this. + +## What is not touched + +- `ui/Banner.tsx` keeps its full-bleed workaround, because three callers still need it. The prop + it asks for in its own comment stays unwritten. +- Message quality. `Rail.tsx:769-799` renders `String(error)`, so a user reads + "Error: Failed to fetch". That is a real defect and a separate one — fixing it means writing + sentences at 40 call sites, not changing a channel. + +## Verification + +**Nothing here is unit-testable, and that is a constraint rather than a choice.** +`docs/ACCESSIBILITY.md:162-165`: `node --test` runs without a DOM and `--experimental-strip-types` +does not transform JSX, so no `.test.tsx` can run at all. There are no component tests in the +repository and this adds none. What holds is the typecheck, `pnpm lint` (which enforces the naming +rules `Field`/`IconButton` carry), and a pass by hand: + +1. `pnpm run typecheck`, `pnpm test`, `pnpm run lint` — the suites that exist. +2. In the browser, against the dev server: raise a success (copy a handle) and confirm it floats, + announces, and expires by itself. +3. Stop the server and let the poll fail: the error appears as a floating card, **does not** + shrink the conversation, and stays. Restart the server: the next successful poll clears it. +4. Fail a send with an action attached: the toast carries "Retry", the button is reachable with + the keyboard without focus having been stolen, and pressing it re-sends. +5. Open a dialog and raise a toast from inside it — it must still be legible above the scrim. +6. `prefers-reduced-motion: reduce` in the browser's rendering panel: entrance and exit collapse + to 1 ms through the tokens, with nothing in the component reading the preference. + +## Coordination + +A peer session is turning settings into a modal and holds `app/SettingsScreen.tsx`, +`app/Shell.tsx` and `ui/Dialog.tsx`. This work holds `state/report.ts`, `ui/Toast.tsx` and +`apps/web/package.json` — disjoint. **`App.tsx` is the one file both could want**: this change +touches lines 683-692 and the `Toasts` mount at 694. Worth telling them before starting, and worth +keeping the edit to those two spots. From 0b63e3819ce0a6cd293fe872ab2f3b14ee1e3260 Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 16:42:36 +0200 Subject: [PATCH 06/13] refactor(web): one build that belongs to no deployment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The web client shows a banner saying the server "could deliver a version that exfiltrates your keys. No browser API fixes that." Making that sentence false means letting somebody check the delivered code against something the delivering server does not control — a manifest of hashes published by CI. That was impossible while three variables were substituted into the bundle at build time: every deployment produced different bytes, so a manifest could only ever describe one instance. Measured before and after, which is the only reason to believe any of this: two builds with completely different configuration used to differ in four files. They are now byte-identical, all 226 of them. - `VITE_API_URL` is gone. The client asks the origin that served it, which `deploy/` already arranges through Caddy, and development gets the same code path through a Vite proxy rather than a second one. The desktop shell is the one target whose own origin names nothing reachable, so it keeps a literal — the same one `tauri.conf.json` already pins, and `csp.test.ts` fails if the two disagree. - `VITE_LOG_PUBKEY` leaves the web bundle and stays on the desktop. `pinning.ts` already argued that on the web the pin is "not a defence against the party that builds the bundle": the server ships the pin along with the code it constrains. What it did buy — a substitution that breaks every client at once instead of silently — is what a verifiable build provides, and provides better. - `VITE_MEDIA_URL` stays, and is the acknowledged exception. A media host has to be named in the policy, so a deployment configuring calls stops matching the published build: verifiable or calls, not both, until the media server sits behind the same origin. `connect-src` is now `'self'` and nothing else, which is strictly tighter than naming an origin. It covers the WebSocket too — CSP level 3 matches `wss:` under an `https:` document — and that was the doubtful part, so it was checked in a browser rather than read in a specification: the API answers 200 on the page's own origin and `/v1/gateway` opens. `socketUrl` exists because `BASE_URL.replace(/^http/, "ws")` was fine while the base was absolute and is not now: an empty base leaves a bare path, and `new WebSocket("/v1/gateway")` throws at the one moment the real-time session is being opened. What this simplifies rather than complicates: the deployment no longer takes the domain as a build argument, and changing it is a restart instead of a rebuild. `docs/DEPLOY.md` said the opposite in a section of its own; it says this instead. --- .env.example | 5 +++ apps/web/src/lib/api.ts | 65 +++++++++++++++++++++++++++++++++++- apps/web/src/lib/csp.test.ts | 32 +++++++++++++----- apps/web/src/lib/csp.ts | 30 +++++++++++++---- apps/web/src/lib/gateway.ts | 4 +-- apps/web/src/lib/pinning.ts | 31 +++++++++++++---- apps/web/vite.config.ts | 35 +++++++++++++++++-- deploy/.env.example | 13 -------- deploy/Dockerfile.web | 22 ++++++------ deploy/docker-compose.yml | 11 +++--- docs/DEPLOY.md | 41 +++++++++++++---------- scripts/dev-env.sh | 6 ++-- scripts/dev-web.sh | 4 +-- 13 files changed, 220 insertions(+), 79 deletions(-) diff --git a/.env.example b/.env.example index eb7d6bbe..f63df619 100644 --- a/.env.example +++ b/.env.example @@ -5,6 +5,11 @@ SERVER_ADDR=127.0.0.1:8787 # The transparency log's public key, base64, 32 bytes — as printed by the server on first boot. # +# **No longer compiled into the web client**, and the variable is kept only for the desktop build +# and for whoever verifies a log head by hand. `apps/web/src/lib/pinning.ts` argues why: on the web +# the server ships the pin along with the code it constrains, so it was never a defence there, and +# taking it out is what makes every deployment's bundle byte-identical. +# # Optional, and empty here on purpose. Set it and the client refuses any log head signed by a # different key, which is the only check that works on a first contact with a server: everything # else compares the server against its own past. It closes that hole in the **desktop binary**, diff --git a/apps/web/src/lib/api.ts b/apps/web/src/lib/api.ts index 14580b43..d89a76f5 100644 --- a/apps/web/src/lib/api.ts +++ b/apps/web/src/lib/api.ts @@ -8,6 +8,7 @@ import type { Admission } from "./call"; import type { DeviceCipher } from "./cipher"; import { fromBase64, toBase64, toHex } from "./keys"; +import { isTauri } from "./platform"; import type { AttestedDevice } from "./wasm"; /** See the note about `buffer` in `keys.ts`. */ @@ -15,7 +16,69 @@ function buffer(bytes: Uint8Array): BufferSource { return bytes as unknown as BufferSource; } -export const BASE_URL = import.meta.env.VITE_API_URL ?? "http://127.0.0.1:8787"; +/** + * Where the delivery service is, and why this is no longer compiled in. + * + * # The empty string is the answer, and it is not a fallback + * + * On the web the API shares this page's origin — `deploy/` puts Caddy in front of both, and the + * development server proxies `/v1` to make the same thing true there. So the base is relative, and + * `fetch("/v1/…")` reaches the right place without anything being configured. + * + * That is what it buys, and the reason is not tidiness: `VITE_API_URL` used to be **substituted + * into the bundle at build time**, so every deployment produced different bytes and no published + * manifest of hashes could describe more than one instance. Three files out of two hundred and + * twenty-six changed with it. Taking it out is what lets one build be checked against one manifest + * by anybody, self-hosted deployments included — see `docs/THREAT-MODEL.md` on what that check is + * and is not. + * + * # Why an injected global rather than an import + * + * The desktop shell is the one target where the page's origin says nothing: it is `tauri://`, and + * the server is elsewhere. The native side sets `__WHISPEE_API__` before the webview runs, which + * keeps the address out of the bytes this file compiles to. + * + * A hostile web server could inject that global too. It gains nothing by it: it is already serving + * every line of this application, so redirecting the API is not a power it lacked. + */ +export const BASE_URL = apiBase(); + +/** + * The address the desktop shell reaches, and the reason a literal here costs nothing. + * + * Compiled in, but **not configurable**, which is the distinction that matters: every build + * contains this same string, so it changes no bytes between deployments. `tauri.conf.json` already + * pins the same origin in its own policy, and `csp.test.ts` fails if the two disagree — so this is + * not a new coupling, it is the existing one written where the code can read it. + * + * A desktop build aimed at another server would set `__WHISPEE_API__` from the native side. Nothing + * does today, and inventing the mechanism before there is a caller would be inventing the wrong + * one. + */ +const DESKTOP_API = "http://127.0.0.1:8787"; + +function apiBase(): string { + const injected = (globalThis as { __WHISPEE_API__?: unknown }).__WHISPEE_API__; + if (typeof injected === "string" && injected !== "") return injected.replace(/\/+$/, ""); + + // The packaged shell is loaded from `tauri://`, so its own origin names nothing reachable. + if (isTauri()) return DESKTOP_API; + + return ""; +} + +/** + * The WebSocket URL for a path, whichever way the base is expressed. + * + * `BASE_URL.replace(/^http/, "ws")` was enough while the base was absolute. It is not now: an empty + * base leaves a bare path, and `new WebSocket("/v1/gateway")` throws `SyntaxError` — at the one + * moment the real-time session is being opened, with a message naming nothing. + */ +export function socketUrl(path: string): string { + const origin = BASE_URL === "" ? globalThis.location.origin : BASE_URL; + + return `${origin.replace(/^http/, "ws")}${path}`; +} export class ApiError extends Error { constructor( diff --git a/apps/web/src/lib/csp.test.ts b/apps/web/src/lib/csp.test.ts index 8d06b3cd..fb0afdb2 100644 --- a/apps/web/src/lib/csp.test.ts +++ b/apps/web/src/lib/csp.test.ts @@ -14,7 +14,14 @@ import { csp } from "./csp.ts"; * be loud, and the only place it can be made loud is here. */ -/** The API origin the desktop configuration is pinned to, so both sides describe the same server. */ +/** + * The API origin the desktop configuration is pinned to. + * + * It is a **desktop-only** source now. The web policy names no origin at all — the API is reached + * on the page's own origin, so `'self'` covers it — while the packaged shell is loaded from + * `tauri://` and has to be told where the server is. That difference is a transport difference, + * exactly like `ipc:`, which is why it belongs in the list below rather than in both policies. + */ const DESKTOP_API = "http://127.0.0.1:8787"; /** @@ -25,7 +32,14 @@ const DESKTOP_API = "http://127.0.0.1:8787"; * bundle. Adding to this list is a deliberate act; that is why it is a list and not a filter. */ const DESKTOP_ONLY: Record = { - "connect-src": ["ipc:", "http://ipc.localhost"], + "connect-src": [ + "ipc:", + "http://ipc.localhost", + // The two forms of the API origin. The web build stopped naming them when the client began + // asking its own origin; the desktop cannot, because its own origin is `tauri://`. + DESKTOP_API, + DESKTOP_API.replace(/^http/, "ws"), + ], "img-src": ["asset:", "http://asset.localhost"], }; @@ -53,7 +67,7 @@ function desktopPolicy(): string { } test("both targets declare exactly the same set of directives", () => { - const web = [...parse(csp(DESKTOP_API)).keys()].sort(); + const web = [...parse(csp()).keys()].sort(); const desktop = [...parse(desktopPolicy()).keys()].sort(); assert.deepEqual( @@ -64,7 +78,7 @@ test("both targets declare exactly the same set of directives", () => { }); test("every directive allows the same sources, apart from the declared desktop transports", () => { - const web = parse(csp(DESKTOP_API)); + const web = parse(csp()); const desktop = parse(desktopPolicy()); for (const [name, webSources] of web) { @@ -93,7 +107,7 @@ test("the desktop policy allows the blob urls the image previews are made of", ( test("neither target ever allows a script source beyond this origin", () => { for (const [target, policy] of [ - ["web", csp(DESKTOP_API)], + ["web", csp()], ["desktop", desktopPolicy()], ] as const) { assert.deepEqual( @@ -113,7 +127,7 @@ test("neither target ever allows a script source beyond this origin", () => { */ test("media-src is blob: and nothing else, on both targets", () => { for (const [target, policy] of [ - ["web", csp(DESKTOP_API)], + ["web", csp()], ["desktop", desktopPolicy()], ] as const) { assert.deepEqual( @@ -132,7 +146,7 @@ test("media-src is blob: and nothing else, on both targets", () => { */ test("workers come from this origin and nowhere else, on both targets", () => { for (const [target, policy] of [ - ["web", csp(DESKTOP_API)], + ["web", csp()], ["desktop", desktopPolicy()], ] as const) { assert.deepEqual( @@ -157,8 +171,8 @@ test("workers come from this origin and nowhere else, on both targets", () => { * once because it would live in the default. */ test("the media origin appears only when a build asks for one", () => { - const without = parse(csp(DESKTOP_API)).get("connect-src") ?? new Set(); - const with_ = parse(csp(DESKTOP_API, "https://media.example")).get("connect-src") ?? new Set(); + const without = parse(csp()).get("connect-src") ?? new Set(); + const with_ = parse(csp("https://media.example")).get("connect-src") ?? new Set(); assert.ok(with_.has("wss://media.example"), "the signalling socket has no origin to reach"); // The HTTP form is not redundant, and this assertion is here because the first version of this diff --git a/apps/web/src/lib/csp.ts b/apps/web/src/lib/csp.ts index 01d8282c..440f046d 100644 --- a/apps/web/src/lib/csp.ts +++ b/apps/web/src/lib/csp.ts @@ -33,11 +33,16 @@ * sees nothing, and the message does not name the cause. Same trap as the one documented on the * CORS header list, server side. * - * # Why `connect-src` carries two origins + * # Why `connect-src` names no origin any more * - * `connect-src` does **not** infer the `ws://` origin from the matching `http://` one. Either one - * alone would cut half the client — requests or the real-time session — without the other - * signalling it. + * It used to carry two — the API's `http://` form and its `ws://` form, because `connect-src` does + * not infer one from the other. Both are gone: the API is now reached relatively, on the page's own + * origin, so `'self'` covers it. CSP level 3 extends `'self'` to `wss:` under an `https:` document, + * which is what the second origin was for. + * + * That is tighter than what it replaces, and it is also what lets one build serve every deployment: + * the policy no longer depends on `VITE_API_URL`, which was substituted into `index.html` at build + * time and made every instance's bytes different. * * # No nonce, and that is hardening * @@ -48,8 +53,7 @@ * No browser policy stands in the way — only the desktop app, whose code is packaged into the * installed binary, closes that path. */ -export function csp(api: string, media?: string): string { - const websocket = api.replace(/^http/, "ws"); +export function csp(media?: string): string { // The media server is a second origin, and it is absent from most deployments: a build with no // media server must not widen its policy for a host it will never contact. Empty rather than a // default, so the directive is exactly as wide as the deployment is. @@ -74,7 +78,19 @@ export function csp(api: string, media?: string): string { // Tailwind injects its styles at runtime. The residual risk of a CSS injection is nowhere // near that of a script. "style-src 'self' 'unsafe-inline'", - `connect-src 'self' ${api} ${websocket}${relay}`, + // **`'self'` and no origin, which is both tighter and the reason one build serves every + // deployment.** The API shares this page's origin — `deploy/` puts Caddy in front of both, and + // the development server proxies `/v1` — so naming a host would be naming the host we are + // already on. + // + // It covers the WebSocket too: CSP level 3 matches `wss:` under `'self'` when the document is + // `https:`, and `ws:` when it is `http:`. That is what the two spelled-out origins used to be + // for, and it is why they are not missed. + // + // What this buys beyond tightness: the policy no longer depends on `VITE_API_URL`, which used + // to be substituted into `index.html` at build time and made every deployment's bytes + // different. See `api.ts` on why that mattered. + `connect-src 'self'${relay}`, // `blob:` is for image previews, and it is not a hole reopening. // // What a received image gets displayed as is a canvas re-encoding of what an image decoder diff --git a/apps/web/src/lib/gateway.ts b/apps/web/src/lib/gateway.ts index 56db3f94..6b4b20de 100644 --- a/apps/web/src/lib/gateway.ts +++ b/apps/web/src/lib/gateway.ts @@ -29,7 +29,7 @@ * discovered along the way is added with a `subscribe` frame, without reopening the connection or * signing another challenge. */ -import { BASE_URL, type Api, type GatewayChallenge } from "./api"; +import { socketUrl, type Api, type GatewayChallenge } from "./api"; import { fromBase64, fromHex, toBase64, toHex } from "./keys"; export interface GatewayHandlers { @@ -179,7 +179,7 @@ export class Gateway { /** One session, from open to close. Resolves on close, rejects on error. */ private session(): Promise { return new Promise((resolve, reject) => { - const url = `${BASE_URL.replace(/^http/, "ws")}/v1/gateway`; + const url = socketUrl("/v1/gateway"); const socket = new WebSocket(url); socket.binaryType = "arraybuffer"; this.socket = socket; diff --git a/apps/web/src/lib/pinning.ts b/apps/web/src/lib/pinning.ts index b4b72f78..0d890a62 100644 --- a/apps/web/src/lib/pinning.ts +++ b/apps/web/src/lib/pinning.ts @@ -34,22 +34,41 @@ import { fromBase64 } from "./keys"; /** - * The pinned key, or `undefined` when this build was compiled without one. + * The pinned key, or `undefined` when nothing pinned one. * - * Read once. A malformed value is a build-time mistake and it is loud: refusing to start beats - * running with a pin that silently checks nothing, which is the failure this whole module exists - * to avoid — a check that looks present and is not. + * Read once. A malformed value is loud: refusing to start beats running with a pin that silently + * checks nothing, which is the failure this whole module exists to avoid — a check that looks + * present and is not. */ export const PINNED_LOG_KEY: Uint8Array | undefined = readPin(); +/** + * # Why this is injected and no longer compiled in + * + * It used to be `import.meta.env.VITE_LOG_PUBKEY`, substituted into the bundle by Vite. That meant + * every deployment produced different bytes, and a published manifest of file hashes could + * therefore describe only one of them — which is what stood between this project and a client + * anybody can check against the source it claims to be built from. + * + * Taking it out of the **web** bundle costs less than it looks, and the paragraphs above say why: + * there the server ships the pin along with the code the pin constrains, so it was never a defence + * against the party building the bundle. What it did buy — turning a silent substitution into one + * that breaks every client at once — is exactly what a verifiable build provides, and provides + * better: a mismatch becomes something a reader can detect deliberately rather than something they + * notice because the application stopped working. + * + * On the **desktop** the pin keeps its full value, and keeps it for the same reason it had it: the + * interface lives inside a signed, reproducible artefact. The native side sets the global before + * the webview runs, so the value is in the binary rather than in these bytes. + */ function readPin(): Uint8Array | undefined { - const raw = import.meta.env.VITE_LOG_PUBKEY; + const raw = (globalThis as { __WHISPEE_LOG_KEY__?: unknown }).__WHISPEE_LOG_KEY__; if (typeof raw !== "string" || raw === "") return undefined; const key = fromBase64(raw); if (key.length !== 32) { throw new Error( - `VITE_LOG_PUBKEY must be 32 bytes of base64 Ed25519 public key, got ${key.length}`, + `the pinned log key must be 32 bytes of base64 Ed25519 public key, got ${key.length}`, ); } return key; diff --git a/apps/web/vite.config.ts b/apps/web/vite.config.ts index c79f72e2..0585d86e 100644 --- a/apps/web/vite.config.ts +++ b/apps/web/vite.config.ts @@ -30,10 +30,16 @@ export default defineConfig(({ mode }) => ({ order: "pre" as const, handler: (html: string) => { const environment = loadEnv(mode, process.cwd(), "VITE_"); - const api = environment.VITE_API_URL ?? "http://127.0.0.1:8787"; - // Absent by default: a deployment with no media server must not carry its origin. - return html.replace("%CSP%", csp(api, environment.VITE_MEDIA_URL || undefined)); + // The API's origin is no longer read here, and that is the point: it used to be + // substituted into this file, so two deployments produced two different `index.html` + // and no published manifest of hashes could describe more than one of them. The policy + // says `'self'` now — see `src/lib/csp.ts`. + // + // The media server is the one origin still able to vary, and a deployment that sets it + // gives up matching the published build. That trade is written down in + // `docs/THREAT-MODEL.md` rather than left here. + return html.replace("%CSP%", csp(environment.VITE_MEDIA_URL || undefined)); }, }, }, @@ -55,6 +61,29 @@ export default defineConfig(({ mode }) => ({ // mode `src/lib/csp.ts` describes, arrived at by convenience rather than by misconfiguration. // Refusing to start says which port is taken, which is a sentence somebody can act on. strictPort: true, + + /** + * Development reaches the API through here rather than across origins. + * + * The client no longer carries the server's address: it asks its own origin, because in a + * deployment Caddy serves both. Without this proxy that would be true everywhere except on + * the machine where the code is written — one code path in production and another in + * development is how a bug ships that nobody could reproduce. + * + * `ws: true` because `/v1/gateway` is a WebSocket upgrade, and a proxy that forwards the + * requests but not the upgrade leaves the real-time session failing while everything else + * looks well. + * + * `WHISPEE_API` for the branch-scoped port `scripts/dev-env.sh` hands out; the default is the + * one a plain `pnpm run dev` expects. + */ + proxy: { + "/v1": { + target: process.env.WHISPEE_API ?? "http://127.0.0.1:8787", + changeOrigin: true, + ws: true, + }, + }, }, build: { diff --git a/deploy/.env.example b/deploy/.env.example index 0f8251b8..5b51a238 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -27,19 +27,6 @@ POSTGRES_PASSWORD= # ----------------------------------------------------------------------------- optional -# The transparency log's public key, base64, 32 bytes — printed by the server on its first boot, -# so there is nothing to set until then and an unset value leaves behaviour exactly as it was. -# -# Set it and the client refuses any log head signed by a different key. That is the only check -# that works on a first contact with a server: everything else compares the server against its -# own past. It closes the hole in the **desktop binary**, whose interface is packaged in a signed -# artefact. On the web this deployment serves both the pin and the code it constrains, so there -# it turns a silent substitution into one that breaks every client at once — worth having, and -# not a defence. -# -# Build-time. Setting it means `docker compose build web`. -VITE_LOG_PUBKEY= - # Ceiling on the bytes one account may keep here: its history vault plus the attachments it # uploaded. 256 MiB when unset; `0` removes the ceiling and leaves the vault unbounded. # Envelopes are counted in neither case — bounding them per account takes anonymous tokens, see diff --git a/deploy/Dockerfile.web b/deploy/Dockerfile.web index 79b5c44e..fea50f00 100644 --- a/deploy/Dockerfile.web +++ b/deploy/Dockerfile.web @@ -14,26 +14,24 @@ # already hit twice, documented on both `allowed_origins` and `src/lib/csp.ts`: a browser that # refuses **before** sending, so the server logs nothing and the message names no cause. # -# # Why the API URL is a build argument and not an environment variable +# # Why this image takes almost no build arguments any more # -# The Content-Security-Policy is computed into `index.html` at build time (`src/lib/csp.ts`), and -# `connect-src` must name this deployment's exact origin — both the `https://` form and the -# `wss://` one, which it does not infer. Changing the domain therefore means rebuilding the -# client, not restarting a container. `docker compose build` is where that happens. +# It used to need the deployment's own domain, because the API address and the +# Content-Security-Policy were both frozen into the bundle. They are not: the client asks its own +# origin, and the policy says `connect-src 'self'`. So **changing the domain no longer requires +# rebuilding the client** — and, more importantly, two deployments of the same commit now produce +# byte-identical bundles, which is what lets a published manifest of hashes describe all of them. +# +# `VITE_MEDIA_URL` is the one that remains, and it is the exception that proves the rule: a +# deployment configuring calls has to name the media origin in its policy, and its bundle then +# stops matching the published build. See `docs/THREAT-MODEL.md`. FROM node:22-bookworm-slim AS build -# The origin this deployment answers on, scheme included: https://whispee.example -ARG VITE_API_URL # The media server's origin, for calls. Empty by default, and it must stay that way for a # deployment that runs none: an empty value keeps its host out of `connect-src` instead of # widening the policy for a service that will never be contacted. ARG VITE_MEDIA_URL= -# The transparency log's public key, base64, as the server prints it on first boot. Optional. -# On the web it does not close the substitution hole — this image serves both the pin and the -# code it constrains — but it turns a silent substitution into one that breaks every client at -# once. See `.env.example` for the full argument. -ARG VITE_LOG_PUBKEY= # The version is pinned rather than left to corepack's default, for the reason # `rust-toolchain.toml` gives at length about the compiler: "recent" is not a version. diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 679f6eac..01f437db 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -72,12 +72,13 @@ services: context: .. dockerfile: deploy/Dockerfile.web args: - # Build-time, not runtime: the Content-Security-Policy is computed into `index.html` and - # must name this exact origin. Changing the domain means `docker compose build web`, not - # a restart. - VITE_API_URL: https://${WHISPEE_DOMAIN:?set WHISPEE_DOMAIN in deploy/.env} + # The domain is **not** here any more, and its absence is the feature: the client reaches + # the API on whatever origin served it, so this image is the same bytes for every + # deployment of a given commit. Changing the domain no longer means rebuilding the client. + # + # `VITE_MEDIA_URL` is the one exception, because a media host has to be named in the + # policy. Setting it means this build no longer matches the published one. VITE_MEDIA_URL: ${VITE_MEDIA_URL:-} - VITE_LOG_PUBKEY: ${VITE_LOG_PUBKEY:-} restart: unless-stopped depends_on: - server diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index 433da8d7..0121e4f2 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -63,28 +63,35 @@ docker compose logs server | grep log_key It is printed rather than fetched because the only route carrying it, `/v1/log/sth`, requires a signed request — an operator has no signing device yet at this point. -Put it in `.env` as `VITE_LOG_PUBKEY` and rebuild the client: +There is nothing to put it in on the web any more, and that is deliberate. The pin used to be +compiled into the bundle as `VITE_LOG_PUBKEY`; it is not, because on the web it was never a +defence — this deployment served both the pin and the code the pin constrains, so a server willing +to forge a log was willing to serve a build that trusted it. What it did buy, a substitution that +broke every client at once instead of silently, is replaced by something stronger: a build anybody +can check against the published manifest. See `apps/web/src/lib/pinning.ts`. -```sh -docker compose up -d --build web -``` +The desktop binary still pins, and there it means what it always meant: the interface is inside a +signed, reproducible artefact the server cannot reach. + +Keep the key anyway. It is what a verifier compares a log head against, and it is the value +`scripts/verify-web.sh` will want. + +## Changing the domain no longer rebuilds the client -What this buys, precisely: a client with the pin refuses any log head signed by a different key. -That is the only check that works on a **first** contact with a server — every other one compares -the server against its own past. In the desktop binary, whose interface is packaged in a signed -artefact, it closes the substitution hole. On the web it does not: this deployment serves both -the pin and the code the pin constrains. There it turns a silent substitution into one that -breaks every client at once, which is worth having and is not a defence. +It used to. The API address and the Content-Security-Policy were both frozen into `index.html` at +build time, so a new domain meant `docker compose build web`. -## Changing the domain means rebuilding the client +Neither is now: the client reaches the API on whatever origin served it — Caddy puts both behind +one — and the policy says `connect-src 'self'`, which is strictly tighter than naming a host. A +domain change is a restart. -The Content-Security-Policy is computed into `index.html` at build time -(`apps/web/src/lib/csp.ts`), and its `connect-src` has to name this deployment's exact origin — -both the `https://` form and the `wss://` one, which it does not infer. `VITE_API_URL` is -therefore a build argument, not an environment variable. Change the domain and run -`docker compose build web`. +The point of that is not convenience. **Two deployments of the same commit now produce +byte-identical bundles**, which is what makes a single published manifest of hashes able to +describe all of them, self-hosted included. -The same applies to `VITE_LOG_PUBKEY` and `VITE_MEDIA_URL`. +`VITE_MEDIA_URL` is the one build argument left, and it is the exception: a media host has to be +named in the policy. A deployment that sets it gives up matching the published build — verifiable +or calls, not both, until the media server sits behind the same origin. ## Updating diff --git a/scripts/dev-env.sh b/scripts/dev-env.sh index b96f4fa0..1d7fa324 100755 --- a/scripts/dev-env.sh +++ b/scripts/dev-env.sh @@ -25,7 +25,9 @@ # from one index: # # - `SERVER_ADDR`, where the server listens. -# - `VITE_API_URL`, where the client looks. It also determines the CSP computed into +# - `WHISPEE_API`, where the development server proxies `/v1`. The client no longer carries an +# address at all — it asks its own origin, and Vite forwards. See `vite.config.ts`. +# Historically this was `VITE_API_URL`, which also determined the CSP computed into # `index.html` (`apps/web/vite.config.ts`), and a `connect-src` that omits the port blocks # the request in the browser before it is sent — no server log, no cause named. See the # header of `apps/web/src/lib/csp.ts`. @@ -107,7 +109,7 @@ web_port=$((5173 + index)) printf "export DATABASE_URL='%s'\n" "${base_url%/*}/${database}${query}" printf "export SERVER_ADDR='127.0.0.1:%s'\n" "$server_port" -printf "export VITE_API_URL='http://127.0.0.1:%s'\n" "$server_port" +printf "export WHISPEE_API='http://127.0.0.1:%s'\n" "$server_port" printf "export ALLOWED_ORIGINS='http://127.0.0.1:%s,http://localhost:%s'\n" "$web_port" "$web_port" printf "export WEB_PORT='%s'\n" "$web_port" printf "export WHISPEE_DEV_DATABASE='%s'\n" "$database" diff --git a/scripts/dev-web.sh b/scripts/dev-web.sh index 203e7d36..3433210c 100755 --- a/scripts/dev-web.sh +++ b/scripts/dev-web.sh @@ -2,7 +2,7 @@ # # Runs the web client on the port this branch owns, pointed at this branch's server. # -# `scripts/dev-env.sh` decides both and explains why they travel together: `VITE_API_URL` is +# `scripts/dev-env.sh` decides both and explains why they travel together: `WHISPEE_API` is # read by `apps/web/src/lib/api.ts` *and* computed into the page's CSP, and `WEB_PORT` has to # match the `ALLOWED_ORIGINS` the server was started with. Starting the client any other way — # `pnpm run dev` in `apps/web` — reverts to the defaults and reaches the wrong server, or none. @@ -11,6 +11,6 @@ set -euo pipefail cd "$(git rev-parse --show-toplevel)" eval "$(scripts/dev-env.sh)" -echo "dev-web: $WHISPEE_DEV_BRANCH -> port $WEB_PORT, api $VITE_API_URL" >&2 +echo "dev-web: $WHISPEE_DEV_BRANCH -> port $WEB_PORT, api $WHISPEE_API" >&2 cd apps/web exec pnpm run dev "$@" From af1e79d94e4db6f49bd4159b5d85571e75eb52cb Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 16:58:38 +0200 Subject: [PATCH 07/13] feat(release): publish what the web client is, where the server cannot reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client tells its user that the server "could deliver a version that exfiltrates your keys. No browser API fixes that." Nothing removes that sentence, but something answers it: a list of hashes published somewhere the delivering server does not control, so that a substitution stops being invisible. `scripts/release-web.sh` produces the list. It refuses a dirty tree for the reason `release.sh` does — a manifest matching no commit is one nobody can rebuild to compare — and exports `SOURCE_DATE_EPOCH` from the commit, which changes nothing measurable today and means the day a dependency starts embedding a timestamp, it embeds the same one everywhere. `.github/workflows/release.yml` is where the mechanism actually acquires its meaning, and it is not the script. A manifest built anywhere says "somebody hashed some files"; `attest-build-provenance` binds it to this commit and this workflow through Sigstore, and nobody outside Actions — the maintainer included — can produce that binding. The repository's own Ed25519 release key would not do: `verify-release.sh` already says it lives in the repository, so whoever controls the repository can replace it, and a manifest meant to be independent of the party serving the code cannot rest on a key that party holds. It is the first workflow here to declare `permissions:` — no other one does — because `id-token: write` is what lets the runner prove which workflow it is. `scripts/verify-web.sh` is the other end, and it states its own ceiling on success rather than in a header nobody reads: it establishes that a server serves the build a manifest describes, not that the server is honest. One willing to serve one build to the world and another to one person passes this for everybody who runs it and fails only for the person it is attacking — who is the person not running it. That is what the extension is for, and why this script is the version a human can run today rather than the answer. The manifest describes every deployment at once, which is only true because of the commit before this one: the bundle no longer carries any deployment's configuration, so two instances of a commit serve identical bytes. Without that, this would have been a service to the official deployment and to nobody else. --- .github/workflows/release.yml | 105 ++++++++++++++++++++++++++++++++++ scripts/release-web.sh | 101 ++++++++++++++++++++++++++++++++ scripts/verify-web.sh | 101 ++++++++++++++++++++++++++++++++ 3 files changed, 307 insertions(+) create mode 100644 .github/workflows/release.yml create mode 100755 scripts/release-web.sh create mode 100755 scripts/verify-web.sh diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 00000000..c6bd039e --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,105 @@ +# Publishes the manifest of the web client, attested to this repository. +# +# # Why this workflow is the point, and not the script it runs +# +# `scripts/release-web.sh` can be run by anybody, which is what makes the build verifiable. But a +# manifest produced on a laptop says only "somebody hashed some files"; the reader has no way to +# tell it from a manifest describing a hostile build. What closes that is `attest-build-provenance`: +# it binds the artefact to **this commit** and to **this workflow**, signed through Sigstore, and +# nobody — the maintainer included — can produce that binding outside GitHub Actions. +# +# That is the whole reason the check means anything. The repository already carries an Ed25519 key +# for the desktop release, and `verify-release.sh` says in as many words what it is worth: the key +# lives in the repository, so whoever controls the repository can replace it. For a manifest whose +# job is to be independent of the party serving the code, a key the same party carries is not +# independence. +# +# # Why a tag and not every push +# +# A manifest is a claim about a build somebody can install. `dev` moves several times a day and +# nothing deploys from it; a manifest per commit would be a list of hashes nobody could act on, +# and would make the released ones harder to find. +name: Release + +on: + push: + tags: + - "v*" + # For rehearsing the workflow before there is a tag to rehearse it on. It publishes nothing: + # `gh release` only runs on a tag ref. + workflow_dispatch: + +# Absent from every other workflow in this repository, and required by two of the steps below. +# `id-token` is what lets the runner prove to Sigstore which workflow it is; `attestations` is what +# lets it record the result. `contents: write` is for creating the release itself. +permissions: + contents: write + id-token: write + attestations: write + +jobs: + web: + name: Web client manifest + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v5 + + - uses: actions/setup-node@v5 + with: + node-version: 22 + # The cache keys on a lockfile path this repository does not have at the root, and a + # half-hit cache is a slower build with a confusing log. + package-manager-cache: false + + # Pinned rather than left to corepack's default, the same statement `deploy/Dockerfile.web` + # makes: `apps/web/package.json` declares no `packageManager`, so nothing else in the tree + # records which pnpm produced `pnpm-lock.yaml`. + # + # It matters more here than anywhere else. A manifest is a claim that a given commit + # produces given bytes; if the tool that produces them is whatever version happened to be + # current that day, the claim is about a build nobody can reproduce. + - name: Enable pnpm + run: corepack enable && corepack prepare pnpm@11.22.0 --activate + + - name: Build and hash + run: scripts/release-web.sh + + # Everything worth reading in the log, because a mismatch reported weeks later is + # investigated from here. + - name: What was built + run: | + cat release/web/BUILD-INFO + echo + head -5 release/web/WEB-SHA256SUMS + + - uses: actions/attest-build-provenance@v3 + with: + subject-path: release/web/WEB-SHA256SUMS + + # The manifest and nothing else. The bundle itself is not published: a reader does not need + # our copy of the files, they need the hashes to compare against the copy their own browser + # was served — and publishing the bundle would invite verifying the wrong thing. + - name: Publish + if: startsWith(github.ref, 'refs/tags/') + env: + GH_TOKEN: ${{ github.token }} + run: | + gh release create "${GITHUB_REF_NAME}" \ + --title "${GITHUB_REF_NAME}" \ + --notes "Manifest of the web client for \`${GITHUB_SHA}\`. + + Check a deployment against it: + + \`\`\`sh + gh release download ${GITHUB_REF_NAME} --pattern WEB-SHA256SUMS + gh attestation verify WEB-SHA256SUMS --repo ${GITHUB_REPOSITORY} + scripts/verify-web.sh https://your.deployment WEB-SHA256SUMS + \`\`\` + + The second line is the one that matters: it establishes that this manifest came out of + this repository's workflow rather than out of somebody's laptop. Verifying the hashes + without it checks that a server is consistent with a file you were handed, which is not + the same claim." \ + release/web/WEB-SHA256SUMS \ + release/web/BUILD-INFO diff --git a/scripts/release-web.sh b/scripts/release-web.sh new file mode 100755 index 00000000..f581ed7f --- /dev/null +++ b/scripts/release-web.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# +# Builds the web client and lists what it produced, hash by hash. +# +# # What this is for +# +# The client tells its user, in a banner, that the server "could deliver a version that +# exfiltrates your keys". That is true of every web application and no browser API fixes it. What +# does help is a list of hashes published somewhere the delivering server does not control: then a +# reader — or an extension — can compare the bytes their browser received against the bytes this +# commit produces, and a substitution stops being invisible. +# +# This script produces that list. `.github/workflows/release.yml` runs it on a tag and publishes +# the result with a GitHub attestation, which is what makes the manifest independent of whoever +# runs the deployment. `scripts/verify-web.sh` is the other end. +# +# # Why the manifest describes every deployment at once +# +# Because the bundle no longer carries any deployment's configuration. The API is reached on the +# page's own origin and the Content-Security-Policy says `'self'`, so two instances of the same +# commit serve byte-identical files — measured, and the reason `apps/web/src/lib/api.ts` and +# `csp.ts` are written the way they are. Before that, a manifest could only ever have described +# one instance, which would have made this whole mechanism a service to the official deployment +# and to nobody else. +# +# The exception is `VITE_MEDIA_URL`: a deployment configuring calls names a media origin in its +# policy and its `index.html` stops matching. That trade is in `docs/THREAT-MODEL.md`, and this +# script deliberately builds **without** it — the published manifest describes the build a +# verifier can reproduce, not the one a particular operator chose. +# +# # Why `SOURCE_DATE_EPOCH` +# +# For the same reason `release.sh` exports it: the commit's own date is the same for everybody who +# rebuilds this commit, where the current time is different for each of them. Vite embeds no +# timestamp today, so this changes nothing measurable — it is here so that the day a dependency +# starts embedding one, it embeds the same one everywhere. +# +# # Usage +# +# scripts/release-web.sh [output-directory] +# +# Default output is `release/web`. Nothing here is signed: the attestation happens in CI, where +# the signing identity is the workflow rather than a key somebody carries. +set -euo pipefail + +root="$(git rev-parse --show-toplevel)" +cd "$root" + +output="${1:-$root/release/web}" + +# A manifest whose content matches no commit is not verifiable: nobody would know what to rebuild +# in order to compare. Same refusal as `release.sh`, for the same reason, and it is the one check +# that cannot be added later — by then the artefact exists. +if [[ -n "$(git status --porcelain)" ]]; then + echo "error: the working tree has uncommitted changes" >&2 + echo "a manifest must match exactly one commit" >&2 + exit 1 +fi + +commit="$(git rev-parse HEAD)" + +SOURCE_DATE_EPOCH="$(git log -1 --pretty=%ct)" +export SOURCE_DATE_EPOCH + +echo "→ building the client" +# `--frozen-lockfile` because a manifest built from a resolved-on-the-fly dependency tree +# describes a build nobody else can reproduce. +(cd apps/web && pnpm install --frozen-lockfile && pnpm run build) + +rm -rf "$output" +mkdir -p "$output" + +echo "→ hashing $(find apps/web/dist -type f | wc -l) files" + +# Paths relative to the served root, so a verifier can turn each line straight into a URL. Sorted +# by `LC_ALL=C` so the file is byte-identical wherever it is produced — a manifest that differs +# only in line order would defeat its own purpose. +( + cd apps/web/dist + find . -type f -print0 \ + | LC_ALL=C sort -z \ + | xargs -0 sha256sum \ + | sed 's# \./# #' +) > "$output/WEB-SHA256SUMS" + +# What produced it. Not decoration: two of these lines are the first thing to compare when a +# rebuild does not match, and the toolchain versions are what a verifier has to install. +cat > "$output/BUILD-INFO" < " >&2 + echo " scripts/verify-web.sh https://whispee.example WEB-SHA256SUMS" >&2 + exit 2 +fi + +if [[ ! -r "$manifest" ]]; then + echo "error: cannot read $manifest" >&2 + exit 1 +fi + +origin="${origin%/}" + +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +checked=0 +missing=0 +altered=0 + +# Read the manifest rather than crawl the site: what is being asked is "does the server have these +# bytes at these paths", and a crawl would only ever find the files the server chose to link. A +# file served at a path the manifest does not list is invisible here — see the note at the end. +while read -r expected path; do + [[ -z "$path" ]] && continue + + if ! curl -fsSL --max-time 30 "$origin/$path" -o "$work/fetched" 2>/dev/null; then + echo "MISSING $path" + missing=$((missing + 1)) + continue + fi + + actual="$(sha256sum "$work/fetched" | cut -d' ' -f1)" + + if [[ "$actual" != "$expected" ]]; then + echo "ALTERED $path" + echo " expected $expected" + echo " served $actual" + altered=$((altered + 1)) + continue + fi + + checked=$((checked + 1)) +done < "$manifest" + +echo +echo "$checked file(s) matched, $altered altered, $missing missing" + +if (( altered > 0 || missing > 0 )); then + echo + echo "This deployment is not serving the build that manifest describes." >&2 + exit 1 +fi + +echo +echo "Every file this manifest lists is served byte for byte." +echo +# Stated on success rather than buried in the header, because success is the moment somebody is +# most likely to over-read the result. +echo "What that does not establish: that the manifest is genuine — check it with" +echo "\`gh attestation verify\`, against the repository, not against this server — and that the" +echo "server serves the same thing to somebody else. A file at a path the manifest does not list" +echo "is not covered by this check at all." From 04a14e26fe3174940023447e90850312f866447f Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 17:05:19 +0200 Subject: [PATCH 08/13] fix(release): pin node to the patch, or the manifest describes nobody MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rehearsal tag found this, which is what a rehearsal is for. `Build and hash` succeeded in CI, and its manifest agrees byte for byte with one built in `node:22-bookworm-slim` — the image the deployment uses. Three of the five sampled chunks also matched this development machine. Two did not: `index-*.js` and `PdfViewer-*.js` differ between node 22.21.0 and node 22.23.2, while the CSS and the remaining chunks are identical. So reproducibility holds across machines, and holds on the version of node. `node-version: 22` in the workflow and `node:22-bookworm-slim` in the Dockerfile both resolve to "whatever 22.x is current", and they happened to agree on the day this was written. That is not a property to rest a manifest on: the day they drift, a deployment stops matching its own published hashes and the mismatch looks exactly like the substitution this mechanism exists to make visible. `apps/web/.nvmrc` is now the one place that says which node, read by the workflow through `node-version-file` and repeated by hand in the Dockerfile — whose comment says to change both in the same commit. The same discipline `rust-toolchain.toml` already applies to the compiler, and for the same reason it gives: "recent" is not a version. Not fixed here, and deliberately: `test.yml` still says `node-version: 22`. It builds nothing anybody verifies, so pinning it would be a change with no argument behind it beyond symmetry. --- .github/workflows/release.yml | 12 +++++++++++- apps/web/.nvmrc | 1 + deploy/Dockerfile.web | 10 +++++++++- 3 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 apps/web/.nvmrc diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c6bd039e..734e506a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -45,9 +45,19 @@ jobs: steps: - uses: actions/checkout@v5 + # **The exact version, from `apps/web/.nvmrc`, and this is the line the manifest rests on.** + # + # Measured: node 22.21.0 and node 22.23.2 produce different bytes for two of the fourteen + # generated files — `index-*.js` and `PdfViewer-*.js` — while the CSS and the other chunks + # match. A manifest built here and a deployment built elsewhere would therefore disagree on + # two files, and the disagreement would look exactly like an attack. + # + # `node-version: 22` resolves to whatever 22.x the runner has that week. It happened to + # match `deploy/Dockerfile.web` on the day this was written, which is not a property anybody + # should rely on — it is the kind of agreement that holds until it silently stops. - uses: actions/setup-node@v5 with: - node-version: 22 + node-version-file: apps/web/.nvmrc # The cache keys on a lockfile path this repository does not have at the root, and a # half-hit cache is a slower build with a confusing log. package-manager-cache: false diff --git a/apps/web/.nvmrc b/apps/web/.nvmrc new file mode 100644 index 00000000..c9471194 --- /dev/null +++ b/apps/web/.nvmrc @@ -0,0 +1 @@ +22.23.2 diff --git a/deploy/Dockerfile.web b/deploy/Dockerfile.web index fea50f00..a6c398ef 100644 --- a/deploy/Dockerfile.web +++ b/deploy/Dockerfile.web @@ -26,7 +26,15 @@ # deployment configuring calls has to name the media origin in its policy, and its bundle then # stops matching the published build. See `docs/THREAT-MODEL.md`. -FROM node:22-bookworm-slim AS build +# **Pinned to the patch, and kept in step with `apps/web/.nvmrc` by hand.** +# +# Not tidiness: node 22.21.0 and node 22.23.2 produce different bytes for two of the generated +# chunks. A deployment built on a floating `node:22` would drift away from the manifest +# `.github/workflows/release.yml` publishes, and the drift would be indistinguishable from a +# substitution — the one failure this whole mechanism exists to make visible. +# +# Raising it is a deliberate act: change `.nvmrc` in the same commit, or the two stop agreeing. +FROM node:22.23.2-bookworm-slim AS build # The media server's origin, for calls. Empty by default, and it must stay that way for a # deployment that runs none: an empty value keeps its host out of `connect-src` instead of From a72fe9ecc31edc2fb366f2960b3953bc7705520d Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 17:11:22 +0200 Subject: [PATCH 09/13] feat(extension): check the served code, and say so where the server cannot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manifest published on a tag is only worth what somebody does with it. `scripts/verify-web.sh` is the version a human runs, and it has a ceiling it states itself: a server willing to serve one build to the world and another to one person passes it for everybody who runs it and fails only for the person it is attacking — who is the person not running it. This is the half that runs for that person. **The verdict is in the toolbar icon and never in the page.** That is the reason an extension exists rather than a badge in the application: everything a page displays is drawn by the server being checked, so a "verified" mark there would be forged by exactly the server it is meant to catch. **The manifest is fetched from GitHub, never from the inspected origin.** A server handing over both the code and the hashes of that code has certified itself, which is the defect the whole mechanism removes — the same one the threat model records about the transparency log being signed by the server it watches. `verify.js` holds the comparison and takes every input as an argument, so it runs under `node --test` without a browser. Nine tests, and the four that matter are the failures: an altered byte, a script at a path the manifest never described, a resource pulled from another origin, and one that cannot be re-fetched. Each is a way past a hash check that a verifier walking only the manifest would report as clean. Checking nothing answers `unknown` rather than `ok`, which is the single most dangerous thing it could display. The compromise it cannot avoid, written at the top of `background.js` rather than discovered: Chrome exposes no way to read the bytes a page actually received, so resources are re-requested with `cache: "force-cache"`. A server answering differently to a second request defeats that, and nothing here detects it. What it raises is the cost of an attack from "serve anything" to "serve one thing consistently and hope nobody compares". Host access is requested at a click and declared nowhere. An extension able to read every site from the moment it is installed is a worse thing than the problem it solves. The banner splits, which was worth doing on its own. It used to make two unrelated claims in one paragraph — that this code arrives from a server on every load, and that the project is unaudited — so the desktop build, where the first is false, silently dropped the second as well. Somebody who installed the signed binary was told nothing about the audit, which is the half that still applies to them. The delivery half now names what to check with; the audit half is untouched by any of this and shows everywhere. --- .github/workflows/test.yml | 10 ++ apps/web/src/app/WebClientWarning.tsx | 57 ++++++++- extension/background.js | 130 ++++++++++++++++++++ extension/manifest.json | 17 +++ extension/package.json | 5 + extension/popup.html | 20 +++ extension/popup.js | 65 ++++++++++ extension/verify.js | 122 +++++++++++++++++++ extension/verify.test.js | 167 ++++++++++++++++++++++++++ 9 files changed, 592 insertions(+), 1 deletion(-) create mode 100644 extension/background.js create mode 100644 extension/manifest.json create mode 100644 extension/package.json create mode 100644 extension/popup.html create mode 100644 extension/popup.js create mode 100644 extension/verify.js create mode 100644 extension/verify.test.js diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cc9812c8..f25ad462 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -284,6 +284,16 @@ jobs: working-directory: apps/web run: pnpm test + # The extension's comparison, which is the part of it that can be checked without a browser. + # + # In this job rather than one of its own: it needs node and nothing else, and a job that + # spends thirty seconds installing a runtime to run nine tests would cost more than it + # reports. It runs unconditionally for the same reason the rest of this job does — the + # extension is what makes the published manifest mean anything, and a silent regression in + # it would turn a green icon into a claim nobody checked. + - name: Extension + run: node --test extension/*.test.js + # The bundle, and it is not decoration. # # `typecheck`, `lint` and `test` all read the source; none of them resolve an import, diff --git a/apps/web/src/app/WebClientWarning.tsx b/apps/web/src/app/WebClientWarning.tsx index 80e946a1..d793d9fa 100644 --- a/apps/web/src/app/WebClientWarning.tsx +++ b/apps/web/src/app/WebClientWarning.tsx @@ -15,6 +15,17 @@ import { isTauri } from "@/lib/platform"; * past: the empty centre, which is the first thing seen on a cold start, and the top of the * settings screen, which is where someone goes when they are deciding how much to rely on this. * + * # Two claims, and only one of them is about delivery + * + * The banner used to make both in one paragraph: that this code arrives from a server on every + * load, and that the project is unaudited. They are unrelated, they have different remedies, and + * merging them meant the desktop build — where the first is false — silently dropped the second + * as well. Somebody who installed the signed binary was told nothing about the audit, which is + * the half that still applies to them. + * + * So there are two banners now. [`DeliveryWarning`] is about the web target and answerable; + * [`AuditWarning`] is about the project and is not. + * * # Why it is silent under Tauri * * The argument is specifically about **delivery**: a web server hands over this code on every @@ -29,12 +40,56 @@ import { isTauri } from "@/lib/platform"; * not in a banner. */ export function WebClientWarning({ className }: { className?: string }) { + return ( + <> + + + + ); +} + +/** + * The delivery problem, and what can now be done about it. + * + * The sentence changed when the answer became true rather than because it read better. Every + * release publishes a manifest of the bundle's hashes, attested by GitHub to the commit and the + * workflow that produced it — so the claim "this is the published build" is checkable by somebody + * other than the party making it. `scripts/verify-web.sh` does it by hand; the extension under + * `extension/` does it continuously. + * + * **What did not change is that this banner stays.** The verdict cannot live in this page: + * everything here is drawn by the server being checked, so a badge that went green on a + * "verified" signal would be forged by exactly the server it was meant to catch. The check + * belongs in the extension's own icon, and this paragraph can do no more than say so. + */ +function DeliveryWarning({ className }: { className?: string }) { if (isTauri()) return null; return ( The server delivers this code on every load, and could deliver a version that exfiltrates - your keys. No browser API fixes that. This is a learning project, unaudited — for genuinely + your keys. No browser API fixes that — but every release publishes the hashes of this + bundle, so what you were served can be compared against what the source produced. The + answer has to come from outside this page: see the repository’s{" "} + extension/ and scripts/verify-web.sh. + + ); +} + +/** + * The audit, which no amount of build verification touches. + * + * Shown on **every** target, desktop included. A reproducible signed binary establishes that the + * bytes match the source; it says nothing about whether the source is right, and this project's + * README is explicit that no external review has happened or will. Hiding that from the people + * who took the trouble to install the packaged build would be telling the least worried users the + * least. + */ +function AuditWarning({ className }: { className?: string }) { + return ( + + This is a learning project. Its cryptography has had no external review, and a protocol that + is correct on paper fails in practice on details only an audit finds. For genuinely sensitive conversations, use Signal. ); diff --git a/extension/background.js b/extension/background.js new file mode 100644 index 00000000..9e12c23e --- /dev/null +++ b/extension/background.js @@ -0,0 +1,130 @@ +/** + * The service worker: fetch the manifest, hash what the page loaded, colour the icon. + * + * # The verdict is here and never in the page + * + * That is the whole reason an extension exists rather than a banner. Everything a page displays is + * under the control of the server that served it, so a "verified" badge drawn by the application + * would be erased — or forged — by exactly the server it is supposed to catch. The toolbar icon is + * outside that server's reach, and it is the only place this answer can be believed. + * + * # Where the manifest comes from, and why it matters more than how it is compared + * + * From GitHub's API, never from the site being inspected. A server that hands over both the code + * and the list of hashes for that code has certified itself, which is the defect this whole + * mechanism exists to remove — the same one `docs/THREAT-MODEL.md` records about the transparency + * log being signed by the server it watches. + * + * # The compromise this cannot avoid, stated plainly + * + * The extension cannot read the bytes the page actually received. Chrome gives no API for it + * outside the debugger protocol. So it **re-requests** each resource with `cache: "force-cache"`, + * which normally returns the copy the page itself used. + * + * Normally. A server that answers differently to a second request defeats this, and nothing here + * detects that. It is the same compromise Code Verify makes, it is real, and it is why the banner + * in the application does not go away. What this raises is the cost of an attack from "serve + * anything" to "serve one thing consistently and hope nobody compares" — worth having, and not the + * same as impossible. + */ +import { parseManifest, verifyResources, verdict } from "./verify.js"; + +const REPOSITORY = "Sycatle/whispee"; + +/** The colours. Red is failure; grey is "not answered", which is not the same as pass. */ +const BADGE = { + ok: { text: "ok", colour: "#1a7f37" }, + failed: { text: "!", colour: "#cf222e" }, + unknown: { text: "?", colour: "#6e7781" }, +}; + +async function latestManifest() { + const release = await fetch(`https://api.github.com/repos/${REPOSITORY}/releases/latest`, { + headers: { accept: "application/vnd.github+json" }, + }); + if (!release.ok) throw new Error(`no published release (${release.status})`); + + const body = await release.json(); + const asset = body.assets?.find((candidate) => candidate.name === "WEB-SHA256SUMS"); + if (!asset) throw new Error("the latest release publishes no manifest"); + + const manifest = await fetch(asset.browser_download_url); + if (!manifest.ok) throw new Error(`manifest unreachable (${manifest.status})`); + + return { tag: body.tag_name, entries: parseManifest(await manifest.text()) }; +} + +/** + * What the page loaded, asked of the page itself. + * + * `performance.getEntriesByType("resource")` is the browser's own record of every request the + * document made, which is better than parsing the HTML for ` diff --git a/extension/popup.js b/extension/popup.js new file mode 100644 index 00000000..79ab23e2 --- /dev/null +++ b/extension/popup.js @@ -0,0 +1,65 @@ +/** + * The popup asks the service worker to check, and reports what came back. + * + * It holds no logic of its own on purpose: the comparison lives in `verify.js`, where it is + * testable, and the fetching lives in `background.js`, where the permissions are. A popup that + * duplicated either would be a second implementation to keep honest. + */ +const answer = document.querySelector("#answer"); +const detail = document.querySelector("#detail"); + +/** + * Host access is requested here, at a click, and never declared up front. + * + * An extension that could read every site from the moment it is installed is a worse thing than + * the problem it solves. The user points it at their own deployment, once, and Chrome remembers. + */ +async function permitted(origin) { + return await chrome.permissions.request({ origins: [`${origin}/*`] }); +} + +document.querySelector("#run").addEventListener("click", async () => { + const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); + if (!tab?.url) return; + + const origin = new URL(tab.url).origin; + + answer.className = "unknown"; + answer.textContent = "Checking…"; + detail.textContent = ""; + + if (!(await permitted(origin))) { + answer.textContent = "Not checked — access to this site was declined"; + return; + } + + const result = await chrome.runtime.sendMessage({ kind: "check", tabId: tab.id, origin }); + + answer.className = result.answer; + answer.textContent = + result.answer === "ok" + ? `Every file matches ${result.tag}` + : result.answer === "failed" + ? "This page is not the published build" + : `Could not check: ${result.error ?? "unknown"}`; + + const wrong = (result.findings ?? []).filter((finding) => finding.state !== "matched"); + if (wrong.length > 0) { + const list = document.createElement("ul"); + for (const finding of wrong.slice(0, 12)) { + const item = document.createElement("li"); + item.textContent = `${finding.state}: ${finding.path ?? finding.url}`; + list.append(item); + } + detail.append(list); + } + + if (result.answer === "ok") { + const note = document.createElement("p"); + // Said on success, where somebody is most likely to over-read the result. + note.textContent = + "This says the files match the manifest GitHub published. It does not say this server " + + "sends the same files to somebody else."; + detail.append(note); + } +}); diff --git a/extension/verify.js b/extension/verify.js new file mode 100644 index 00000000..1d21e2f4 --- /dev/null +++ b/extension/verify.js @@ -0,0 +1,122 @@ +/** + * The comparison itself, with nothing browser-specific in it. + * + * # Why this is a separate file + * + * So it can be tested. The rest of the extension is service-worker plumbing that only runs inside + * Chrome, and an extension whose only proof is "it looked right when I clicked it" is exactly the + * kind of verification this project spends its time refusing elsewhere. Everything here takes its + * inputs as arguments — no `chrome`, no `fetch` at module scope — so `verify.test.js` can drive it + * under `node --test`. + * + * # What it is comparing + * + * A manifest published by GitHub Actions, in `sha256sum` format, against the bytes a browser was + * served. Whichever way those bytes are obtained, this file does not care: it takes a function. + */ + +/** + * Parses a `sha256sum` manifest into a map of path to expected digest. + * + * The format is two spaces between digest and path, which is `sha256sum`'s own output and what + * `scripts/release-web.sh` emits. Lines that do not look like that are ignored rather than + * refused: a future manifest may carry a header, and refusing a whole verification over a line + * nobody reads would be the wrong failure. + */ +export function parseManifest(text) { + const entries = new Map(); + + for (const line of text.split("\n")) { + const match = /^([0-9a-f]{64})\s\s(.+)$/.exec(line.trim()); + if (match) entries.set(match[2], match[1]); + } + + return entries; +} + +/** Hex of the SHA-256 of some bytes, the same digest `sha256sum` prints. */ +export async function digest(bytes, subtle) { + const hash = await subtle.digest("SHA-256", bytes); + + return [...new Uint8Array(hash)].map((byte) => byte.toString(16).padStart(2, "0")).join(""); +} + +/** + * Turns a resource URL into the path a manifest would list it under. + * + * `null` when the resource does not belong to this origin — a font from a CDN, an analytics + * script, anything the deployment did not build. Those are **not** ignorable: a page is not + * verified if part of what it runs came from somewhere the manifest never described, so the + * caller reports them rather than skipping them. See `UNLISTED` below. + */ +export function pathOf(url, origin) { + if (!url.startsWith(`${origin}/`)) return null; + + const path = url.slice(origin.length + 1); + + // The query string is not part of what was hashed, and a cache-buster would otherwise make + // every resource unverifiable. + return path.split(/[?#]/)[0]; +} + +export const MATCHED = "matched"; +export const ALTERED = "altered"; +/** Served from this origin, at a path the manifest does not describe. */ +export const UNLISTED = "unlisted"; +/** Loaded from somewhere else entirely. */ +export const FOREIGN = "foreign"; + +/** + * Compares what a page loaded against what a manifest says it should be. + * + * `fetchBytes` is injected: in the extension it re-requests through the browser cache, in the test + * it hands back whatever the test decided. That indirection is not neutral and the caller must + * understand it — see `background.js` on why re-fetching is a compromise rather than a solution. + * + * **Fails closed.** Anything that is not a confirmed match counts against the verdict: a resource + * that could not be fetched, one at an unlisted path, one from another origin. A verifier that + * answers "fine" when it does not know is worse than no verifier, because somebody relies on it. + */ +export async function verifyResources({ urls, origin, manifest, fetchBytes, subtle }) { + const findings = []; + + for (const url of urls) { + const path = pathOf(url, origin); + + if (path === null) { + findings.push({ url, state: FOREIGN }); + continue; + } + + const expected = manifest.get(path); + if (expected === undefined) { + findings.push({ url, path, state: UNLISTED }); + continue; + } + + let actual; + try { + actual = await digest(await fetchBytes(url), subtle); + } catch { + // Unreachable is not innocent here: the page ran this resource, so something served it. + findings.push({ url, path, state: ALTERED }); + continue; + } + + findings.push({ url, path, state: actual === expected ? MATCHED : ALTERED, actual, expected }); + } + + return findings; +} + +/** + * The one-word answer, and it is only `ok` when every single resource matched. + * + * There is no partial pass. A page whose main bundle matches and whose one injected script does + * not is not "mostly verified" — it is a page running code nobody published. + */ +export function verdict(findings) { + if (findings.length === 0) return "unknown"; + + return findings.every((finding) => finding.state === MATCHED) ? "ok" : "failed"; +} diff --git a/extension/verify.test.js b/extension/verify.test.js new file mode 100644 index 00000000..21739d1f --- /dev/null +++ b/extension/verify.test.js @@ -0,0 +1,167 @@ +/** + * The comparison, driven without a browser. + * + * An extension whose only evidence is "the icon went green when I clicked it" verifies nothing — + * it is the same claim as an unaudited check, made by the thing doing the checking. What can be + * tested here is everything that decides an answer: the parsing, the path mapping, and above all + * that the failure cases fail. + * + * Run: `node --test extension/` + */ +import assert from "node:assert/strict"; +import { webcrypto } from "node:crypto"; +import { test } from "node:test"; + +import { + ALTERED, + FOREIGN, + MATCHED, + UNLISTED, + digest, + parseManifest, + pathOf, + verdict, + verifyResources, +} from "./verify.js"; + +const ORIGIN = "https://whispee.example"; +const bytes = (text) => new TextEncoder().encode(text); + +async function manifestFor(files) { + const lines = []; + for (const [path, content] of Object.entries(files)) { + lines.push(`${await digest(bytes(content), webcrypto.subtle)} ${path}`); + } + return parseManifest(lines.join("\n")); +} + +function served(files) { + return async (url) => { + const path = url.slice(ORIGIN.length + 1).split(/[?#]/)[0]; + if (!(path in files)) throw new Error("404"); + return bytes(files[path]); + }; +} + +test("a manifest in sha256sum format parses, and a header line does not break it", () => { + const entries = parseManifest( + ["# whispee", `${"a".repeat(64)} index.html`, `${"b".repeat(64)} assets/app.js`, ""].join( + "\n", + ), + ); + + assert.equal(entries.size, 2); + assert.equal(entries.get("index.html"), "a".repeat(64)); + assert.equal(entries.get("assets/app.js"), "b".repeat(64)); +}); + +test("a query string is not part of the path a manifest lists", () => { + assert.equal(pathOf(`${ORIGIN}/assets/app.js?v=2`, ORIGIN), "assets/app.js"); + assert.equal(pathOf(`${ORIGIN}/index.html#top`, ORIGIN), "index.html"); +}); + +test("a resource from another origin has no path here", () => { + assert.equal(pathOf("https://cdn.example/app.js", ORIGIN), null); + // The prefix check is on the origin plus a slash, so a lookalike host does not pass. + assert.equal(pathOf("https://whispee.example.evil/app.js", ORIGIN), null); +}); + +test("an untouched deployment matches every file", async () => { + const files = { "index.html": "", "assets/app.js": "console.log(1)" }; + + const findings = await verifyResources({ + urls: Object.keys(files).map((path) => `${ORIGIN}/${path}`), + origin: ORIGIN, + manifest: await manifestFor(files), + fetchBytes: served(files), + subtle: webcrypto.subtle, + }); + + assert.deepEqual( + findings.map((finding) => finding.state), + [MATCHED, MATCHED], + ); + assert.equal(verdict(findings), "ok"); +}); + +/** **The test the extension exists for.** One byte, and the answer has to change. */ +test("one altered byte fails the whole page", async () => { + const published = { "index.html": "", "assets/app.js": "console.log(1)" }; + const actuallyServed = { ...published, "assets/app.js": "console.log(1);evil()" }; + + const findings = await verifyResources({ + urls: Object.keys(published).map((path) => `${ORIGIN}/${path}`), + origin: ORIGIN, + manifest: await manifestFor(published), + fetchBytes: served(actuallyServed), + subtle: webcrypto.subtle, + }); + + assert.equal(findings.find((finding) => finding.path === "assets/app.js").state, ALTERED); + assert.equal(verdict(findings), "failed"); +}); + +/** + * The obvious way past a hash check: do not touch the listed files, add one. + * + * A verifier that walked the manifest and stopped there would report a clean page while an + * injected script ran beside it. This walks what the page **loaded**, so an extra file is a + * finding rather than an absence. + */ +test("a script the manifest never described is a failure, not a silence", async () => { + const published = { "index.html": "" }; + const actuallyServed = { ...published, "assets/injected.js": "exfiltrate()" }; + + const findings = await verifyResources({ + urls: [`${ORIGIN}/index.html`, `${ORIGIN}/assets/injected.js`], + origin: ORIGIN, + manifest: await manifestFor(published), + fetchBytes: served(actuallyServed), + subtle: webcrypto.subtle, + }); + + assert.equal(findings.find((finding) => finding.path === "assets/injected.js").state, UNLISTED); + assert.equal(verdict(findings), "failed"); +}); + +/** The other way round: keep the files, load the payload from somewhere else. */ +test("a resource pulled from another origin fails too", async () => { + const published = { "index.html": "" }; + + const findings = await verifyResources({ + urls: [`${ORIGIN}/index.html`, "https://cdn.evil/payload.js"], + origin: ORIGIN, + manifest: await manifestFor(published), + fetchBytes: served(published), + subtle: webcrypto.subtle, + }); + + assert.equal(findings.find((finding) => finding.url.includes("evil")).state, FOREIGN); + assert.equal(verdict(findings), "failed"); +}); + +/** Unreachable is not innocent: the page ran it, so something served it. */ +test("a resource that cannot be re-fetched counts against the verdict", async () => { + const published = { "index.html": "", "assets/app.js": "console.log(1)" }; + + const findings = await verifyResources({ + urls: Object.keys(published).map((path) => `${ORIGIN}/${path}`), + origin: ORIGIN, + manifest: await manifestFor(published), + fetchBytes: served({ "index.html": published["index.html"] }), + subtle: webcrypto.subtle, + }); + + assert.equal(findings.find((finding) => finding.path === "assets/app.js").state, ALTERED); + assert.equal(verdict(findings), "failed"); +}); + +/** + * Nothing checked is not the same as nothing wrong. + * + * A verdict of `ok` on an empty list would paint the icon green for a page the extension never + * looked at, which is the single most dangerous thing it could display. + */ +test("checking nothing answers unknown rather than ok", () => { + assert.equal(verdict([]), "unknown"); +}); From ed931c5a42f30ffa02884328768e4ca62fedd7c5 Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 17:14:29 +0200 Subject: [PATCH 10/13] fix(csp): derive both media origins however the variable is spelled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `connect-src` matched on `media.replace(/^http/, "ws")`, which assumes the deployment wrote `http://`. Given `ws://` — what `.env.example` recommends for a local media server, and how a LiveKit URL is written everywhere — the replacement matched nothing and returned its input. The policy then listed the same origin twice and **omitted the HTTP one**. That form is not decoration: the comment three lines above says the SDK asks the media server over HTTP why a connection failed, so a broken call reported a vaguer reason than the browser had. Exactly the class of omission the file exists to catch. `bothSchemes` normalises to HTTP first and returns the pair, so all four spellings — `ws`, `wss`, `http`, `https` — produce the same two sources. Found by configuring calls locally and reading the policy the dev server served: `connect-src 'self' ws://127.0.0.1:7880 ws://127.0.0.1:7880`. The existing test agreed with the bug because it only ever passed `https://`; the new one passes the other spelling and fails against the old code with "ws://127.0.0.1:7880 does not allow http://127.0.0.1:7880". --- apps/web/src/lib/csp.test.ts | 31 +++++++++++++++++++++++++++++++ apps/web/src/lib/csp.ts | 24 +++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/apps/web/src/lib/csp.test.ts b/apps/web/src/lib/csp.test.ts index fb0afdb2..d2db5e5e 100644 --- a/apps/web/src/lib/csp.test.ts +++ b/apps/web/src/lib/csp.test.ts @@ -186,6 +186,37 @@ test("the media origin appears only when a build asks for one", () => { ); }); +/** + * **The regression that made `bothSchemes` exist.** + * + * The pair was derived with `media.replace(/^http/, "ws")`, which only works on a variable spelled + * `http://`. `.env.example` recommends `ws://127.0.0.1:7880` for a local media server, and a + * LiveKit URL is written that way everywhere — given one, the replacement matched nothing, the + * policy listed the same origin twice, and the HTTP form the test above insists on was absent. + * + * The test above did not catch it because it only ever passed `https://`. This one passes the + * other spelling, which is the one a deployment is most likely to write. + */ +test("both forms are derived however the media origin is spelled", () => { + for (const [written, expected] of [ + ["ws://127.0.0.1:7880", ["http://127.0.0.1:7880", "ws://127.0.0.1:7880"]], + ["wss://media.example", ["https://media.example", "wss://media.example"]], + ["http://127.0.0.1:7880", ["http://127.0.0.1:7880", "ws://127.0.0.1:7880"]], + ["https://media.example", ["https://media.example", "wss://media.example"]], + ] as const) { + const sources = parse(csp(written)).get("connect-src") ?? new Set(); + + for (const origin of expected) { + assert.ok(sources.has(origin), `${written} does not allow ${origin}`); + } + + // A `Set` would hide a duplicate, so the raw directive is what is counted. + const directive = csp(written).split("; ").find((part) => part.startsWith("connect-src")) ?? ""; + const occurrences = directive.split(" ").filter((source) => source === written).length; + assert.equal(occurrences, 1, `${written} appears ${occurrences} times in connect-src`); + } +}); + test("media-src is not among the differences the desktop target is allowed", () => { assert.equal( DESKTOP_ONLY["media-src"], diff --git a/apps/web/src/lib/csp.ts b/apps/web/src/lib/csp.ts index 440f046d..d4eb4a7e 100644 --- a/apps/web/src/lib/csp.ts +++ b/apps/web/src/lib/csp.ts @@ -53,6 +53,20 @@ * No browser policy stands in the way — only the desktop app, whose code is packaged into the * installed binary, closes that path. */ +/** + * One origin, spelled both ways. + * + * A `connect-src` source matches on scheme, so `wss://host` and `https://host` are two sources + * and a policy needs whichever the code will actually use. Normalising to HTTP first makes the + * function total: it accepts `ws`, `wss`, `http` or `https` and returns the pair, rather than + * quietly returning its input for two of the four. + */ +function bothSchemes(origin: string): [string, string] { + const http = origin.replace(/^ws/, "http"); + + return [http, http.replace(/^http/, "ws")]; +} + export function csp(media?: string): string { // The media server is a second origin, and it is absent from most deployments: a build with no // media server must not widen its policy for a host it will never contact. Empty rather than a @@ -68,7 +82,15 @@ export function csp(media?: string): string { // // The audio itself travels over WebRTC, which no directive here can constrain — see // `lib/call.ts` for what does. - const relay = media ? ` ${media} ${media.replace(/^http/, "ws")}` : ""; + // + // **Both forms are derived, whichever one the deployment wrote.** This used to be + // `media.replace(/^http/, "ws")`, which assumed the variable was spelled `http://`. Given the + // `ws://` form — which is what `.env.example` recommends for a local media server, and what a + // LiveKit URL looks like everywhere — the replacement matched nothing and returned its input. + // The policy then listed the same origin twice and **omitted the HTTP one entirely**: the + // paragraph above says why that costs a broken call its explanation. The test only ever passed + // `https://`, so it agreed. + const relay = media ? ` ${bothSchemes(media).join(" ")}` : ""; return [ "default-src 'self'", From f720805b53451c54fe9be5ee6a2c7d8714f988f8 Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 17:15:32 +0200 Subject: [PATCH 11/13] docs(threat-model): what a checkable build establishes, and what it does not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §2.3 lists "serve hostile JavaScript, on every load, to one person" among a malicious server's powers, with no answer beside it but the desktop binary. There is one now, and it is worth less than it first reads — which is exactly why it needs a section rather than a line. §4quinquies says what was built: a manifest of the bundle's hashes published per release and attested by GitHub Actions through Sigstore, binding it to a commit and to the workflow. It says why that binding is the part that matters, and why the repository's own Ed25519 key would not have done — `verify-release.sh` already records that the key lives in the repository, so whoever controls the repository can replace it, and a manifest whose job is independence from the party serving the code cannot rest on that party's key. The rest of the section is what it does not establish, ordered by how easily each is over-read: - the banner does not go away and no version of this removes it, because a badge drawn by the page would be forged by the server being checked; - a targeted attack on somebody who does not check is untouched — the manual script passes for everybody who runs it and fails only for the person being attacked, who is the person not running it; - the extension re-requests rather than reads, so a server answering differently the second time defeats it; - verifiable or calls, not both, while `VITE_MEDIA_URL` still enters the policy; - the extension is its own supply chain, and unpublished; - none of it concerns the audit. Two existing entries were false after the change and are corrected. §2.3 gains the pointer. The limitations table said the web build pins the log key through `VITE_LOG_PUBKEY`; it does not any more, and the row now says why the removal was deliberate rather than a regression — the server shipped the pin alongside the code it constrained, and compiling it in was what stopped one manifest from describing every deployment. --- docs/THREAT-MODEL.md | 73 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/docs/THREAT-MODEL.md b/docs/THREAT-MODEL.md index 3d00fedd..d2496022 100644 --- a/docs/THREAT-MODEL.md +++ b/docs/THREAT-MODEL.md @@ -167,6 +167,9 @@ Same position, now actively lying, withholding, injecting and delaying. meantime. The server-side membership filter narrows the window without closing it. - **serve hostile JavaScript**, on the web target, on every load. No browser policy fixes that — which is what the desktop application, with its interface inside the signed binary, exists for. + What the web target has instead is a published, attested manifest of the bundle's hashes, which + makes a substitution **detectable by whoever checks** rather than impossible. See §4quinquies for + what that is worth and what it is not. - **choose whom to wake**, wherever push is configured. See §4. - **keep an envelope forever.** There is no purge, and no proof of deletion for anything. @@ -481,6 +484,74 @@ survive the loss of every device. Nothing archives it, so nothing restores it. --- +## 4quinquies. The served code is checkable, and the banner still stays + +§2.3 lists what a malicious server can do, and one entry has no cryptographic answer: **serve +hostile JavaScript, on every load, to one person**. No browser policy fixes it. The desktop +application exists partly for that reason — its interface is inside a signed binary — and the web +target had nothing. + +It now has something short of a defence and well short of nothing. + +### What was built + +Every release publishes `WEB-SHA256SUMS`, a hash per file of the bundle, produced by +`scripts/release-web.sh` and **attested by GitHub Actions** through Sigstore +(`.github/workflows/release.yml`). The attestation binds the manifest to a commit *and* to the +workflow that produced it: nobody, the maintainer included, can mint that binding outside Actions. + +That is the part that matters. The repository already carries an Ed25519 key for the desktop +release, and `scripts/verify-release.sh` states its own limit — the key lives in the repository, so +whoever controls the repository can replace it. For a manifest whose entire job is to be +independent of the party serving the code, a key that party holds is not independence. + +Two consumers: `scripts/verify-web.sh` compares a live deployment against a manifest, and +`extension/` does the same continuously from a browser. + +### The precondition, and why it is stated here + +**One manifest describes every deployment**, self-hosted included, because the bundle carries no +deployment's configuration any more. The API is reached on the page's own origin, the policy says +`connect-src 'self'`, and the log pin left the web build. Measured: two builds with entirely +different configuration are byte-identical across all 226 files, and a build in +`node:22-bookworm-slim` agrees with CI byte for byte. + +Without that, a published manifest would have described the official deployment and nothing else — +a service to one operator, dressed as a general mechanism. + +### What it does not establish, in order of how easily it is over-read + +**The banner does not go away, and no version of this work removes it.** Everything the page +displays is drawn by the server being checked. A "verified" badge in the application would be +forged by exactly the server it is meant to catch, which is why the verdict lives in the +extension's toolbar icon and nowhere else. What the sentence changes is its second half: from +"nobody can check this" to "here is what to check it with". + +**A targeted attack on somebody who does not check is untouched.** `verify-web.sh` passes for +everybody who runs it and fails only for the person being attacked — who is, by construction, the +person not running it. That is the ceiling of the manual half and the whole argument for the +extension. + +**The extension re-requests rather than reads.** Chrome exposes no way to obtain the bytes a page +actually received, so resources are fetched again with `cache: "force-cache"`. A server that +answers differently to a second request defeats this and nothing here detects it. What it raises is +the cost of an attack from "serve anything" to "serve one thing consistently and hope nobody +compares" — real, and not the same as impossible. + +**Verifiable or calls, not both.** `VITE_MEDIA_URL` still enters the Content-Security-Policy, so a +deployment configuring calls produces a bundle that no longer matches the published one. Until the +media server sits behind the same origin, an operator chooses between the two. + +**The extension is its own supply chain.** It is another artefact, from another store, and +"verified by an extension" is worth exactly what the extension is worth. It is unpublished today, +so installing it means loading it from source. + +**None of this concerns the audit.** A reproducible, attested, verified build establishes that the +bytes match the source. It says nothing about whether the source is right. That is why the banner +in the interface is now two banners, and why the second one shows on the desktop as well. + +--- + ## 5. Known limitations The full table, in order of real importance. Nothing here is softened. @@ -489,7 +560,7 @@ The full table, in order of real importance. Nothing here is softened. |---|---| | **Metadata** | Message sizes are now padded in buckets, and the sender is no longer identified to the server (sealed sender). Still visible: **who belongs to which group, when a post happens, and from which IP address**. Often more revealing than the content; hiding it would need a third-party relay and cover traffic. | | **Log signed by the party it watches** | The auditable log exists, but it is signed by the same party it monitors. A serious deployment would hand it to distinct operators. Gossip between clients partially compensates — it does not erase the defect. | -| **Log key served by the server** | The client discovers it from the very server it is meant to monitor. `VITE_LOG_PUBKEY` now lets a build be compiled against a known key, which is the only check that works on a first contact — but it closes the hole only in the **desktop binary**, packaged inside a signed artefact. On the web the server ships the bundle, so it ships the pin with it: there a pin turns a silent substitution into one that breaks every deployed client at once, which is worth having and is not a defence against the party that builds it. Unset, behaviour is as before, and the client still refuses to let the key change afterwards. | +| **Log key served by the server** | The client discovers it from the very server it is meant to monitor. The **desktop binary** pins it — packaged inside a signed artefact, that closes the hole on a first contact. The **web build no longer does**, and the removal was deliberate: there the server shipped the pin along with the code the pin constrained, so it was never a defence against the party building the bundle, and compiling it in made every deployment's bytes different — which is what stopped one published manifest from describing them all. What it did buy, a substitution that breaks every client at once instead of silently, is what §4quinquies provides and provides better. Either way the client still refuses to let the key change afterwards. | | **Account deletion** | There is no mechanism, and that is deliberate: an append-only log forbids removing an entry. Removing one outside the code shrinks the log, which gossip immediately reports as an attack — rightly. | | **Post noise** | The server holds each group's posting key: it can deposit envelopes. They will not decrypt — it cannot produce valid MLS — but it can pollute. That is the price of a symmetric MAC. | | **Typing-post rhythm** | The signal's content is opaque and never reaches the disk, but the server sees that a post is happening towards a given group. In a one-to-one it infers that one of the two is writing. Sealed sender hides *who*, not *that* — disabling the indicator is the only real protection. That setting is reciprocal, and was not always: it used to cut emission alone, which let an account watch its correspondents hesitate while showing them nothing. Privacy from the server, taken as an advantage over the person on the other side. | From 7787d70162a8e32f273db45448ec7abbb24e33b0 Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 17:34:13 +0200 Subject: [PATCH 12/13] fix(dev-env): pass the whole of .env through, not seven names of it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script sources `.env` and then printed seven `export` lines. Everything else it had just read died with the subshell, because both callers do `eval "$(scripts/dev-env.sh)"` — so a variable added to `.env` never reached the server. `README.md` said "the script loads .env, which the server does not do itself", and it loaded seven keys. # How it presents, which is the worst part Silently, and split in two. `MEDIA_URL` unset makes the call route answer 503, while `VITE_MEDIA_URL` — read by Vite from `apps/web/.env`, a different file — still shows the call button. The client offers a call the server refuses and neither side says why: finding it takes reading the network panel for a 503 on `/v1/groups/{id}/call/token`. `VAPID_SUBJECT` behaves the same way, which is why web push had to be started with the variable on the command line. `ACCOUNT_STORAGE_BYTES` silently reverts to its default. # What is emitted now Every name the file defines, minus the five this script computes. Re-emitting `DATABASE_URL` or `SERVER_ADDR` from the file would put every branch back on one database and one port, which is the thing this file exists to prevent. The names come from the file rather than from the environment: `set -a` exported `.env` into this process, but so is `PATH` and everything else a shell carries, and emitting the environment would hand the caller a copy of ours. # Checked by hand, since nothing here is unit-testable - `scripts/dev-server.sh --release` alone now starts a server whose `/proc` environment carries `MEDIA_URL`, `MEDIA_API_KEY`, `MEDIA_API_SECRET`, `RELAY_URLS` and `RELAY_SECRET`. It carried none before. - `DATABASE_URL` still resolves to the branch's own database, and `SERVER_ADDR` to its own port. - A value with an apostrophe survives the `eval` — that is what the escaping is for, and it is the case that would have broken it. - An empty value stays empty rather than failing under `set -u`. --- README.md | 7 +++++-- scripts/dev-env.sh | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2f3e39eb..0ac87c52 100644 --- a/README.md +++ b/README.md @@ -71,8 +71,11 @@ docker compose up -d # 2. Configuration. The committed defaults point at that container. cp .env.example .env -# 3. Server — listens on 127.0.0.1:8787. The script loads .env, which the -# server does not do itself, and gives the branch its own database and port. +# 3. Server — listens on 127.0.0.1:8787. The script passes .env through, which +# the server does not read itself, and gives the branch its own database and +# port. Everything the file defines reaches the server: the two values the +# script computes — the database and the address — are the only ones it +# overrides. ./scripts/dev-server.sh # 4. Client, in a second terminal. `wasm` builds crypto-core to WebAssembly diff --git a/scripts/dev-env.sh b/scripts/dev-env.sh index 1d7fa324..855b0b0f 100755 --- a/scripts/dev-env.sh +++ b/scripts/dev-env.sh @@ -114,3 +114,39 @@ printf "export ALLOWED_ORIGINS='http://127.0.0.1:%s,http://localhost:%s'\n" "$we printf "export WEB_PORT='%s'\n" "$web_port" printf "export WHISPEE_DEV_DATABASE='%s'\n" "$database" printf "export WHISPEE_DEV_BRANCH='%s'\n" "$branch" + +# And everything else the file defines, unchanged. +# +# # The bug this closes, which cost an afternoon +# +# This script sources `.env` and then printed seven names. Everything else it had just read died +# with the subshell, because the callers do `eval "$(scripts/dev-env.sh)"` — so a variable added +# to `.env` never reached the server. `README.md` said "the script loads .env, which the server +# does not do itself", and it loaded seven keys. +# +# The failure is silent and splits in two. `MEDIA_URL` unset makes the call route answer 503, +# while `VITE_MEDIA_URL` — read by Vite from `apps/web/.env`, a different file entirely — still +# shows the call button. So the client offers a call the server refuses, and neither side says +# why: it takes reading the network panel to find the 503. `VAPID_SUBJECT` behaves the same way, +# and `ACCOUNT_STORAGE_BYTES` silently reverts to its default. +# +# # Why the names come from the file rather than from the environment +# +# `set -a` exported `.env` into this process, but so is `PATH` and everything else a shell +# carries. Emitting the whole environment would hand the caller a copy of ours. Reading the keys +# back out of the file is what makes "what the file defines" the exact boundary. +# +# The five above are excluded because this script *computes* them: re-emitting `DATABASE_URL` or +# `SERVER_ADDR` from the file would put every branch back on one database and one port, which is +# the thing this file exists to prevent. +derived=" DATABASE_URL SERVER_ADDR WHISPEE_API ALLOWED_ORIGINS WEB_PORT " + +sed -n 's/^[[:space:]]*\([A-Za-z_][A-Za-z0-9_]*\)=.*/\1/p' "$env_file" | while read -r name; do + case "$derived" in *" $name "*) continue ;; esac + + # Indirect expansion, and `-` so that a key with no value is an empty string rather than an + # error under `set -u`. Single quotes inside the value are escaped the only way sh allows: + # close the quote, emit an escaped one, open again. + value=${!name-} + printf "export %s='%s'\n" "$name" "${value//\'/\'\\\'\'}" +done From 6a9ff372e7b1ab44538fa364b1d6c2a84e8337be Mon Sep 17 00:00:00 2001 From: sycatle Date: Tue, 25 Aug 2026 18:04:36 +0200 Subject: [PATCH 13/13] fix(dev-env): read the export-prefixed lines a .env may carry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `.env` is sourced, so `export FOO=bar` is as valid in it as `FOO=bar`, and a file copied from somewhere else very often carries the prefix. The pattern required `=` immediately after the name, so those lines were skipped — **silently**, which is the same failure #42 removed: a variable present in the file, absent from the server, and nothing anywhere saying so. Only the name has to be recognised. The value needs no unwrapping, because `set -a` and the `.` above already sourced the file: the shell treated `export FOO=bar` as the assignment it is, so `${!name-}` reads what the file set, prefix or not. Nothing else in this script has to know about the two spellings — worth saying in the comment, or the next reader wonders whether the prefix must be stripped somewhere. Named and not fixed: two assignments on one line, `export FOO=bar baz=qux`. Valid shell, and the pattern takes only the first. Nobody writes that in a `.env` and no file here does, but the silence has the same shape as the bug above, so it is written down rather than left to be found. # Checked by hand, in a fresh shell The test that means something is not that the name is recognised — it is that it survives the round trip with the right value. Sourced, emitted, re-evaluated in a separate `bash -c`, read back: - `export PREFIXED=with_export` → `with_export`, where `dev` gives nothing at all - `export SPACED_PREFIX=…` with several spaces → read - `export APOSTROPHE="it's exported"` → `it's exported`, the two difficulties at once, since an apostrophe is what breaks the emitted quoting - `export EMPTY_EXPORT=` → empty string, not unset - `PLAIN=bare` still read, and `DATABASE_URL` still derived per branch One trap worth recording, because it cost both of us an hour between us: a test `.env` must itself be valid shell. A bare apostrophe in it makes the sourcing fail silently and every value comes out empty, which reads exactly like a broken patch. --- scripts/dev-env.sh | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/scripts/dev-env.sh b/scripts/dev-env.sh index 855b0b0f..a62b5c49 100755 --- a/scripts/dev-env.sh +++ b/scripts/dev-env.sh @@ -141,7 +141,23 @@ printf "export WHISPEE_DEV_BRANCH='%s'\n" "$branch" # the thing this file exists to prevent. derived=" DATABASE_URL SERVER_ADDR WHISPEE_API ALLOWED_ORIGINS WEB_PORT " -sed -n 's/^[[:space:]]*\([A-Za-z_][A-Za-z0-9_]*\)=.*/\1/p' "$env_file" | while read -r name; do +# `export` is optional in the pattern, because it is optional in the file. +# +# A `.env` is sourced, so `export FOO=bar` is as valid in it as `FOO=bar` — and a file copied from +# somewhere else very often carries the prefix. Matching only the bare form skipped those lines +# **silently**, which is the exact failure this whole block was written to remove: a variable +# present in the file, absent from the server, and nothing anywhere saying so. +# +# Only the name has to be recognised. The value needs no unwrapping, because `set -a` and the `.` +# above already sourced the file: the shell treated `export FOO=bar` as the assignment it is, so +# `${!name-}` below reads what the file set, prefix or not. Nothing else in this script has to +# know about the two spellings. +# +# What this still does not read: two assignments on one line — `export FOO=bar baz=qux`. Valid +# shell, and the pattern would take only the first. Nobody writes that in a `.env` and no file in +# this repository does, but the silence is the same shape as the bug above, so it is named here +# rather than left to be discovered. +sed -n 's/^[[:space:]]*\(export[[:space:]]\{1,\}\)\{0,1\}\([A-Za-z_][A-Za-z0-9_]*\)=.*/\2/p' "$env_file" | while read -r name; do case "$derived" in *" $name "*) continue ;; esac # Indirect expansion, and `-` so that a key with no value is an empty string rather than an