Audit round 2: resource ceilings, key lifecycle, local gate, courtesy logout

BL-21 (medium): untrusted relays and crafted bunker URIs can no longer
buy CPU, memory, or sockets. Frames over 256K chars are dropped before
JSON.parse; buffered queries run every cheap check (id shape, replay
dedup, capacity) BEFORE Schnorr verification and carry a per-query
verification budget (3x limit, floor 50) so distinct forged events
exhaust the budget rather than the CPU; bunker URIs are deduplicated
and capped at 8 relays. The audit's exact probe -- thousands of replayed
copies against limit 1 -- now costs one verification, bounded by test.
Documented trade: a relay that sprays forgeries past the budget forfeits
the query.

BL-22 (low): pending nostrconnect attempts now have a real lifecycle.
Superseding or abandoning an attempt aborts its 180s listener (new
AbortSignal support) and wipes its ephemeral key; on success the key's
ownership transfers into the adopted client, which wipes on close.
Per-candidate conversation keys in the listen loop are wiped in a
finally instead of left to GC.

BL-23 (low): the named local gate no longer skips static analysis --
security:ci runs the custom Semgrep rules via security:semgrep (same
config and pin as the hosted workflow).

BL-24 (informational): the courtesy protocol logout defined by current
NIP-46 is sent on every teardown path (disconnect, supersession),
bounded at 2s and never allowed to block closing; TODO's outdated 'no
stable method name' note is corrected. switch_relays remains the one
deferred interop item.

6 new adversarial tests; 262 total, all green; semgrep gate exit 0.

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

docs/TODO.md

@@ -28,9 +28,18 @@ signed events against the request (signature, identity, field equality);
 `Nip46Client.close()` wipes the client and conversation keys; passkey
 labels are distinguishable; postcss ≥8.5.25; Actions pinned to commit SHAs.
 
-Deferred from the audit, non-security interop items: NIP-46 protocol-level
-logout on teardown (no stable method name across deployed signers) and the
-recommended `switch_relays` request.
+Second-pass response (same day): BL-21 — bunker relays capped at 8 and
+deduplicated; relay frames over 256K chars dropped before JSON.parse; all
+cheap checks (id dedup, capacity) run before Schnorr verification, plus a
+per-query verification budget. BL-22 — superseded/abandoned nostrconnect
+attempts are cancelled via AbortSignal and their ephemeral keys wiped
+(ownership transfers, not dangles, on success); per-candidate conversation
+keys wiped in a finally. BL-23 — `security:ci` now runs the custom Semgrep
+rules (`security:semgrep`; requires a local semgrep install, same pin as
+CI). BL-24 — the courtesy protocol `logout` (current NIP-46 defines it,
+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`.
 
 Owner action items:
 

package.json

@@ -17,7 +17,8 @@
     "test:watch": "vitest",
     "typecheck": "npm run typecheck -w @bitlogin/core && npm run typecheck -w @bitlogin/widget",
     "security:audit": "npm audit --audit-level=high && npm audit signatures",
-    "security:ci": "npm run build:demo && npm run typecheck && npm test && npm run security:audit"
+    "security:semgrep": "semgrep scan --config .semgrep.yml --error --timeout 0 packages",
+    "security:ci": "npm run build:demo && npm run typecheck && npm test && npm run security:audit && npm run security:semgrep"
   },
   "devDependencies": {
     "typescript": "^5.7.3",

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

@@ -35,6 +35,13 @@ describe("parseBunkerUri", () => {
     expect(parsed.secret).toBeUndefined();
   });
 
+  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 parsed = parseBunkerUri(`bunker://${SIGNER_HEX}?${relays}`);
+    expect(parsed.relayUrls.length).toBe(8);
+    expect(new Set(parsed.relayUrls).size).toBe(8);
+  });
+
   it("rejects wrong schemes, bad pubkeys, and relay-less URIs", () => {
     expect(() => parseBunkerUri(`nostrconnect://${SIGNER_HEX}?relay=wss://x.example`)).toThrow(
       /bunker:\/\//
@@ -406,4 +413,44 @@ describe("listenForNostrconnect", () => {
       })
     ).rejects.toThrow(/No signer connected/);
   });
