Audit round 3: live-sub budgets, revocation-before-courtesy, pending-key catch, CI semgrep, input bounds

BL-25 (medium): live subscriptions get the same adversarial-relay
posture buffered queries got in round 2 -- a bounded already-seen id
cache (2048 ids, FIFO eviction) so replaying one retained valid event
costs one verification and ONE delivery ever, plus a fixed-window
verification budget (100 per 10s) that quarantines distinct-event spray
for the rest of the window. The audit's 2,000-replay probe now produces
one callback, bounded by test.

BL-26 (medium): teardown order inverted. retireNip46Client pre-signs
the courtesy logout event while the key exists, then close() runs
IMMEDIATELY -- rejecting every in-flight request and wiping keys -- and
only then is the pre-signed event delivered, asynchronously and bounded
(publishCourtesy, safe after close). The audit's probe -- a pending
get_public_key resolving after logout began -- is now impossible and
regression-tested: in-flight requests reject within the same tick of
close().

BL-27 (low): a nostrconnect listen that fails naturally (timeout,
unreachable relays) discards the pending ephemeral key in a guarded
catch -- ownership-transfer on success unchanged.

BL-28 (low): the hosted verify job installs the pinned Semgrep 1.171.0
before security:ci, which round 2 taught to require it; the job was red
at the previous HEAD and skipping its SBOM/checksum artifacts.

Informational: bunker URI input bounds -- 4096 chars total, 256-char
secret (rejected beyond), 512-char relay URLs (filtered beyond).

8 new/updated adversarial tests; 266 total, all green.

Co-Authored-By: Claude Fable 5 <[email protected]>

.github/workflows/security.yml

@@ -22,7 +22,9 @@ jobs:
           node-version: 22
           cache: npm
       - run: npm ci --ignore-scripts
-      - name: Run build, typecheck, tests, dependency audit, and registry signature verification
+      - name: Install pinned Semgrep (security:ci runs the custom rules)
+        run: pipx install semgrep==1.171.0
+      - name: Run build, typecheck, tests, dependency audit, and static-analysis gate
         run: npm run security:ci
       - name: Generate checksums and CycloneDX SBOM
         shell: bash

docs/TODO.md

@@ -41,6 +41,22 @@ explicitly not a security boundary) is now sent on teardown; this corrects
 the earlier "no stable method name" note. Still deferred, interop only:
 `switch_relays`.
 
