diff --git a/js/nostr/client.js b/js/nostr/client.js index ec0473e0..05071b75 100644 --- a/js/nostr/client.js +++ b/js/nostr/client.js @@ -3337,17 +3337,20 @@ export class NostrClient { // We collect events here instead of processing them instantly to avoid // 1000s of React re-renders during the initial relay dump. // The buffer acts as a pressure valve between the network and the UI. - const buffer = new VideoEventBuffer(this, onVideo); + // `videoEventVerifier` lets tests virtualize the off-thread signature worker; + // undefined in production so the buffer uses its default worker verifier. + const buffer = new VideoEventBuffer(this, onVideo, { + verifyEvents: this.videoEventVerifier, + }); // 1) On each incoming event, just push to the buffer and schedule a flush sub.on("event", (event) => { buffer.push(event); }); - // You can still use sub.on("eose") if needed - sub.on("eose", () => { - buffer.handleEose(); - }); + // You can still use sub.on("eose") if needed. Return the flush promise so + // callers/tests can await the (now async, off-thread-verified) commit. + sub.on("eose", () => buffer.handleEose()); // Return the subscription object if you need to unsub manually later const originalUnsub = diff --git a/js/nostr/managers/ConnectionManager.js b/js/nostr/managers/ConnectionManager.js index 35fff540..c96dbc6d 100644 --- a/js/nostr/managers/ConnectionManager.js +++ b/js/nostr/managers/ConnectionManager.js @@ -141,7 +141,13 @@ export class ConnectionManager { } const creation = Promise.resolve().then(() => { - const instance = new SimplePool(); + // PERF (pending security decision — see PR discussion): the default + // SimplePool verifies every incoming event's schnorr signature on the main + // thread, which profiling showed costs ~1.2s of blocking CPU during feed + // load / tab switches (it dropped to ~16ms when skipped). Skipping trusts + // relays not to forge authorship. Alternatives: verify in a Web Worker, or + // only skip for the user's own trustedRelayURLs. + const instance = new SimplePool({ verifyEvent: () => true }); if (typeof instance.ensureRelay === "function") { const originalEnsureRelay = instance.ensureRelay.bind(instance); diff --git a/js/nostr/signatureVerifyWorker.js b/js/nostr/signatureVerifyWorker.js new file mode 100644 index 00000000..8680cad4 --- /dev/null +++ b/js/nostr/signatureVerifyWorker.js @@ -0,0 +1,56 @@ +// Web Worker: verifies Nostr event signatures off the main thread. +// +// The relay SimplePool is constructed with `verifyEvent: () => true` so it never +// blocks the main thread verifying incoming events (profiling showed ~1.2s of +// schnorr work during feed load). Instead, batches of events are verified here, +// in parallel with the UI, and the caller commits only the ids reported valid. +// +// Message protocol: +// in: { id, events: Array } +// out: { id, ok: true, validIds: string[] } | { id, ok: false, error } + +import { ensureNostrTools, getCachedNostrTools } from "./toolkit.js"; + +async function resolveTools() { + const cached = getCachedNostrTools(); + if (cached && typeof cached.verifyEvent === "function") { + return cached; + } + return (await ensureNostrTools()) || cached; +} + +function verifyOne(tools, event) { + try { + if (!event || typeof event !== "object") return false; + if (typeof tools.validateEvent === "function" && !tools.validateEvent(event)) { + return false; + } + return tools.verifyEvent(event) === true; + } catch (_) { + return false; + } +} + +self.addEventListener("message", async (msg) => { + const data = msg?.data || {}; + const { id, events } = data; + try { + const tools = await resolveTools(); + if (!tools || typeof tools.verifyEvent !== "function") { + throw new Error("signature-verification-unavailable"); + } + const validIds = []; + for (const event of Array.isArray(events) ? events : []) { + if (event && event.id && verifyOne(tools, event)) { + validIds.push(event.id); + } + } + self.postMessage({ id, ok: true, validIds }); + } catch (error) { + self.postMessage({ + id, + ok: false, + error: { name: error?.name || "Error", message: error?.message || "verify-worker-error" }, + }); + } +}); diff --git a/js/nostr/signatureVerifyWorkerClient.js b/js/nostr/signatureVerifyWorkerClient.js new file mode 100644 index 00000000..472f3ae9 --- /dev/null +++ b/js/nostr/signatureVerifyWorkerClient.js @@ -0,0 +1,118 @@ +// Client for signatureVerifyWorker. Verifies event batches off the main thread +// and resolves with the Set of valid event ids. Falls back to main-thread +// verification only when Web Workers are unavailable, so signatures are still +// checked (never silently trusted) in that degraded case. + +import { devLogger } from "../utils/logger.js"; +import { ensureNostrTools, getCachedNostrTools } from "./toolkit.js"; + +const DEFAULT_TIMEOUT_MS = 10000; + +let workerInstance = null; +let workerReady = false; +let requestId = 0; +const pending = new Map(); + +function attachWorkerListeners(worker) { + if (!worker || workerReady) return; + + worker.addEventListener("message", (event) => { + const data = event?.data || {}; + const entry = pending.get(data.id); + if (!entry) return; + pending.delete(data.id); + clearTimeout(entry.timeoutId); + if (data.ok && Array.isArray(data.validIds)) { + entry.resolve(new Set(data.validIds)); + } else { + entry.reject(new Error(data?.error?.message || "verify-worker-error")); + } + }); + + worker.addEventListener("error", (error) => { + devLogger.warn("[signatureVerifyWorkerClient] Worker error", error); + for (const entry of pending.values()) { + clearTimeout(entry.timeoutId); + entry.reject(error instanceof Error ? error : new Error("verify-worker-error")); + } + pending.clear(); + }); + + workerReady = true; +} + +function ensureWorker() { + if (workerInstance) return workerInstance; + if (typeof Worker === "undefined") return null; + try { + workerInstance = new Worker(new URL("./signatureVerifyWorker.js", import.meta.url), { + type: "module", + }); + attachWorkerListeners(workerInstance); + } catch (error) { + devLogger.warn("[signatureVerifyWorkerClient] Failed to create worker", error); + workerInstance = null; + } + return workerInstance; +} + +// Main-thread fallback used only when Workers are unavailable. Preserves the +// security guarantee (still verifies) at the cost of main-thread time. +async function verifyOnMainThread(events) { + const tools = getCachedNostrTools() || (await ensureNostrTools()); + const valid = new Set(); + if (!tools || typeof tools.verifyEvent !== "function") { + // Cannot verify at all — fail closed by returning no valid ids. + return valid; + } + for (const event of events) { + try { + if (!event || !event.id) continue; + if (typeof tools.validateEvent === "function" && !tools.validateEvent(event)) continue; + if (tools.verifyEvent(event) === true) valid.add(event.id); + } catch (_) { + // skip + } + } + return valid; +} + +/** + * Verify a batch of events. Resolves with a Set of valid event ids. + * @param {Array} events + * @param {{ timeoutMs?: number }} [options] + * @returns {Promise>} + */ +export function verifyEventsInWorker(events, { timeoutMs = DEFAULT_TIMEOUT_MS } = {}) { + const list = Array.isArray(events) ? events.filter((e) => e && e.id) : []; + if (!list.length) return Promise.resolve(new Set()); + + const worker = ensureWorker(); + if (!worker) { + return verifyOnMainThread(list); + } + + const id = ++requestId; + return new Promise((resolve, reject) => { + const timeoutId = setTimeout(() => { + pending.delete(id); + reject(new Error("verify-worker-timeout")); + }, timeoutMs); + pending.set(id, { resolve, reject, timeoutId }); + try { + worker.postMessage({ id, events: list }); + } catch (error) { + clearTimeout(timeoutId); + pending.delete(id); + reject(error instanceof Error ? error : new Error("verify-worker-post-failed")); + } + }).catch(async (error) => { + // On worker failure/timeout, fall back to main-thread verification rather + // than dropping events or trusting them blindly. + devLogger.warn("[signatureVerifyWorkerClient] Falling back to main-thread verify:", error?.message || error); + return verifyOnMainThread(list); + }); +} + +// Test/diagnostic helpers. +export const __signatureVerifyInternals = { verifyOnMainThread }; diff --git a/js/nostr/videoEventBuffer.js b/js/nostr/videoEventBuffer.js index 0af08a73..b0c2f9af 100644 --- a/js/nostr/videoEventBuffer.js +++ b/js/nostr/videoEventBuffer.js @@ -8,19 +8,29 @@ import { isDevMode } from "../config.js"; import { devLogger, userLogger } from "../utils/logger.js"; import { convertEventToVideo } from "./nip71.js"; +import { verifyEventsInWorker } from "./signatureVerifyWorkerClient.js"; export class VideoEventBuffer { /** * @param {import("./client.js").NostrClient} client - The parent client instance (for state access). * @param {function(object[]): void} onVideo - The UI callback to fire with batched updates. + * @param {{ verifyEvents?: (events: object[]) => Promise> }} [options] + * Injectable signature verifier (returns the set of valid event ids). + * Defaults to the off-main-thread worker; tests can pass a passthrough. */ - constructor(client, onVideo) { + constructor(client, onVideo, options = {}) { this.client = client; this.onVideo = onVideo; this.buffer = []; this.invalidEvents = []; this.flushTimerId = null; this.FLUSH_DEBOUNCE_MS = 75; + this._verifyEvents = + typeof options.verifyEvents === "function" + ? options.verifyEvents + : verifyEventsInWorker; + // Tracks the in-flight verify+commit so callers/tests can await a flush. + this._flushPromise = Promise.resolve(); // Buffer updates while hidden to avoid UI thrashing this.pendingVideos = []; @@ -48,21 +58,21 @@ export class VideoEventBuffer { scheduleFlush(immediate = false) { if (this.flushTimerId) { if (!immediate) { - return; + return this._flushPromise; } clearTimeout(this.flushTimerId); this.flushTimerId = null; } if (immediate) { - this.flush(); - return; + return this.flush(); } this.flushTimerId = setTimeout(() => { this.flushTimerId = null; this.flush(); }, this.FLUSH_DEBOUNCE_MS); + return this._flushPromise; } /** @@ -83,6 +93,40 @@ export class VideoEventBuffer { const toProcess = this.buffer; this.buffer = []; + // The relay pool no longer verifies signatures on the main thread (that cost + // ~1.2s during feed load). Verify this batch off-thread and commit only the + // events that pass; fail closed (drop the batch) on total verification + // failure rather than rendering unverified content. Returns the in-flight + // promise so callers/tests can await commit completion. + this._flushPromise = this._verifyAndCommit(toProcess); + return this._flushPromise; + } + + async _verifyAndCommit(toProcess) { + let verified; + try { + const validIds = await this._verifyEvents(toProcess); + verified = toProcess.filter((evt) => evt && evt.id && validIds.has(evt.id)); + const dropped = toProcess.length - verified.length; + if (dropped > 0) { + devLogger.warn( + `[VideoEventBuffer] Dropped ${dropped} event(s) with invalid/unverifiable signatures.`, + ); + } + } catch (error) { + devLogger.error( + "[VideoEventBuffer] Signature verification failed; dropping batch:", + error, + ); + return; + } + if (!verified.length) { + return; + } + this._commitVerifiedBatch(verified); + } + + _commitVerifiedBatch(toProcess) { const updatedVideos = []; for (const evt of toProcess) { @@ -198,7 +242,7 @@ export class VideoEventBuffer { devLogger.log( "[subscribeVideos] Reached EOSE for all relays (historical load done)" ); - this.scheduleFlush(true); + return this.scheduleFlush(true); } /** @@ -214,6 +258,6 @@ export class VideoEventBuffer { document.removeEventListener("visibilitychange", this.handleVisibilityChange); } // Ensure any straggling events are flushed before tearing down. - this.flush(); + return this.flush(); } } diff --git a/tests/nostr-delete-flow.test.mjs b/tests/nostr-delete-flow.test.mjs index d625b90e..90b50021 100644 --- a/tests/nostr-delete-flow.test.mjs +++ b/tests/nostr-delete-flow.test.mjs @@ -6,6 +6,7 @@ import { NostrService } from "../js/services/nostrService.js"; await (async function testDeleteFlowPublishesDeletionFlag() { localStorage.clear(); const client = new NostrClient(); + client.videoEventVerifier = async (events) => new Set(events.map((e) => e.id)); client.hydrateVideoHistory = async () => {}; const pubkey = "0000000000000000000000000000000000000000000000000000000000000001"; @@ -219,7 +220,7 @@ await (async function testDeleteFlowPublishesDeletionFlag() { handlers.event(deleteEvent); if (typeof handlers.eose === "function") { - handlers.eose(); + await handlers.eose(); } assert.equal( @@ -246,6 +247,7 @@ await (async function testDeleteFlowPublishesDeletionFlag() { await (async function testTombstonePersistenceAcrossSaveRestore() { localStorage.clear(); const client = new NostrClient(); + client.videoEventVerifier = async (events) => new Set(events.map((e) => e.id)); const videoRootId = "root-persist"; const activeKey = `ROOT:${videoRootId}`; @@ -272,6 +274,7 @@ await (async function testTombstonePersistenceAcrossSaveRestore() { await client.saveLocalData("test", { immediate: true }); const restored = new NostrClient(); + restored.videoEventVerifier = async (events) => new Set(events.map((e) => e.id)); await restored.restoreLocalData(); assert.equal( @@ -292,6 +295,7 @@ await (async function testTombstonePersistenceAcrossSaveRestore() { await (async function testSubscribeVideosSkipsOlderThanTombstone() { localStorage.clear(); const client = new NostrClient(); + client.videoEventVerifier = async (events) => new Set(events.map((e) => e.id)); client.populateNip71MetadataForVideos = async () => {}; const seenUpdates = []; @@ -342,7 +346,7 @@ await (async function testSubscribeVideosSkipsOlderThanTombstone() { handlers.event(deleteEvent); if (typeof handlers.eose === "function") { - handlers.eose(); + await handlers.eose(); } const activeKey = `ROOT:${videoRootId}`; @@ -377,7 +381,7 @@ await (async function testSubscribeVideosSkipsOlderThanTombstone() { handlers.event(olderEvent); if (typeof handlers.eose === "function") { - handlers.eose(); + await handlers.eose(); } assert.equal( diff --git a/tests/nostr/client.test.mjs b/tests/nostr/client.test.mjs index e8787cbc..3c83b0d6 100644 --- a/tests/nostr/client.test.mjs +++ b/tests/nostr/client.test.mjs @@ -33,6 +33,10 @@ describe("NostrClient", () => { client.pool = mockPool; client.isInitialized = true; // Skip init network calls client.relays = ["wss://relay.example.com"]; + // Virtualize the off-thread signature verifier: synthetic test events are + // unsigned, so treat all as valid (the real verifier would correctly drop + // them). Production keeps the default worker verifier. + client.videoEventVerifier = async (events) => new Set(events.map((e) => e.id)); }); afterEach(() => { diff --git a/tests/nostr/signatureVerifyWorkerClient.test.mjs b/tests/nostr/signatureVerifyWorkerClient.test.mjs new file mode 100644 index 00000000..aef4dac1 --- /dev/null +++ b/tests/nostr/signatureVerifyWorkerClient.test.mjs @@ -0,0 +1,84 @@ +// Tests the signature-verify worker client protocol: a batch of events is sent +// to the worker and the client resolves with the Set of ids the worker reports +// valid. This is the contract the feed ingestion relies on to drop forged events +// after the pool stopped verifying on the main thread. + +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; + +// A controllable fake Worker installed as the global before importing nothing +// stateful — the client lazily constructs the worker on first use. +class FakeWorker { + constructor() { + this.listeners = { message: [], error: [] }; + FakeWorker.instances.push(this); + } + addEventListener(type, fn) { + (this.listeners[type] ||= []).push(fn); + } + postMessage(msg) { + // Respond asynchronously, echoing as valid only the ids in FakeWorker.validIds + // (or all ids if validIds is null), to mimic real verification. + setTimeout(() => { + const ids = (msg.events || []).map((e) => e.id); + const validIds = FakeWorker.validIds === null ? ids : ids.filter((id) => FakeWorker.validIds.has(id)); + for (const fn of this.listeners.message) fn({ data: { id: msg.id, ok: true, validIds } }); + }, 0); + } + terminate() {} +} +FakeWorker.instances = []; +FakeWorker.validIds = null; + +describe("signatureVerifyWorkerClient", () => { + let verifyEventsInWorker; + + beforeEach(async () => { + globalThis.Worker = FakeWorker; + FakeWorker.instances = []; + FakeWorker.validIds = null; + // Fresh module each test so the singleton worker is rebuilt against FakeWorker. + const mod = await import(`../../js/nostr/signatureVerifyWorkerClient.js?bust=${Math.random()}`); + verifyEventsInWorker = mod.verifyEventsInWorker; + }); + + afterEach(() => { + delete globalThis.Worker; + }); + + it("resolves with the set of ids the worker reports valid", async () => { + const events = [ + { id: "a".repeat(64), sig: "x" }, + { id: "b".repeat(64), sig: "y" }, + { id: "c".repeat(64), sig: "z" }, + ]; + const valid = await verifyEventsInWorker(events); + assert.ok(valid instanceof Set); + assert.equal(valid.size, 3); + assert.ok(valid.has("a".repeat(64))); + }); + + it("drops events the worker reports invalid", async () => { + FakeWorker.validIds = new Set(["a".repeat(64)]); // only 'a' is valid + const events = [ + { id: "a".repeat(64) }, + { id: "b".repeat(64) }, + ]; + const valid = await verifyEventsInWorker(events); + assert.equal(valid.size, 1); + assert.ok(valid.has("a".repeat(64))); + assert.ok(!valid.has("b".repeat(64))); + }); + + it("returns an empty set for empty input without invoking a worker", async () => { + const valid = await verifyEventsInWorker([]); + assert.equal(valid.size, 0); + assert.equal(FakeWorker.instances.length, 0, "must not construct a worker for empty input"); + }); + + it("ignores events without ids", async () => { + const valid = await verifyEventsInWorker([{ sig: "no-id" }, null, { id: "d".repeat(64) }]); + assert.equal(valid.size, 1); + assert.ok(valid.has("d".repeat(64))); + }); +}); diff --git a/tests/nostr/videoEventBuffer.test.mjs b/tests/nostr/videoEventBuffer.test.mjs index 638ee820..cbb0c2ce 100644 --- a/tests/nostr/videoEventBuffer.test.mjs +++ b/tests/nostr/videoEventBuffer.test.mjs @@ -76,19 +76,27 @@ const deletedEvent = { }) }; +// Virtualize the off-thread signature verifier: treat every seeded test event as +// valid (the seeds are synthetic and unsigned). This isolates the buffer's +// batching/commit logic from real schnorr verification while preserving the +// "only valid ids are committed" contract. +const passthroughVerify = async (events) => new Set(events.map((e) => e.id)); +const newBuffer = (client, onVideo = () => {}) => + new VideoEventBuffer(client, onVideo, { verifyEvents: passthroughVerify }); + test('VideoEventBuffer', async (t) => { - await t.test('Push and Flush', () => { + await t.test('Push and Flush', async () => { const client = new MockClient(); let onVideoCalls = []; const onVideo = (videos) => onVideoCalls.push(videos); - const buffer = new VideoEventBuffer(client, onVideo); + const buffer = newBuffer(client, onVideo); buffer.push(validEvent); assert.equal(buffer.buffer.length, 1); assert.notEqual(buffer.flushTimerId, null); - // Force flush - buffer.scheduleFlush(true); + // Force flush (now async: verification happens off-thread before commit) + await buffer.scheduleFlush(true); assert.equal(buffer.buffer.length, 0); assert.equal(client.allEvents.size, 1); assert.equal(client.activeMap.size, 1); @@ -99,30 +107,30 @@ test('VideoEventBuffer', async (t) => { assert.equal(client.activeMap.get("root1").id, validEvent.id); }); - await t.test('Latest Wins', () => { + await t.test('Latest Wins', async () => { const client = new MockClient(); let onVideoCalls = []; const onVideo = (videos) => onVideoCalls.push(videos); - const buffer = new VideoEventBuffer(client, onVideo); + const buffer = newBuffer(client, onVideo); // Setup: activeMap has older event client.activeMap.set("root1", { ...validEvent, created_at: 1000, videoRootId: "root1" }); buffer.push(newerEvent); - buffer.scheduleFlush(true); // Force flush + await buffer.scheduleFlush(true); // Force flush // Should update because newerEvent.created_at (2000) > 1000 assert.equal(client.activeMap.get("root1").id, newerEvent.id); assert.equal(client.activeMap.get("root1").created_at, 2000); }); - await t.test('Tombstone Handling', () => { + await t.test('Tombstone Handling', async () => { const client = new MockClient(); - const buffer = new VideoEventBuffer(client, () => {}); + const buffer = newBuffer(client); // Process deleted event buffer.push(deletedEvent); - buffer.scheduleFlush(true); + await buffer.scheduleFlush(true); // Should verify tombstone recorded assert.ok(client.tombstones.has("root1")); @@ -133,26 +141,26 @@ test('VideoEventBuffer', async (t) => { // Now try pushing an older event buffer.push(newerEvent); // created_at 2000 < 3000 - buffer.scheduleFlush(true); + await buffer.scheduleFlush(true); // Should be guarded by tombstone assert.ok(!client.activeMap.has("root1")); }); - await t.test('Cleanup', () => { + await t.test('Cleanup', async () => { const client = new MockClient(); - const buffer = new VideoEventBuffer(client, () => {}); + const buffer = newBuffer(client); buffer.push(validEvent); assert.notEqual(buffer.flushTimerId, null); - buffer.cleanup(); + await buffer.cleanup(); assert.equal(buffer.flushTimerId, null); assert.equal(buffer.buffer.length, 0); // Should have flushed assert.equal(client.allEvents.size, 1); }); - await t.test('Visibility Gating', () => { + await t.test('Visibility Gating', async () => { // Mock document const originalDocument = global.document; let visibilityHandler = null; @@ -168,11 +176,11 @@ test('VideoEventBuffer', async (t) => { const client = new MockClient(); let onVideoCalls = []; const onVideo = (videos) => onVideoCalls.push(videos); - const buffer = new VideoEventBuffer(client, onVideo); + const buffer = newBuffer(client, onVideo); // Push event while hidden buffer.push(validEvent); - buffer.scheduleFlush(true); + await buffer.scheduleFlush(true); // Should NOT have called onVideo yet assert.equal(onVideoCalls.length, 0);