From 47a5071b4a0b3993a4a7891d4b8d87b0a514d684 Mon Sep 17 00:00:00 2001
From: Padreug <padreug@aiolabs.dev>
Date: Sat, 27 Jun 2026 13:08:37 +0200
Subject: [PATCH] =?UTF-8?q?fix(admin):=20clear=20pending=20callbacks=20on?=
 =?UTF-8?q?=20ACL=20timeout/response=20=E2=80=94=20stop=20the=20leak=20(re?=
 =?UTF-8?q?view=20AD-1/CS-2)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

requestPermission's 10s approval timeout called resolve(undefined) but never
clearPending, and a single admin's response cleared only its own id. So every
timed-out approval, and (with multiple admin npubs) the non-responding admins'
entries, leaked permanently in transport.pending — each retaining its closure
over remotePubkey/keyName/method/the serialized sign_event payload, so growth
tracked signing traffic. Scope: the admin-DM auth path (only when no web baseUrl
is set).

- Collect every issued request id; a single `finish()` resolves once and clears
  ALL of them, on both timeout and the first response.
- A `settled` latch makes late/duplicate responses no-ops — which also stops a
  late 'always' approval from running allowAllRequestsFromKey after the request
  already resolved (review AD-2).
- Guard nip19.decode so a malformed admin npub skips that admin instead of
  throwing through the loop.
- Defense-in-depth: bound transport.pending at 1000 (evict oldest) so any other
  un-cleared path can't grow it without limit.

tsc at baseline; daemon bundles; admin + nip46 suites green.

Refs: transport review AD-1/CS-2/AD-2; #42
---
 src/daemon/admin/index.ts     | 33 +++++++++++++++++++++++++++------
 src/daemon/nip46/transport.ts |  8 ++++++++
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/src/daemon/admin/index.ts b/src/daemon/admin/index.ts
index a54ad60..a085607 100644
--- a/src/daemon/admin/index.ts
+++ b/src/daemon/admin/index.ts
@@ -397,13 +397,33 @@ class AdminInterface {
         return new Promise((resolve) => {
             console.log(`requesting permission for`, keyName, { remotePubkey, method });
 
+            const ids: string[] = [];
+            let settled = false;
+            // Resolve once and ALWAYS clear every pending sendRequest callback —
+            // on timeout AND on the first admin response. Previously the timeout
+            // cleared nothing and a single admin's response cleared only its own
+            // id, so every timed-out request and (with multiple admins) the
+            // non-responding admins' entries leaked in transport.pending for the
+            // process lifetime (review AD-1/CS-2). The `settled` latch also stops
+            // a late approval from acting after the request resolved (review AD-2).
+            const finish = (value: boolean | undefined) => {
+                if (settled) return;
+                settled = true;
+                for (const id of ids) this.transport.clearPending(id);
+                resolve(value);
+            };
+
             // If an admin doesn't respond within 10 seconds, report timeout.
-            setTimeout(() => {
-                resolve(undefined);
-            }, 10000);
+            setTimeout(() => finish(undefined), 10000);
 
             for (const npub of this.npubs) {
-                const adminPubkey = nip19.decode(npub).data as string;
+                let adminPubkey: string;
+                try {
+                    adminPubkey = nip19.decode(npub).data as string;
+                } catch {
+                    console.log(`skipping malformed admin npub: ${npub}`);
+                    continue;
+                }
                 const params = JSON.stringify({
                     keyName,
                     remotePubkey,
@@ -419,17 +439,18 @@ class AdminInterface {
                     'nip44',
                     NIP46_ADMIN_RESPONSE_KIND,
                     (res) => {
-                        this.transport.clearPending(id);
+                        if (settled) return; // ignore late / duplicate responses
                         this.requestPermissionResponse(
                             remotePubkey,
                             keyName,
                             method,
                             param,
-                            resolve,
+                            finish,
                             res
                         );
                     }
                 );
+                ids.push(id);
             }
         });
     }
diff --git a/src/daemon/nip46/transport.ts b/src/daemon/nip46/transport.ts
index b787cee..2a08a5f 100644
--- a/src/daemon/nip46/transport.ts
+++ b/src/daemon/nip46/transport.ts
@@ -132,6 +132,14 @@ export class Nip46Transport {
     ): string {
         const id = Math.random().toString(36).substring(2, 12);
         this.pending.set(id, cb);
+        // Defense-in-depth bound: callers (admin requestPermission) clear pending
+        // entries on resolve/timeout, but cap the map so any un-cleared path can't
+        // grow it without limit — evict the oldest (it would time out anyway).
+        // (review AD-1/CS-2)
+        if (this.pending.size > 1000) {
+            const oldest = this.pending.keys().next().value;
+            if (oldest !== undefined) this.pending.delete(oldest);
+        }
         const content = this.encrypt(remotePubkey, JSON.stringify({ id, method, params }), encryption);
         const event = finalizeEvent(
             { kind, created_at: Math.floor(Date.now() / 1000), tags: [["p", remotePubkey]], content },