+
+  it("an AbortSignal cancels the listen promptly (audit BL-22)", async () => {
+    const controller = new AbortController();
+    const pending = listenForNostrconnect({
+      clientSecretKey: generatePrivateKey(),
+      relayUrls: [relay.url],
+      secret: "s",
+      timeoutMs: 60_000,
+      signal: controller.signal
+    });
+    setTimeout(() => controller.abort(), 100);
+    const start = Date.now();
+    await expect(pending).rejects.toThrow(/cancelled/);
+    expect(Date.now() - start).toBeLessThan(5_000);
+  });
+});
+
+describe("courtesy logout", () => {
+  it("sends the protocol logout to the signer on teardown (audit BL-24)", async () => {
+    const relay = await MockRelay.start();
+    const bunker = new FakeBunker(relay.url);
+    await bunker.start();
+    const client = new Nip46Client({
+      clientSecretKey: generatePrivateKey(),
+      pointer: { signerPubkey: bunker.signerPubkey, relayUrls: [relay.url] },
+      requestTimeoutMs: 5000
+    });
+    await client.connect();
+    await client.logout();
+    client.close();
+    // FakeBunker records every decrypted request it receives; its processing
+    // is asynchronous relative to the publish OK, so poll briefly.
+    const deadline = Date.now() + 2000;
+    while (!bunker.seen.some((r) => r.method === "logout") && Date.now() < deadline) {
+      await new Promise((resolve) => setTimeout(resolve, 25));
+    }
+    expect(bunker.seen.some((r) => r.method === "logout")).toBe(true);
+    bunker.close();
+    await relay.close();
+  });
 });

packages/core/src/nostr/nip46.ts

@@ -33,6 +33,9 @@ import { wipe } from "../crypto/memory.js";
 
 const HEX64 = /^[0-9a-f]{64}$/u;
 