+Third-pass response (same day): BL-25 — live subscriptions now carry a
+bounded already-seen id cache (2048, FIFO eviction; a replayed retained
+event costs one verification and one delivery, ever) and a fixed-window
+verification budget (100 per 10s; distinct-event spray is quarantined for
+the window). BL-26 — teardown order inverted: the courtesy logout event is
+pre-signed, then close() IMMEDIATELY rejects every in-flight request and
+wipes keys, then the pre-signed event is delivered asynchronously and
+bounded — nothing can resolve after logout begins (regression-tested).
+BL-27 — a nostrconnect listen failing naturally (timeout, unreachable
+relays) now discards the pending ephemeral key in a guarded catch instead
+of orphaning it. BL-28 — the hosted verify job installs the pinned Semgrep
+before running security:ci (it was red at the previous HEAD, skipping SBOM
+and checksums). Informational — bunker URIs are bounded: 4096 chars total,
+256-char secret, 512-char relay URLs (oversized relays filtered, oversized
+URI/secret rejected).
+
 Owner action items:
 
 1. **Live-fire the NIP-46 QR with Amber** on bitlogin.network (flow was

packages/core/src/nostr/nip46.test.ts

@@ -36,12 +36,26 @@ describe("parseBunkerUri", () => {
   });
 
   it("caps and deduplicates relays from a crafted URI (audit BL-21)", () => {
-    const relays = Array.from({ length: 5000 }, (_, i) => `relay=wss://r${i % 20}.example`).join("&");
+    const relays = Array.from({ length: 120 }, (_, i) => `relay=wss://r${i % 20}.example`).join("&");
     const parsed = parseBunkerUri(`bunker://${SIGNER_HEX}?${relays}`);
     expect(parsed.relayUrls.length).toBe(8);
     expect(new Set(parsed.relayUrls).size).toBe(8);
   });
 
+  it("bounds the URI, secret, and per-relay lengths (audit round 3)", () => {
+    expect(() => parseBunkerUri(`bunker://${SIGNER_HEX}?relay=wss://r.example&x=${"a".repeat(5000)}`)).toThrow(
+      /unreasonably long/
+    );
+    expect(() =>
+      parseBunkerUri(`bunker://${SIGNER_HEX}?relay=wss://r.example&secret=${"s".repeat(300)}`)
+    ).toThrow(/secret is unreasonably long/);
+    // An oversized relay URL is filtered out rather than opening a socket.
+    const parsed = parseBunkerUri(
+      `bunker://${SIGNER_HEX}?relay=wss://${"r".repeat(600)}.example&relay=wss://ok.example`
+    );
+    expect(parsed.relayUrls).toEqual(["wss://ok.example"]);
+  });
+
   it("rejects wrong schemes, bad pubkeys, and relay-less URIs", () => {
     expect(() => parseBunkerUri(`nostrconnect://${SIGNER_HEX}?relay=wss://x.example`)).toThrow(
       /bunker:\/\//
@@ -323,6 +337,22 @@ describe("Nip46Client against a fake bunker", () => {
     ).rejects.toThrow(/different event/);
   });
 
+  it("close() rejects in-flight requests immediately -- nothing resolves after logout begins (audit BL-26)", async () => {
+    bunker = new FakeBunker(relay.url);
+    bunker.handle = () => []; // a signer that never answers
+    await bunker.start();
+    const c = makeClient(undefined, 60_000);
+    const pending = c.getUserPublicKey();
+    pending.catch(() => {}); // observed below; avoid unhandled-rejection noise
+    await new Promise((resolve) => setTimeout(resolve, 100)); // let the request go out
+    const farewell = c.buildLogoutEvent();
+    const start = Date.now();
+    c.close();
+    await expect(pending).rejects.toThrow(/connection was closed/);
+    expect(Date.now() - start).toBeLessThan(1_000);
+    expect(farewell).not.toBeNull();
+  });
+
   it("times out with a typed error when the signer never answers", async () => {
     bunker = new FakeBunker(relay.url);
     bunker.handle = () => []; // listens, never replies
@@ -441,8 +471,11 @@ describe("courtesy logout", () => {
       requestTimeoutMs: 5000
     });
     await client.connect();
-    await client.logout();
+    // Local revocation first (BL-26): pre-sign, close immediately, deliver after.
+    const farewell = client.buildLogoutEvent();
     client.close();
+    expect(farewell).not.toBeNull();
+    await client.publishCourtesy(farewell!);
     // FakeBunker records every decrypted request it receives; its processing
     // is asynchronous relative to the publish OK, so poll briefly.
     const deadline = Date.now() + 2000;

packages/core/src/nostr/nip46.ts

@@ -36,6 +36,13 @@ const HEX64 = /^[0-9a-f]{64}$/u;
 /** Ceiling on relays taken from a bunker:// URI (BL-21). */
 const MAX_BUNKER_RELAYS = 8;
 
+/** Input bounds on the bunker URI as a whole and its parts. Legitimate URIs
+ *  are a few hundred characters; these stop memory amplification from
+ *  crafted multi-megabyte inputs before any further processing. */
+const MAX_BUNKER_URI_CHARS = 4096;
+const MAX_BUNKER_SECRET_CHARS = 256;
+const MAX_BUNKER_RELAY_URL_CHARS = 512;
+
 export interface BunkerPointer {
   signerPubkey: string;
   relayUrls: string[];
@@ -81,6 +88,9 @@ export function sanitizeAuthUrl(candidate: string): string | null {
 }
 
 export function parseBunkerUri(uri: string): BunkerPointer {
+  if (uri.length > MAX_BUNKER_URI_CHARS) {
+    throw new BunkerUriParseError("That bunker URI is unreasonably long.");
+  }
   let url: URL;
   try {
     url = new URL(uri.trim());
@@ -104,7 +114,7 @@ export function parseBunkerUri(uri: string): BunkerPointer {
       url.searchParams
         .getAll("relay")
         .map((value) => value.trim())
-        .filter(Boolean)
+        .filter((value) => value.length > 0 && value.length <= MAX_BUNKER_RELAY_URL_CHARS)
     )
   ].slice(0, MAX_BUNKER_RELAYS);
   if (relayUrls.length === 0) {
@@ -113,6 +123,9 @@ export function parseBunkerUri(uri: string): BunkerPointer {
     );
   }
   const secret = url.searchParams.get("secret") ?? undefined;
+  if (secret && secret.length > MAX_BUNKER_SECRET_CHARS) {
+    throw new BunkerUriParseError("The bunker URI's secret is unreasonably long.");
+  }
   return { signerPubkey, relayUrls, secret: secret || undefined };
 }
 
@@ -359,19 +372,21 @@ export class Nip46Client {
   }
 
   /**
-   * Courtesy logout per current NIP-46 -- explicitly not a security boundary
-   * (the signer revokes authorization on its own side); best-effort, bounded,
-   * and never allowed to block or fail teardown. Call before close().
+   * Pre-signs the courtesy protocol logout event (current NIP-46 defines
+   * logout as exactly that -- a courtesy, never a security boundary). Must
+   * run BEFORE close(), while the keys still exist; the caller then closes
+   * IMMEDIATELY (local revocation must not wait on the network, BL-26) and
+   * delivers the pre-signed event afterward via publishCourtesy().
    */
-  async logout(timeoutMs = 2000): Promise<void> {
-    if (this.closed) return;
+  buildLogoutEvent(): NostrEvent | null {
+    if (this.closed) return null;
     try {
       const id = bytesToHex(randomBytes(8));
       const content = nip44Encrypt(
         this.conversationKey,
         JSON.stringify({ id, method: "logout", params: [] })
       );
-      const event = signNostrEvent(
+      return signNostrEvent(
         {
           pubkey: this.clientPubkey,
           created_at: Math.floor(Date.now() / 1000),
@@ -381,12 +396,26 @@ export class Nip46Client {
         },
         this.clientSecretKey
       );
+    } catch {
+      return null;
+    }
+  }
+
+  /**
+   * Best-effort, bounded delivery of a pre-signed event. Safe (and intended)
+   * to run AFTER close(): it needs no key material, and RelayConnection
+   * reconnects transiently to publish. Never throws.
+   */
+  async publishCourtesy(event: NostrEvent, timeoutMs = 2000): Promise<void> {
+    try {
       await Promise.race([
         Promise.allSettled(this.conns.map((conn) => conn.publish(event))),
         new Promise((resolve) => setTimeout(resolve, timeoutMs))
       ]);
     } catch {
       // Courtesy only.
+    } finally {
+      for (const conn of this.conns) conn.close();
     }
   }
 

packages/core/src/nostr/relay.ts

@@ -69,6 +69,26 @@ function verifyBudgetFor(filter: NostrFilter): number {
   return Math.max(50, cap * 3);
 }
 
+/** Live-subscription protections (BL-25): a bounded already-seen id cache
+ *  (replaying one retained valid event must cost one verification and one
+ *  delivery, ever) and a fixed-window verification rate budget (a relay
+ *  spraying DISTINCT events gets quarantined for the rest of the window
+ *  rather than burning CPU). Legitimate NIP-46 traffic -- responses to our
+ *  own requests plus the odd auth_url -- sits orders of magnitude below
+ *  the budget. */
+const LIVE_SEEN_CAP = 2048;
+const LIVE_WINDOW_MS = 10_000;
+const LIVE_VERIFIES_PER_WINDOW = 100;
+
+function rememberSeen(seenIds: Set<string>, id: string): void {
+  if (seenIds.size >= LIVE_SEEN_CAP) {
+    // Sets iterate in insertion order; evict the oldest.
+    const oldest = seenIds.values().next().value;
+    if (oldest !== undefined) seenIds.delete(oldest);
+  }
+  seenIds.add(id);
+}
+
 export class RelayConnection {
   readonly url: string;
   private ws: WebSocketLike | null = null;
@@ -82,6 +102,9 @@ export class RelayConnection {
       /** Remaining signature verifications this subscription will pay for;
        *  Infinity for live subscriptions (BL-21). */
       verifyBudget: number;
+      /** Fixed-window verification rate state for live subscriptions (BL-25). */
+      liveWindowStart: number;
+      liveVerifies: number;
       filter: NostrFilter;
       onEose: () => void;
       /** Present on live subscriptions (subscribeLive): each verified,
@@ -176,17 +199,30 @@ export class RelayConnection {
       // Relays are untrusted and are not entitled to define the result set.
       // Verify both the event and the full requested NIP-01 filter centrally;
       // callers may still re-check security-sensitive identity constraints.
-      if (sub.onEvent) {
-        if (verifyNostrEvent(event) && matchesNostrFilter(event, sub.filter)) sub.onEvent(event);
-        return;
-      }
-      // Buffered query: every CHEAP check runs before Schnorr verification,
-      // so replayed or over-limit spam cannot buy signature checks (BL-21).
-      // Replayed copies of an accepted id cost one Set lookup; events past
-      // capacity cost nothing; and a hostile relay spraying distinct invalid
-      // events runs out of verification budget rather than our CPU.
+      // Every CHEAP check runs before Schnorr verification, so replayed or
+      // over-limit spam cannot buy signature checks (BL-21, BL-25).
       const claimedId = (event as { id?: unknown } | null | undefined)?.id;
       if (typeof claimedId !== "string" || sub.seenIds.has(claimedId)) return;
+      if (sub.onEvent) {
+        // Live subscription: bounded dedup plus a rate-limited verification
+        // window (BL-25). Exceeding the window quarantines further delivery
+        // from this relay until it rolls over.
+        const now = Date.now();
+        if (now - sub.liveWindowStart >= LIVE_WINDOW_MS) {
+          sub.liveWindowStart = now;
+          sub.liveVerifies = 0;
+        }
+        if (sub.liveVerifies >= LIVE_VERIFIES_PER_WINDOW) return;
+        sub.liveVerifies++;
+        if (!verifyNostrEvent(event) || !matchesNostrFilter(event, sub.filter)) return;
+        rememberSeen(sub.seenIds, event.id);
+        sub.onEvent(event);
+        return;
+      }
+      // Buffered query: replayed copies of an accepted id cost one Set
+      // lookup; events past capacity cost nothing; and a hostile relay
+      // spraying distinct invalid events runs out of verification budget
+      // rather than our CPU.
       const cap = Math.min(sub.filter.limit ?? MAX_BUFFERED_EVENTS, MAX_BUFFERED_EVENTS);
       if (sub.events.length >= cap) return;
       if (sub.verifyBudget <= 0) return;
@@ -276,7 +312,16 @@ export class RelayConnection {
   ): Promise<{ close: () => void }> {
     await this.connect();
     const subId = bytesToHex(randomBytes(8));
-    this.subs.set(subId, { events: [], seenIds: new Set(), verifyBudget: Number.POSITIVE_INFINITY, filter, onEose: () => {}, onEvent });
+    this.subs.set(subId, {
+      events: [],
+      seenIds: new Set(),
+      verifyBudget: Number.POSITIVE_INFINITY,
+      liveWindowStart: Date.now(),
+      liveVerifies: 0,
+      filter,
+      onEose: () => {},
+      onEvent
+    });
     this.send(["REQ", subId, filter]);
     return {
       close: () => {
@@ -333,7 +378,15 @@ export class RelayConnection {
           ),
         timeoutMs,
       );
-      this.subs.set(subId, { events, seenIds: new Set(), verifyBudget: verifyBudgetFor(filter), filter, onEose: () => settle() });
+      this.subs.set(subId, {
+        events,
+        seenIds: new Set(),
+        verifyBudget: verifyBudgetFor(filter),
+        liveWindowStart: 0,
+        liveVerifies: 0,
+        filter,
+        onEose: () => settle()
+      });
       this.send(["REQ", subId, filter]);
     });
   }

packages/core/src/nostr/relayHardening.test.ts

@@ -116,6 +116,39 @@ describe("relay hardening", () => {
     expect(events.length).toBeLessThanOrEqual(1);
   });
 
+  it("BL-25: live subscriptions dedupe replayed events -- one delivery, ever", async () => {
+    const key = generatePrivateKey();
+    const note = signedNote(key, "live");
+    relay.unsolicitedQueryEvents = Array.from({ length: 2000 }, () => note);
+    const delivered: string[] = [];
+    const start = Date.now();
+    const handle = await conn.subscribeLive({ kinds: [1], authors: [getPublicKeyHex(key)] }, (event) =>
+      delivered.push(event.id)
+    );
+    await new Promise((resolve) => setTimeout(resolve, 600));
+    handle.close();
+    expect(delivered).toHaveLength(1);
+    // 2,000 replayed frames cost one verification plus Set lookups.
+    expect(Date.now() - start).toBeLessThan(3_000);
+  });
+
+  it("BL-25: live subscriptions rate-limit verification of distinct events per window", async () => {
+    const key = generatePrivateKey();
+    relay.unsolicitedQueryEvents = Array.from({ length: 300 }, (_, i) =>
+      signedNote(key, `distinct-${i}`, 1_700_000_000 + i)
+    );
+    const delivered: string[] = [];
+    const handle = await conn.subscribeLive({ kinds: [1], authors: [getPublicKeyHex(key)] }, (event) =>
+      delivered.push(event.id)
+    );
+    await new Promise((resolve) => setTimeout(resolve, 600));
+    handle.close();
+    // The fixed window admits at most 100 verifications; the rest of the
+    // spray is quarantined until the window rolls over.
+    expect(delivered.length).toBeGreaterThan(0);
+    expect(delivered.length).toBeLessThanOrEqual(100);
+  });
+
   it("BL-19: distinct events beyond the requested limit are not buffered", async () => {
     const key = generatePrivateKey();
     relay.unsolicitedQueryEvents = Array.from({ length: 10 }, (_, i) =>

packages/widget/src/worker/cryptoWorker.ts

@@ -277,14 +277,16 @@ function discardNostrconnectPending(): void {
   nostrconnectPending = null;
 }
 
-/** Retires a previous session with the courtesy protocol logout (current
- *  NIP-46 defines it as exactly that -- not a security boundary), then
- *  closes it, which wipes its keys. */
+/** Retires a session with local revocation FIRST (BL-26): the logout event
+ *  is pre-signed while the key exists, then close() immediately rejects
+ *  every pending request and wipes keys -- nothing can resolve after logout
+ *  begins -- and only then is the courtesy event delivered, asynchronously
+ *  and bounded. Current NIP-46 defines logout as a courtesy, never a
+ *  security boundary, so its delivery is allowed to fail silently. */
 function retireNip46Client(client: Nip46Client): void {
-  void client
-    .logout()
-    .catch(() => {})
-    .finally(() => client.close());
+  const farewell = client.buildLogoutEvent();
+  client.close();
+  if (farewell) void client.publishCourtesy(farewell);
 }
 
 function adoptNip46Client(client: Nip46Client, userPubkey: string): void {
@@ -343,13 +345,23 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
     case "nip46NostrconnectAwait": {
       const pending = nostrconnectPending;
       if (!pending) throw new Error("No signer-connection attempt is in progress.");
-      const { signerPubkey } = await listenForNostrconnect({
-        clientSecretKey: pending.clientSecretKey,
-        relayUrls: pending.relayUrls,
-        secret: pending.secret,
-        timeoutMs: 180_000,
-        signal: pending.abort.signal
-      });
+      let signerPubkey: string;
+      try {
+        ({ signerPubkey } = await listenForNostrconnect({
+          clientSecretKey: pending.clientSecretKey,
+          relayUrls: pending.relayUrls,
+          secret: pending.secret,
+          timeoutMs: 180_000,
+          signal: pending.abort.signal
+        }));
+      } catch (err) {
+        // Natural failure (timeout, unreachable relays) must not orphan the
+        // ephemeral key in worker memory (BL-27) -- but only if this attempt
+        // still owns the pending slot; a superseding attempt already
+        // discarded it.
+        if (nostrconnectPending === pending) discardNostrconnectPending();
+        throw err;
+      }
       if (nostrconnectPending !== pending) throw new Error("This signer-connection attempt was superseded.");
       // Ownership of the ephemeral key transfers into the client below --
       // dereference only; discardNostrconnectPending would wipe it.