From 913786c7c9a47628d9dad39a5006f9730a780489 Mon Sep 17 00:00:00 2001 From: "shakespeare.diy" Date: Tue, 8 Sep 2026 19:21:32 +0700 Subject: [PATCH] Citrine audit: verify Nostr handshake on probe + stop probe churn (1.3.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-audit of the 1.3.0–1.3.2 Citrine feature found two real issues: 1. False-positive detection (the important one). probeCitrine resolved true on bare socket OPEN, so ANY process squatting on 127.0.0.1:4869 would be treated as Citrine — and we'd then route reads AND published events at a non-Nostr peer. The probe now completes a minimal Nostr handshake: open → send a bounded REQ with a random sub id → require a protocol-shaped reply (EVENT/EOSE/CLOSED echoing the sub id, or a NOTICE) before declaring the relay live. Non-JSON / non-Nostr responses fall through to a clean timeout=false. A real relay answers with EOSE in ~ms on localhost, so the happy path is unchanged. 2. Probe churn. The reachability query re-opened a fresh ws:// probe every 5 min and on every window focus for the whole session — needless battery/wakelock, and a localhost poke on every Amber round-trip. Now it only polls (30s) and refetches-on-focus WHILE still waiting for Citrine to appear; once connected it stops the background churn (a per-session local relay won't vanish silently, and a failed read re-probes naturally). Reviewed and left as-is (deliberate, safe): writing every kind to Citrine incl. gift-wrapped DMs / Stash is intended on-device encrypted backup — all sensitive kinds are NIP-44 encrypted before reaching the publish layer, so nothing plaintext-sensitive is mirrored. Preview flag remains render-only. Co-authored-by: shakespeare.diy --- package.json | 2 +- src/hooks/useCitrine.ts | 14 ++++++--- src/lib/citrine.ts | 68 ++++++++++++++++++++++++++++++++--------- 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/package.json b/package.json index 8d056bd..978d89d 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "skate-spots", "private": true, - "version": "1.3.2", + "version": "1.3.3", "type": "module", "homepage": "https://skate-spots.com", "scripts": { diff --git a/src/hooks/useCitrine.ts b/src/hooks/useCitrine.ts index aa3ada4..9a802de 100644 --- a/src/hooks/useCitrine.ts +++ b/src/hooks/useCitrine.ts @@ -36,12 +36,16 @@ export function useCitrine() { // opted in — no point connecting to localhost otherwise. enabled: possible && enabled, queryFn: () => probeCitrine(CITRINE_RELAY_URL), - // Reachability changes rarely within a session (app installed/running or - // not); keep it warm but re-verify periodically so a freshly-launched - // Citrine is picked up without a manual refresh. staleTime: 60_000, - refetchInterval: 5 * 60_000, - refetchOnWindowFocus: true, + // Poll only while we're still WAITING for Citrine (not yet reachable), so a + // freshly-started relay is picked up automatically. Once connected, stop the + // background churn — a per-session localhost relay doesn't vanish silently, + // and any failed read will naturally re-probe. This avoids opening a fresh + // ws:// probe every few minutes for the whole session. + refetchInterval: (query) => (query.state.data ? false : 30_000), + // Re-verify on focus only until connected (e.g. user returns after starting + // Citrine). No need to re-poke localhost on every Amber round-trip once up. + refetchOnWindowFocus: (query) => !query.state.data, retry: false, }); diff --git a/src/lib/citrine.ts b/src/lib/citrine.ts index 0c54f7e..77d917d 100644 --- a/src/lib/citrine.ts +++ b/src/lib/citrine.ts @@ -88,25 +88,34 @@ export function isCitrinePossible(): boolean { } /** - * Probe whether a Citrine relay is actually running and reachable at `url` by - * opening a short-lived WebSocket. Resolves true only on a clean open; resolves - * false on error or timeout. Never rejects, never throws — safe to call from a - * query or effect. + * Probe whether a Citrine relay is actually running AND speaking Nostr at `url`. * - * We only probe on Android (isCitrinePossible) to avoid pointless localhost - * connection attempts (and console noise) in the browser/desktop. + * A bare "socket opened" check is not enough: any process squatting on the port + * would open cleanly and we'd then wrongly route reads — and, worse, published + * events — at a non-Nostr peer. So we complete a minimal Nostr handshake: + * open → send a tiny bounded REQ → require a protocol-shaped reply + * (EVENT / EOSE / NOTICE / CLOSED) referencing our subscription id. + * + * Resolves true only on a verified Nostr reply; false on error, timeout, or a + * non-Nostr response. Never rejects, never throws — safe from a query/effect. + * Short-circuits to false off-Android (and outside preview) to avoid pointless + * localhost attempts and console noise in the browser/desktop. */ export function probeCitrine( url: string = CITRINE_RELAY_URL, - timeoutMs = 2500, + timeoutMs = 3000, ): Promise { if (!isCitrinePossible()) return Promise.resolve(false); return new Promise((resolve) => { let settled = false; - const done = (result: boolean, ws?: WebSocket) => { + let ws: WebSocket | undefined; + let timer: ReturnType | undefined; + + const done = (result: boolean) => { if (settled) return; settled = true; + if (timer) clearTimeout(timer); try { ws?.close(); } catch { @@ -115,7 +124,6 @@ export function probeCitrine( resolve(result); }; - let ws: WebSocket; try { ws = new WebSocket(url); } catch { @@ -123,15 +131,45 @@ export function probeCitrine( return; } - const timer = setTimeout(() => done(false, ws), timeoutMs); + // Unique, throwaway subscription id we can match in the reply. + const subId = `citrine-probe-${Math.random().toString(36).slice(2, 10)}`; + + timer = setTimeout(() => done(false), timeoutMs); ws.onopen = () => { - clearTimeout(timer); - done(true, ws); + try { + // Ask for a single event of any kind — bounded and cheap. We don't care + // about the payload, only that the peer answers in the Nostr protocol. + ws!.send(JSON.stringify(['REQ', subId, { limit: 1 }])); + } catch { + done(false); + } }; - ws.onerror = () => { - clearTimeout(timer); - done(false, ws); + + ws.onmessage = (ev) => { + try { + const data = JSON.parse(typeof ev.data === 'string' ? ev.data : ''); + // A Nostr relay replies with a JSON array whose first element is a known + // verb. EVENT/EOSE/CLOSED echo our subId; NOTICE is a generic message. + if (Array.isArray(data) && typeof data[0] === 'string') { + const verb = data[0]; + const matchesSub = data[1] === subId; + if ( + ((verb === 'EVENT' || verb === 'EOSE' || verb === 'CLOSED') && matchesSub) || + verb === 'NOTICE' + ) { + done(true); + return; + } + } + // Any other shape: not a Nostr relay we recognise — keep waiting until + // timeout (a valid relay will still send EOSE). + } catch { + // Non-JSON frame → not a Nostr relay; wait for timeout. + } }; + + ws.onerror = () => done(false); + ws.onclose = () => done(false); }); }