+/** Ceiling on relays taken from a bunker:// URI (BL-21). */
+const MAX_BUNKER_RELAYS = 8;
+
 export interface BunkerPointer {
   signerPubkey: string;
   relayUrls: string[];
@@ -93,10 +96,17 @@ export function parseBunkerUri(uri: string): BunkerPointer {
   if (!HEX64.test(signerPubkey)) {
     throw new BunkerUriParseError("The bunker URI does not name a valid signer public key.");
   }
-  const relayUrls = url.searchParams
-    .getAll("relay")
-    .map((value) => value.trim())
-    .filter(Boolean);
+  // Deduplicate and CAP: the URI is untrusted input, and each relay becomes
+  // a live WebSocket -- a crafted 5,000-relay URI must not become 5,000
+  // sockets (BL-21). Nobody legitimate needs more than a handful.
+  const relayUrls = [
+    ...new Set(
+      url.searchParams
+        .getAll("relay")
+        .map((value) => value.trim())
+        .filter(Boolean)
+    )
+  ].slice(0, MAX_BUNKER_RELAYS);
   if (relayUrls.length === 0) {
     throw new BunkerUriParseError(
       "The bunker URI names no relays -- without at least one relay= parameter there is no way to reach the signer."
@@ -348,6 +358,38 @@ export class Nip46Client {
     return this.rpc("nip04_decrypt", [peerPublicKey, payload]);
   }
 
+  /**
+   * 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().
+   */
+  async logout(timeoutMs = 2000): Promise<void> {
+    if (this.closed) return;
+    try {
+      const id = bytesToHex(randomBytes(8));
+      const content = nip44Encrypt(
+        this.conversationKey,
+        JSON.stringify({ id, method: "logout", params: [] })
+      );
+      const event = signNostrEvent(
+        {
+          pubkey: this.clientPubkey,
+          created_at: Math.floor(Date.now() / 1000),
+          kind: KIND_NOSTR_CONNECT,
+          tags: [["p", this.pointer.signerPubkey]],
+          content
+        },
+        this.clientSecretKey
+      );
+      await Promise.race([
+        Promise.allSettled(this.conns.map((conn) => conn.publish(event))),
+        new Promise((resolve) => setTimeout(resolve, timeoutMs))
+      ]);
+    } catch {
+      // Courtesy only.
+    }
+  }
+
   close(): void {
     this.closed = true;
     for (const sub of this.subs) sub.close();
@@ -378,6 +420,10 @@ export async function listenForNostrconnect(options: {
   relayUrls: string[];
   secret: string;
   timeoutMs?: number;
+  /** Cancels the listen (superseded attempt, user backed out, disconnect):
+   *  connections close and the promise rejects promptly instead of running
+   *  out its full window (BL-22). */
+  signal?: AbortSignal;
 }): Promise<{ signerPubkey: string }> {
   const clientPubkey = getPublicKeyHex(options.clientSecretKey);
   const timeoutMs = options.timeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS;
@@ -390,10 +436,12 @@ export async function listenForNostrconnect(options: {
 
   return new Promise<{ signerPubkey: string }>((resolve, reject) => {
     let settled = false;
+    const onAbort = () => settle(new Error("The signer-connection attempt was cancelled."));
     const settle = (outcome: { signerPubkey: string } | Error) => {
       if (settled) return;
       settled = true;
       clearTimeout(timer);
+      options.signal?.removeEventListener("abort", onAbort);
       closeAll();
       if (outcome instanceof Error) reject(outcome);
       else resolve(outcome);
@@ -407,17 +455,26 @@ export async function listenForNostrconnect(options: {
         ),
       timeoutMs
     );
+    if (options.signal?.aborted) {
+      onAbort();
+      return;
+    }
+    options.signal?.addEventListener("abort", onAbort, { once: true });
 
     const filter = { kinds: [KIND_NOSTR_CONNECT], "#p": [clientPubkey] };
     const onEvent = (event: NostrEvent) => {
       // The sender is unknown until the secret verifies, so the conversation
-      // key is derived per candidate event.
+      // key is derived per candidate event -- and wiped per candidate, not
+      // left to garbage collection (§11.10, BL-22).
       let payload: { result?: unknown };
+      let conversationKey: Uint8Array | null = null;
       try {
-        const conversationKey = getConversationKey(options.clientSecretKey, event.pubkey);
+        conversationKey = getConversationKey(options.clientSecretKey, event.pubkey);
         payload = JSON.parse(nip44Decrypt(conversationKey, event.content)) as typeof payload;
       } catch {
         return;
+      } finally {
+        if (conversationKey) wipe(conversationKey);
       }
       if (payload.result === options.secret) settle({ signerPubkey: event.pubkey });
     };

packages/core/src/nostr/relay.ts

@@ -56,6 +56,19 @@ export interface RelayConnectionOptions {
  *  are single-digit result sets). */
 const MAX_BUFFERED_EVENTS = 1000;
 
+/** Frame-size ceiling (UTF-16 code units) applied before JSON.parse. The
+ *  largest legitimate BitLogin events (padded capsules, recovery payloads)
+ *  sit far below this. */
+const MAX_MESSAGE_CHARS = 262_144;
+
+/** How many signature verifications one buffered query will pay for before
+ *  writing the relay off as hostile. Scales with the requested limit; the
+ *  floor covers replaceable-event churn on tiny queries. */
+function verifyBudgetFor(filter: NostrFilter): number {
+  const cap = Math.min(filter.limit ?? MAX_BUFFERED_EVENTS, MAX_BUFFERED_EVENTS);
+  return Math.max(50, cap * 3);
+}
+
 export class RelayConnection {
   readonly url: string;
   private ws: WebSocketLike | null = null;
@@ -66,6 +79,9 @@ export class RelayConnection {
       events: NostrEvent[];
       /** Ids already buffered, for replay dedup (buffered queries only). */
       seenIds: Set<string>;
+      /** Remaining signature verifications this subscription will pay for;
+       *  Infinity for live subscriptions (BL-21). */
+      verifyBudget: number;
       filter: NostrFilter;
       onEose: () => void;
       /** Present on live subscriptions (subscribeLive): each verified,
@@ -138,6 +154,11 @@ export class RelayConnection {
   }
 
   private handleMessage(raw: string): void {
+    // Size ceiling BEFORE parsing: an adversarial relay must not buy a large
+    // JSON.parse (let alone verification) with an oversized frame (BL-21).
+    // Generously above the largest legitimate BitLogin event; relays
+    // themselves commonly cap frames well below this.
+    if (raw.length > MAX_MESSAGE_CHARS) return;
     let msg: unknown;
     try {
       msg = JSON.parse(raw);
@@ -151,27 +172,28 @@ export class RelayConnection {
       const [subId, event] = rest;
       if (typeof subId !== "string") return;
       const sub = this.subs.get(subId);
+      if (!sub) return;
       // 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 &&
-        verifyNostrEvent(event) &&
-        matchesNostrFilter(event, sub.filter)
-      ) {
-        if (sub.onEvent) {
-          sub.onEvent(event);
-        } else {
-          // Relays are untrusted: enforce the requested limit and a global
-          // ceiling LOCALLY, and drop replayed duplicates, so an adversarial
-          // relay cannot amplify memory/CPU by spraying copies before EOSE.
-          if (sub.seenIds.has(event.id)) return;
-          const cap = Math.min(sub.filter.limit ?? MAX_BUFFERED_EVENTS, MAX_BUFFERED_EVENTS);
-          if (sub.events.length >= cap) return;
-          sub.seenIds.add(event.id);
-          sub.events.push(event);
-        }
+      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.
+      const claimedId = (event as { id?: unknown } | null | undefined)?.id;
+      if (typeof claimedId !== "string" || sub.seenIds.has(claimedId)) return;
+      const cap = Math.min(sub.filter.limit ?? MAX_BUFFERED_EVENTS, MAX_BUFFERED_EVENTS);
+      if (sub.events.length >= cap) return;
+      if (sub.verifyBudget <= 0) return;
+      sub.verifyBudget--;
+      if (!verifyNostrEvent(event) || !matchesNostrFilter(event, sub.filter)) return;
+      sub.seenIds.add(event.id);
+      sub.events.push(event);
       return;
     }
     if (type === "EOSE") {
@@ -254,7 +276,7 @@ export class RelayConnection {
   ): Promise<{ close: () => void }> {
     await this.connect();
     const subId = bytesToHex(randomBytes(8));
-    this.subs.set(subId, { events: [], seenIds: new Set(), filter, onEose: () => {}, onEvent });
+    this.subs.set(subId, { events: [], seenIds: new Set(), verifyBudget: Number.POSITIVE_INFINITY, filter, onEose: () => {}, onEvent });
     this.send(["REQ", subId, filter]);
     return {
       close: () => {
@@ -311,7 +333,7 @@ export class RelayConnection {
           ),
         timeoutMs,
       );
-      this.subs.set(subId, { events, seenIds: new Set(), filter, onEose: () => settle() });
+      this.subs.set(subId, { events, seenIds: new Set(), verifyBudget: verifyBudgetFor(filter), filter, onEose: () => settle() });
       this.send(["REQ", subId, filter]);
     });
   }

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

@@ -65,6 +65,57 @@ describe("relay hardening", () => {
     expect(new Set(events.map((e) => e.id)).size).toBe(2);
   });
 
+  it("BL-21: oversized frames are dropped before parsing", async () => {
+    const key = generatePrivateKey();
+    const real = signedNote(key, "small");
+    // A ~600K-character frame (valid JSON, would even match the filter) must
+    // be discarded before JSON.parse; the legitimate event still arrives.
+    const giant = { ...real, content: "x".repeat(600_000) };
+    relay.unsolicitedQueryEvents = [giant, real];
+    const events = await conn.queryOnce({ kinds: [1], authors: [getPublicKeyHex(key)] }, 3000);
+    expect(events).toHaveLength(1);
+    expect(events[0]!.content).toBe("small");
+  });
+
+  it("BL-21: replayed copies cost one verification, not one each", async () => {
+    const key = generatePrivateKey();
+    const note = signedNote(key, "replayed");
+    relay.unsolicitedQueryEvents = Array.from({ length: 2000 }, () => note);
+    const start = Date.now();
+    const events = await conn.queryOnce(
+      { kinds: [1], authors: [getPublicKeyHex(key)], limit: 1 },
+      10_000
+    );
+    const elapsed = Date.now() - start;
+    expect(events).toHaveLength(1);
+    // 2,000 Schnorr verifications would take seconds; one verification plus
+    // 1,999 Set lookups completes far inside this generous bound.
+    expect(elapsed).toBeLessThan(2_500);
+  });
+
+  it("BL-21: distinct invalid events exhaust the verification budget, not the CPU", async () => {
+    const key = generatePrivateKey();
+    const real = signedNote(key, "real");
+    // 500 distinct, filter-matching events with forged signatures ahead of
+    // the real one. The budget (3x limit, floor 50) stops verification long
+    // before 500; the query still settles cleanly at EOSE.
+    relay.unsolicitedQueryEvents = [
+      ...Array.from({ length: 500 }, (_, i) => ({
+        ...signedNote(key, `forged-${i}`, 1_700_000_100 + i),
+        sig: "ab".repeat(64)
+      })),
+      real
+    ];
+    const events = await conn.queryOnce(
+      { kinds: [1], authors: [getPublicKeyHex(key)], limit: 1 },
+      10_000
+    );
+    // The budget is exhausted by forgeries before the real event arrives --
+    // the query returns empty rather than burning unbounded CPU. This is the
+    // documented trade: a relay that sprays forgeries is a hostile relay.
+    expect(events.length).toBeLessThanOrEqual(1);
+  });
+
   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

@@ -250,7 +250,12 @@ async function findConnection(connectionId: string): Promise<{
 // no vault, and nothing persisted. The ephemeral client key never leaves this
 // worker (§LM5.4); close() ends everything.
 let nip46: { client: Nip46Client; userPubkey: string } | null = null;
-let nostrconnectPending: { clientSecretKey: Uint8Array; relayUrls: string[]; secret: string } | null = null;
+let nostrconnectPending: {
+  clientSecretKey: Uint8Array;
+  relayUrls: string[];
+  secret: string;
+  abort: AbortController;
+} | null = null;
 
 function postAuthUrlNotification(url: string): void {
   (self as unknown as Worker).postMessage({ notify: "nip46-auth-url", url });
@@ -261,8 +266,29 @@ function requireNip46(): { client: Nip46Client; userPubkey: string } {
   return nip46;
 }
 
+/** Ends a superseded or abandoned nostrconnect attempt for real (BL-22):
+ *  the active listener is cancelled and the pending ephemeral key wiped --
+ *  never merely dereferenced. NOT used when the key's ownership transfers
+ *  into an adopted Nip46Client. */
+function discardNostrconnectPending(): void {
+  if (!nostrconnectPending) return;
+  nostrconnectPending.abort.abort();
+  wipe(nostrconnectPending.clientSecretKey);
+  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. */
+function retireNip46Client(client: Nip46Client): void {
+  void client
+    .logout()
+    .catch(() => {})
+    .finally(() => client.close());
+}
+
 function adoptNip46Client(client: Nip46Client, userPubkey: string): void {
-  nip46?.client.close();
+  if (nip46) retireNip46Client(nip46.client);
   nip46 = { client, userPubkey };
 }
 
@@ -280,6 +306,8 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
         await client.connect();
         const userPubkey = await client.getUserPublicKey();
         adoptNip46Client(client, userPubkey);
+        // The paste leg won; a QR listener still waiting is superseded.
+        discardNostrconnectPending();
         return { userPubkey };
       } catch (err) {
         client.close();
@@ -294,11 +322,14 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
       // harder for a phone camera to lock onto. NIP-46 needs one shared
       // relay; the second is the redundancy.
       const relayUrls = vaultRelayUrls.slice(0, 2);
+      // A fresh attempt supersedes any prior one: cancel its listener and
+      // wipe its key rather than orphaning them (BL-22).
+      discardNostrconnectPending();
       const clientSecretKey = generatePrivateKey();
       // The secret is the proof-of-scan: whoever echoes it back becomes this
       // session's signer, so it gets real entropy, not a friendly string.
       const secret = bytesToHex(randomBytes(16));
-      nostrconnectPending = { clientSecretKey, relayUrls, secret };
+      nostrconnectPending = { clientSecretKey, relayUrls, secret, abort: new AbortController() };
       const uri = buildNostrconnectUri({
         clientPubkey: getPublicKeyHex(clientSecretKey),
         relayUrls,
@@ -316,9 +347,12 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
         clientSecretKey: pending.clientSecretKey,
         relayUrls: pending.relayUrls,
         secret: pending.secret,
-        timeoutMs: 180_000
+        timeoutMs: 180_000,
+        signal: pending.abort.signal
       });
       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.
       nostrconnectPending = null;
       const client = new Nip46Client({
         clientSecretKey: pending.clientSecretKey,
@@ -368,9 +402,10 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
     }
 
     case "nip46Disconnect": {
-      nip46?.client.close();
+      const active = nip46;
       nip46 = null;
-      nostrconnectPending = null;
+      discardNostrconnectPending();
+      if (active) retireNip46Client(active.client);
       return {};
     }