diff --git a/docs/specs/pocket-app.md b/docs/specs/pocket-app.md index ac7e5f95..a12c4cb6 100644 --- a/docs/specs/pocket-app.md +++ b/docs/specs/pocket-app.md @@ -306,6 +306,36 @@ The existing static serving needs no special-casing: `serveStatic` already answers `application/manifest+json` for `.webmanifest` and `text/javascript` for `sw.js`. +### An expired session drops to sign-in + +Sessions live only in the Server's memory ([server.md](./server.md)), so they +end on their 12h expiry *and* on every Server restart, while the passkey and the +paired-host markers in `localStorage` outlive both. Recovery is therefore one +passkey prompt — but only if the user is offered it. + +Pocket treats a dead session as actionable rather than reportable: +`PocketClient` clears its in-memory token and throws `SessionExpiredError`, and +the app tears down any live adapter and returns to the sign-in screen carrying +that message. Signing back in restores the Hosts list with pairing and push +registration intact. Source of truth: `SessionExpiredError` in +`lib/src/remote/client/pocket-client.ts`, handled in `run` in +`lib/src/remote/pocket-app/App.tsx`. + +Two details this depends on: + +- **The trigger is the session gate specifically**, matched on the shared + `UNAUTHORIZED_ERROR` from `server-lib-common/src/remote/wire.ts` — a 401 alone + is ambiguous, since a wrong setup password and a rejected device signature + answer 401 too, and signing the user out for those would be worse than the bug + this fixes. +- **A rejected relay upgrade carries no status.** The browser surfaces it as a + bare `error` event, so `openSocket` asks an authenticated route what happened: + a 401 there means expiry, anything else leaves it an ordinary socket failure. + +Without this an installed Pocket is stuck — there is no address bar to reload +from, and the in-app Refresh re-sends the same dead token — leaving force-quitting +the app as the only way back in. + ## Deployment: same-origin, always WebAuthn binds passkeys to the serving origin, and Chrome's Private Network diff --git a/docs/specs/server.md b/docs/specs/server.md index 30ec7b27..3e8c2aad 100644 --- a/docs/specs/server.md +++ b/docs/specs/server.md @@ -35,7 +35,7 @@ UI lives in `lib`/`standalone`. | `DORMOUSE_STATE_DIR` | Where the JSON state files live. Default `./data`. | | `PORT` | Default 3000. | | `DORMOUSE_VAPID_PUBLIC_KEY` / `DORMOUSE_VAPID_PRIVATE_KEY` | Web Push signing keypair. Set both or neither. At startup the Server decodes both, derives the P-256 public point from the private key, and exits on a missing, malformed, or mismatched pair. Unset, the server mints a pair on first boot and persists it to `vapid.json`. | -| `DORMOUSE_VAPID_SUBJECT` | `mailto:`/`https:` contact for push-service operators (RFC 8292). Default `mailto:admin@localhost`. The Server parses and validates it at startup and exits on an invalid value. | +| `DORMOUSE_VAPID_SUBJECT` | `mailto:`/`https:` contact for push-service operators (RFC 8292). Defaults to `DORMOUSE_ORIGIN` when that origin is https and not loopback; otherwise there is no default and push stays off. The Server parses and validates it at startup and exits on an invalid value — including a loopback contact, which Apple rejects. | WebAuthn requires a secure context: `localhost` works for development; for a real phone, put the server behind TLS (`tailscale serve` is the intended @@ -157,6 +157,14 @@ so `node --test` can drive setup → pairing → connect end to end via The setup password is compared in constant time with a small fixed delay on failure; that is the extent of the hardening today. +Every session-gated route — including the `/ws/client` upgrade, which is +rejected before `injectWebSocket` ever sees it — answers an unknown or expired +token with 401 and the shared `UNAUTHORIZED_ERROR` from +`server-lib-common/src/remote/wire.ts`. That exact string is load-bearing: +Pocket keys its "sign in again" recovery on it, and a bare 401 is ambiguous, +since a wrong setup password and a rejected device signature answer 401 as well +([pocket-app.md](./pocket-app.md) -> An expired session drops to sign-in). + ### Web Push The relay routes between two live sockets; a push has to reach a phone whose app @@ -214,10 +222,29 @@ Source of truth: `server/src/push.ts` and the routes in `server/src/app.ts`. never silent: the refusal is logged (origin only — the endpoint is a bearer capability) and counted in the response's `failed`, since the route answers 200 either way and the Host needs to tell an all-failed fan-out from - success. TTL is 300s — an alarm that arrives an hour late is noise, not - information. -- Push is disabled, not half-working, when no VAPID key is configured: the - config route reports `null` and subscribe/send answer 503. + success. The log carries the push service's own reason body alongside the + status — whitespace-collapsed and capped at 200 characters so an HTML error + page cannot flood it — because a status alone does not separate a bad subject + from a bad key from a bad payload, and this is the only place that + explanation is ever visible. TTL is 300s — an alarm that arrives an hour late + is noise, not information. +- Push is disabled, not half-working, when no VAPID key **or no VAPID subject** + is configured: the config route reports `null` and subscribe/send answer 503. + The key and the subject are advertised together or not at all — a phone that + registered against a key the Server has no contact to sign with would be + subscribed to a push it can never receive. +- **A VAPID subject naming a loopback host is a startup error, not a default.** + Apple answers `403 {"reason":"BadJwtToken"}` for one — verified against + `web.push.apple.com` for `mailto:admin@localhost` and `https://localhost:3000`, + while `mailto:admin@example.com` and an ordinary https origin were accepted, so + the rule is loopback specifically and not reachability of the contact. + `web-push` only warns about the https form, at send time, and says nothing + about `mailto:` at `localhost`. This mattered: the previous default + (`mailto:admin@localhost`) let a Server boot clean, answer 200 on send, and + deliver nothing to any iPhone — the one platform the feature targets. Hence + the origin-derived default, and hence a loopback dev server turning push off + instead of guessing a placeholder contact. Source of truth: + `defaultVapidSubject` / `assertVapidSubject` in `server/src/push.ts`. ## Relay @@ -373,6 +400,18 @@ Builds the Pocket app (`lib/dist-pocket`) and the server, then serves both on `DORMOUSE_ORIGIN` to your TLS origin (e.g. via `tailscale serve`) — WebAuthn needs a secure context, and only `localhost` is exempt. +On the default localhost origin **push is off** and the server says so at +startup: there is no routable operator contact to sign a VAPID JWT with, and a +phone could not route to localhost anyway. Setting `DORMOUSE_ORIGIN` to an https +origin enables it with no further configuration, since that origin becomes the +subject. To exercise push against a desktop browser on localhost, supply a +contact explicitly: + +```sh +DORMOUSE_SETUP_PASSWORD=hunter2 DORMOUSE_VAPID_SUBJECT=mailto:you@example.com \ + pnpm dev:pocket-server +``` + **2. Host** (the laptop being controlled): `pnpm dev:standalone`, then enroll once from the devtools console of the standalone webview: @@ -394,6 +433,14 @@ First-time setup (password + label) creates the passkey and signs you in → Hosts → **Pair** → approve in the modal on the laptop → **Connect** (one biometric prompt) → pick a pane → type. +To test push, **add Pocket to the Home Screen before signing in** and do all of +the above inside the installed app: iOS delivers Web Push only there, and the +install is a separate storage partition needing its own pairing, so setting up +in the tab first means doing it twice ([pocket-app.md](./pocket-app.md) -> +Installable web app). Alerts are then a per-Host opt-in — **Enable alerts** on the Host's +row, which is the user gesture iOS requires before it will prompt for +permission. Connecting alone does not subscribe. + Limitations to know about: each browser storage partition has its own device key and therefore needs its own Host pairing, even when a synced passkey signs it in; clearing site data destroys that device key → re-pair, per the security diff --git a/lib/src/remote/client/pocket-client.test.ts b/lib/src/remote/client/pocket-client.test.ts index 8f103f24..cea4879f 100644 --- a/lib/src/remote/client/pocket-client.test.ts +++ b/lib/src/remote/client/pocket-client.test.ts @@ -23,6 +23,7 @@ import { hasRecoverablePairingFailure, PASSKEY_UNAVAILABLE_MESSAGE, PocketClient, + SessionExpiredError, type PocketSocket, type PocketStorage, type PocketClientDeps, @@ -160,6 +161,12 @@ class FakeSocket implements PocketSocket { this.#emit('open', {}); } + /** A rejected upgrade: the browser fires `error` with no status, never `open`. */ + fireError(): void { + this.readyState = 3; + this.#emit('error', {}); + } + /** Simulate the server sending a frame to this client. */ server(frame: unknown): void { this.#emit('message', { data: JSON.stringify(frame) }); @@ -541,6 +548,65 @@ async function connectEstablished(harness: Harness): Promise { await connecting; } +describe('session expiry', () => { + /** Signed in, with `/api/hosts` switchable between healthy and session-gate 401. */ + async function withHostsRoute( + response: () => { status?: number; json: unknown }, + ): Promise { + const harness = makeClient({ ...AUTH_ROUTES, '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/api/hosts': response }); + await harness.client.setup('pw', 'My Phone'); + await harness.client.signin(); + return harness; + } + + it('discards the token and reports expiry on the session gate 401', async () => { + let live = true; + const harness = await withHostsRoute(() => + live ? { json: { hosts: [] } } : { status: 401, json: { error: 'unauthorized' } }, + ); + expect(harness.client.sessionToken).toBe('tok-abc'); + + live = false; + await expect(harness.client.listHosts()).rejects.toBeInstanceOf(SessionExpiredError); + // Keeping it would leave the UI believing it is still signed in. + expect(harness.client.sessionToken).toBeNull(); + }); + + // A wrong setup password and a rejected device signature also answer 401; + // treating those as expiry would sign the user out mid-action. + it('leaves a 401 that is not the session gate as an ordinary failure', async () => { + const harness = await withHostsRoute(() => ({ + status: 401, + json: { error: 'device signature rejected' }, + })); + + await expect(harness.client.listHosts()).rejects.toThrow('device signature rejected'); + expect(harness.client.sessionToken).toBe('tok-abc'); + }); + + it('turns a rejected relay upgrade into expiry when the session is the reason', async () => { + let live = true; + const harness = await withHostsRoute(() => + live ? { json: { hosts: [] } } : { status: 401, json: { error: 'unauthorized' } }, + ); + + live = false; + const opening = harness.client.openSocket(); + harness.socket.fireError(); + await expect(opening).rejects.toBeInstanceOf(SessionExpiredError); + expect(harness.client.sessionToken).toBeNull(); + }); + + it('keeps a socket failure a socket failure while the session is alive', async () => { + const harness = await withHostsRoute(() => ({ json: { hosts: [] } })); + + const opening = harness.client.openSocket(); + harness.socket.fireError(); + await expect(opening).rejects.toThrow('relay socket error'); + expect(harness.client.sessionToken).toBe('tok-abc'); + }); +}); + describe('socket lifecycle', () => { it('an unexpected close fires host-gone for an established session and resets the socket', async () => { const harness = await signedIn(); diff --git a/lib/src/remote/client/pocket-client.ts b/lib/src/remote/client/pocket-client.ts index 616eeac7..f5f35a3f 100644 --- a/lib/src/remote/client/pocket-client.ts +++ b/lib/src/remote/client/pocket-client.ts @@ -21,6 +21,7 @@ import { REMOTE_EVENTS, REMOTE_METHODS, SELFHOST_ACCOUNT_ID, + UNAUTHORIZED_ERROR, WS_ROUTES, WS_TOKEN_PARAM, hashPasskeyPublicKey, @@ -108,6 +109,27 @@ export interface PairResult { readonly error?: string; } +/** Shown when the Server no longer accepts our session token. */ +export const SESSION_EXPIRED_MESSAGE = 'Your session expired. Sign in again to continue.'; + +/** + * The Server rejected our session token, so nothing works until the user signs + * in again. Distinct from an ordinary failure because the UI must react rather + * than report: sessions live only in the Server's memory (docs/specs/server.md), + * so they die on a 12h expiry *and* on every Server restart, and an installed + * Pocket has no address bar to reload from. Left as a message, the user is + * stuck holding a dead token with force-quitting the app as the only way out. + * + * {@link PocketClient} clears the token before throwing this, so recovery is + * exactly "sign in again" with the passkey and paired-host markers intact. + */ +export class SessionExpiredError extends Error { + constructor() { + super(SESSION_EXPIRED_MESSAGE); + this.name = 'SessionExpiredError'; + } +} + interface Waiter { resolve(frame: ServerToClientFrame): void; reject(error: Error): void; @@ -305,7 +327,17 @@ export class PocketClient { } /** Open the `/ws/client` relay socket; resolves once it is open. */ - openSocket(): Promise { + async openSocket(): Promise { + try { + await this.#openSocket(); + } catch (err) { + const error = err instanceof Error ? err : new Error(String(err)); + if (error instanceof SessionExpiredError) throw error; + await this.#diagnoseSocketFailure(error); + } + } + + #openSocket(): Promise { const token = this.#requireToken(); const url = `${this.#wsBase}${WS_ROUTES.client}?${WS_TOKEN_PARAM}=${encodeURIComponent(token)}`; const ws = this.#createWebSocket(url); @@ -643,10 +675,42 @@ export class PocketClient { ...(method === 'GET' ? {} : { body: JSON.stringify(body ?? {}) }), }); const parsed = (await response.json().catch(() => ({}))) as T & { error?: string }; + // Only the session gate's 401 means "sign in again" — a wrong setup + // password and a rejected device signature answer 401 too, and bouncing the + // user to sign-in for those would be a worse bug than the one this fixes. + if (response.status === 401 && parsed.error === UNAUTHORIZED_ERROR) { + // Drop the token here rather than at the call site: every later request + // and every relay upgrade would fail the same way, and keeping it would + // let the UI believe it is still signed in. + this.#sessionToken = null; + throw new SessionExpiredError(); + } if (!response.ok) throw new Error(parsed.error ?? `request failed (${response.status})`); return parsed; } + /** + * Turn a relay-socket failure into a {@link SessionExpiredError} when the + * session is the reason. A rejected WS upgrade reaches the browser as a bare + * `error` event with no status, so the only way to tell "session died" from + * "network is down" is to ask an authenticated route — which answers the + * question and costs one request on a path that has already failed. + */ + async #diagnoseSocketFailure(original: Error): Promise { + if (this.#sessionToken === null) throw original; + try { + await this.#api(API_ROUTES.hosts, undefined, { + method: 'GET', + headers: { authorization: `Bearer ${this.#sessionToken}` }, + }); + } catch (err) { + if (err instanceof SessionExpiredError) throw err; + // Probe failed for its own reason — report the socket failure, which is + // what the user actually hit. + } + throw original; + } + #requireToken(): string { if (!this.#sessionToken) throw new Error('sign in first'); return this.#sessionToken; diff --git a/lib/src/remote/pocket-app/App.tsx b/lib/src/remote/pocket-app/App.tsx index f622b2f3..2dd1bb74 100644 --- a/lib/src/remote/pocket-app/App.tsx +++ b/lib/src/remote/pocket-app/App.tsx @@ -18,6 +18,7 @@ import { clsx } from 'clsx'; import { tv } from 'tailwind-variants'; import { PocketClient, + SessionExpiredError, type ConnectDecision, type PocketSocket, } from '../client/pocket-client'; @@ -230,18 +231,41 @@ export default function App(): React.ReactElement { if (!client.socketOpen) await client.openSocket(); }, [client]); - const run = useCallback(async (label: string, fn: () => Promise) => { - setError(null); - setBusy(label); - try { - await fn(); - } catch (err) { - setError(err instanceof Error ? err.message : String(err)); - } finally { - setBusy(null); - } + /** Tear down the live session and return to the hosts list. */ + const teardownAdapter = useCallback(() => { + void adapterRef.current?.dispose(); + adapterRef.current = null; + disposeAllSessions(); }, []); + const run = useCallback( + async (label: string, fn: () => Promise) => { + setError(null); + setBusy(label); + try { + await fn(); + } catch (err) { + // A dead session is not reportable, it is actionable: the token is + // already discarded, so every view above sign-in would fail the same + // way, and an installed Pocket has no reload affordance to escape with. + // Drop to sign-in, where one passkey prompt restores everything — + // pairing and push registration both outlive the session. + if (err instanceof SessionExpiredError) { + teardownAdapter(); + client.close(); + setActiveHost(null); + setPhase('auth'); + setError(err.message); + return; + } + setError(err instanceof Error ? err.message : String(err)); + } finally { + setBusy(null); + } + }, + [client, teardownAdapter], + ); + const loadHosts = useCallback(async () => { await ensureSocket(); const list = await client.listHosts(); @@ -250,13 +274,6 @@ export default function App(): React.ReactElement { setPhase('hosts'); }, [client, ensureSocket]); - /** Tear down the live session and return to the hosts list. */ - const teardownAdapter = useCallback(() => { - void adapterRef.current?.dispose(); - adapterRef.current = null; - disposeAllSessions(); - }, []); - // Socket drop / host-gone: dispose the adapter and fall back to Hosts. useEffect(() => { client.setOnHostGone(() => { diff --git a/server-lib-common/src/remote/wire.ts b/server-lib-common/src/remote/wire.ts index 22e85eef..7b8258bc 100644 --- a/server-lib-common/src/remote/wire.ts +++ b/server-lib-common/src/remote/wire.ts @@ -30,6 +30,15 @@ export const API_ROUTES = { pushSend: '/api/push/send', } as const; +/** + * The `error` a session-gated route answers 401 with when the session token is + * unknown or expired. Shared because Pocket keys recovery on it: a 401 alone is + * ambiguous (a wrong setup password and a rejected device signature also answer + * 401), and only this one means "sign in again". Changing the string on one + * side without the other would silently strand users on a dead session. + */ +export const UNAUTHORIZED_ERROR = 'unauthorized'; + export const WS_ROUTES = { host: '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/ws/host', client: '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/ws/client', diff --git a/server/src/app.ts b/server/src/app.ts index 845b1f60..f1997946 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -32,6 +32,7 @@ import { HELLO_ROUTE, HostChallengeIssuer, SELFHOST_ACCOUNT_ID, + UNAUTHORIZED_ERROR, WS_ROUTES, WS_TOKEN_PARAM, fromBase64Url, @@ -398,7 +399,7 @@ export function createApp(config: AppConfig): CreatedApp { const requireSession: MiddlewareHandler = async (c, next) => { const token = bearerToken(c); const session = token ? sessions.validate(token) : null; - if (!session) return c.json({ error: 'unauthorized' }, 401); + if (!session) return c.json({ error: UNAUTHORIZED_ERROR }, 401); c.set('session', session); await next(); }; @@ -665,7 +666,7 @@ export function createApp(config: AppConfig): CreatedApp { (c, next) => { const token = c.req.query(WS_TOKEN_PARAM); const session = token ? sessions.validate(token) : null; - if (!session) return c.json({ error: 'unauthorized' }, 401); + if (!session) return c.json({ error: UNAUTHORIZED_ERROR }, 401); return next(); }, upgradeWebSocket((c) => { diff --git a/server/src/index.ts b/server/src/index.ts index 06efa138..35e4ea0c 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -11,10 +11,10 @@ import { serve } from '@hono/node-server'; import { createApp } from './app.js'; import { - DEFAULT_VAPID_SUBJECT, assertVapidKeyPair, assertVapidSubject, createWebPushSender, + defaultVapidSubject, generateVapidKeys, } from './push.js'; import { VapidStore } from './state.js'; @@ -55,22 +55,41 @@ const vapid = envVapidPublic && envVapidPrivate ? { publicKey: envVapidPublic, privateKey: envVapidPrivate } : await new VapidStore(stateDir).loadOrCreate(generateVapidKeys); -const vapidSubject = process.env.DORMOUSE_VAPID_SUBJECT ?? DEFAULT_VAPID_SUBJECT; +// The JWT is signed with an operator contact, so no subject means no push at +// all — `web-push` cannot construct a send without one. An unset +// DORMOUSE_VAPID_SUBJECT therefore falls back to this server's own origin, +// which is unusable only for a loopback dev server. There push is switched off +// rather than left half-working: a phone cannot route to localhost anyway, and +// booting with a subject a push service rejects is what made every iPhone +// delivery fail silently before. +const vapidSubject = process.env.DORMOUSE_VAPID_SUBJECT ?? defaultVapidSubject(origin); try { assertVapidKeyPair(vapid); - assertVapidSubject(vapidSubject); + if (vapidSubject !== null) assertVapidSubject(vapidSubject); } catch (err) { console.error(`Invalid VAPID configuration: ${err instanceof Error ? err.message : String(err)}`); process.exit(1); } +if (vapidSubject === null) { + console.warn( + `push is disabled: no VAPID subject. DORMOUSE_ORIGIN (${origin}) cannot serve as one — ` + + 'set DORMOUSE_VAPID_SUBJECT to a routable mailto: or https: contact to enable it.', + ); +} const { app, injectWebSocket } = createApp({ setupPassword, origin, stateDir, pocketDir, - vapidPublicKey: vapid.publicKey, - pushSender: createWebPushSender(vapid, vapidSubject), + // Both together or neither: advertising a key the server has no subject to + // sign with would let a phone register against a push it can never receive. + ...(vapidSubject === null + ? {} + : { + vapidPublicKey: vapid.publicKey, + pushSender: createWebPushSender(vapid, vapidSubject), + }), }); const server = serve({ fetch: app.fetch, port }, (info) => { diff --git a/server/src/push.ts b/server/src/push.ts index 205430d4..74011f74 100644 --- a/server/src/push.ts +++ b/server/src/push.ts @@ -51,11 +51,58 @@ export interface VapidKeys { } /** - * `mailto:` or `https:` contact for the push service operator, per RFC 8292. - * Push services may use it to reach whoever is responsible for a misbehaving - * sender; some reject a JWT without one. + * Hosts a push service will not accept in a VAPID subject. Apple answers + * `403 {"reason":"BadJwtToken"}` for a loopback subject — verified against + * `web.push.apple.com` for both `mailto:admin@localhost` and + * `https://localhost:3000`, while `mailto:admin@example.com` and an ordinary + * https origin were accepted. Apple does not check that the contact is + * *reachable*, only that it is not loopback. */ -export const DEFAULT_VAPID_SUBJECT = 'mailto:admin@localhost'; +const LOOPBACK_SUBJECT_HOSTS = new Set(['localhost', '127.0.0.1', '::1', '[::1]']); + +/** The host a subject names: the domain half for `mailto:`, the hostname otherwise. */ +function subjectHost(subject: URL): string { + if (subject.protocol === 'mailto:') { + const at = subject.pathname.lastIndexOf('@'); + return at === -1 ? '' : subject.pathname.slice(at + 1).toLowerCase(); + } + return subject.hostname.toLowerCase(); +} + +function isLoopbackSubjectHost(host: string): boolean { + if (!host) return false; + if (LOOPBACK_SUBJECT_HOSTS.has(host)) return true; + // RFC 6761 reserves the whole `.localhost` TLD for loopback. + if (host.endsWith('.localhost')) return true; + return /^127\./.test(host); +} + +/** + * The `mailto:`/`https:` operator contact to sign VAPID JWTs with (RFC 8292) + * when `DORMOUSE_VAPID_SUBJECT` is unset, or `null` when this deployment has no + * usable one and push must stay off. + * + * The server's own origin is the right zero-config answer: it is a real contact + * for whoever runs this server, and every deployment that can serve Pocket at + * all already has a valid https origin, because WebAuthn requires one. A + * loopback dev server has no such contact — and could not reach a phone anyway, + * since the phone cannot route to it. Returning `null` there disables push + * rather than inventing a placeholder contact that a push service may reject, + * which is the failure this default exists to prevent: the previous default + * (`mailto:admin@localhost`) let the server boot clean, answer 200 on send, and + * silently deliver nothing to any iPhone. + */ +export function defaultVapidSubject(origin: string): string | null { + let parsed: URL; + try { + parsed = new URL(origin); + } catch { + return null; + } + if (parsed.protocol !== 'https:') return null; + if (isLoopbackSubjectHost(parsed.hostname.toLowerCase())) return null; + return parsed.origin; +} /** Generate a VAPID keypair in the exact encoding the sender expects. */ export function generateVapidKeys(): VapidKeys { @@ -90,8 +137,14 @@ export function assertVapidKeyPair(keys: VapidKeys): void { /** * Validate the operator contact before the first delivery. `web-push` performs - * this check while constructing a send, which would otherwise let a malformed - * environment value survive startup and fail every notification at runtime. + * the syntax half of this check while constructing a send, which would + * otherwise let a malformed environment value survive startup and fail every + * notification at runtime. + * + * The loopback rule is stricter than `web-push`'s: it warns about a loopback + * *https* subject on every send and says nothing at all about `mailto:` at + * `localhost`, and a warning buried in send-time stderr is exactly how a + * deployment ends up delivering nothing to iPhones without noticing. */ export function assertVapidSubject(subject: string): void { let parsed: URL; @@ -103,6 +156,13 @@ export function assertVapidSubject(subject: string): void { if (parsed.protocol !== 'mailto:' && parsed.protocol !== 'https:') { throw new Error('VAPID subject must be a valid mailto: or https: URL.'); } + if (isLoopbackSubjectHost(subjectHost(parsed))) { + throw new Error( + 'VAPID subject must not name a loopback host — Apple rejects such a JWT with ' + + 'BadJwtToken, so every push to an iPhone would fail. Use a routable contact, ' + + "e.g. this server's https origin or a real mailto: address.", + ); + } } function decodeVapidKey(value: string, name: 'public' | 'private', length: number): Buffer { @@ -131,6 +191,25 @@ function endpointOrigin(endpoint: string): string { } } +/** + * The push service's own explanation of a rejection, e.g. Apple's + * `{"reason":"BadJwtToken"}`. Worth logging because the status code alone does + * not distinguish a bad VAPID subject from a bad key from a bad payload, and + * this is the only place that explanation is ever visible. Whitespace-collapsed + * and capped so an HTML error page cannot flood the log. + */ +const MAX_LOGGED_ERROR_BODY = 200; + +function pushErrorDetail(err: unknown): string { + const body = (err as { body?: unknown }).body; + if (typeof body !== 'string') return ''; + const collapsed = body.replace(/\s+/g, ' ').trim(); + if (!collapsed) return ''; + return collapsed.length > MAX_LOGGED_ERROR_BODY + ? `${collapsed.slice(0, MAX_LOGGED_ERROR_BODY)}…` + : collapsed; +} + export function createWebPushSender(keys: VapidKeys, subject: string): PushSender { const agent = createPublicPushAgent(); return { @@ -166,6 +245,7 @@ export function createWebPushSender(keys: VapidKeys, subject: string): PushSende console.warn( `push delivery failed for ${endpointOrigin(target.endpoint)}:`, status ?? (err instanceof Error ? err.message : String(err)), + pushErrorDetail(err), ); return 'failed'; } diff --git a/server/test/push.test.mjs b/server/test/push.test.mjs index c1173854..52a9192a 100644 --- a/server/test/push.test.mjs +++ b/server/test/push.test.mjs @@ -16,7 +16,12 @@ import { join } from 'node:path'; import { API_ROUTES, signPushSubscribe } from 'server-lib-common'; import { SimClient } from '../../server-lib-common/test/harness/actors.mjs'; -import { assertVapidKeyPair, assertVapidSubject, generateVapidKeys } from '../dist/push.js'; +import { + assertVapidKeyPair, + assertVapidSubject, + defaultVapidSubject, + generateVapidKeys, +} from '../dist/push.js'; import { ORIGIN, enrollHost, fakePushSender, freshApp, ownerSession, post } from './helpers.mjs'; const VAPID_PUBLIC = 'BJxKIjEEuJH0dLHTAcMFVYRnLsIBWcuMt5S1FCdDLbxCkmpUuLfHTFzWSFCPFTFsFvT8sVFTFxKIjEE'; @@ -41,6 +46,45 @@ test('VAPID subject validation accepts contact URLs and rejects invalid values', } }); +// Apple answers 403 BadJwtToken for a loopback subject, so accepting one would +// boot a server that reports success and delivers nothing to any iPhone. +test('VAPID subject validation rejects loopback contacts', () => { + for (const subject of [ + 'mailto:admin@localhost', + 'mailto:admin@dev.localhost', + 'mailto:admin@127.0.0.1', + 'https://localhost:3000', + 'https://127.0.0.1:3000', + 'https://[::1]:3000', + ]) { + assert.throws(() => assertVapidSubject(subject), /loopback host/, subject); + } +}); + +test('default VAPID subject is the https origin, and absent for one push cannot use', () => { + assert.equal( + defaultVapidSubject('https://dormouse.example.com'), + 'https://dormouse.example.com', + ); + // Only the origin — a path or trailing slash is not part of the contact. + assert.equal( + defaultVapidSubject('https://dormouse.example.com/pocket/'), + 'https://dormouse.example.com', + ); + + // No usable contact → push off rather than a placeholder a service rejects. + for (const origin of [ + 'http://localhost:3000', + 'https://localhost:3000', + 'https://127.0.0.1:3000', + 'https://[::1]:3000', + 'http://dormouse.example.com', + 'not a url', + ]) { + assert.equal(defaultVapidSubject(origin), null, origin); + } +}); + function subscription(endpoint = 'https://push.example.com/sub/abc') { return { endpoint, keys: { p256dh: 'BFakeP256dhKey', auth: 'FakeAuthSecret' } }; }