From 434817c899152adb4b044c7a6c60b01651f482d9 Mon Sep 17 00:00:00 2001 From: Padreug Date: Sat, 27 Jun 2026 12:22:24 +0200 Subject: [PATCH] =?UTF-8?q?fix(admin):=20harden=20the=20boot=20DM=20?= =?UTF-8?q?=E2=80=94=20guard=20teardown=20+=20unhandled=20rejection=20(rev?= =?UTF-8?q?iew=20CS-3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folds the review's CS-3 finding into the boot-DM fix: - notifyAdminsOfNewConnection wraps the publish loop in try/finally so a throw mid-loop can't leak the throwaway pool's reconnect loops + sockets for the process lifetime. - The constructor's fire-and-forget call now .catch()es (both the notify and the config() it chains off), so a boot-DM failure can't surface as an unhandled rejection — process-terminating under Node defaults. - dmUser guards its whole body: nip19.decode/nip04.encrypt ran *before* the old publish-only try, so a malformed admin npub (passes startsWith, fails the bech32 checksum) threw past the caller. Now best-effort, never throws. Refs: #48, review CS-3 --- src/daemon/admin/index.ts | 20 +++++++++++++++----- src/utils/dm-user.ts | 32 ++++++++++++++++++-------------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/daemon/admin/index.ts b/src/daemon/admin/index.ts index 36764bb..569e1c3 100644 --- a/src/daemon/admin/index.ts +++ b/src/daemon/admin/index.ts @@ -110,9 +110,13 @@ class AdminInterface { this.config().then((config) => { if (config.admin?.notifyAdminsOnBoot) { - this.notifyAdminsOfNewConnection(connectionString); + // .catch so a boot-DM failure can't surface as an unhandled + // rejection (process-terminating under Node defaults). #48 / CS-3. + this.notifyAdminsOfNewConnection(connectionString).catch((e) => + console.log('notifyAdminsOfNewConnection failed:', e?.message ?? e), + ); } - }); + }).catch((e) => console.log('config() failed during admin init:', e?.message ?? e)); } public async config(): Promise { @@ -137,10 +141,16 @@ class AdminInterface { await new Promise((r) => setTimeout(r, 100)); } - for (const npub of this.npubs || []) { - await dmUser(sk, npub, `nsecBunker has started; use ${connectionString} to connect to it and unlock your key(s)`, pool); + // try/finally so a throw mid-loop can't leak the pool's reconnect loops + + // sockets for the process lifetime (#48 / review CS-3). dmUser itself is + // now fully guarded, but keep the finally as belt-and-suspenders. + try { + for (const npub of this.npubs || []) { + await dmUser(sk, npub, `nsecBunker has started; use ${connectionString} to connect to it and unlock your key(s)`, pool); + } + } finally { + pool.stop(); } - pool.stop(); } /** diff --git a/src/utils/dm-user.ts b/src/utils/dm-user.ts index db3dc06..34c6add 100644 --- a/src/utils/dm-user.ts +++ b/src/utils/dm-user.ts @@ -12,22 +12,26 @@ export async function dmUser( content: string, pool: RelayPool, ): Promise { - const recipientHex = recipient.startsWith("npub1") - ? (nip19.decode(recipient).data as string) - : recipient; - const ciphertext = nip04.encrypt(sk, recipientHex, content); - const event = finalizeEvent( - { - kind: 4, - created_at: Math.floor(Date.now() / 1000), - tags: [["p", recipientHex]], - content: ciphertext, - }, - sk, - ); + // Guard the whole thing: nip19.decode throws on a malformed npub (passes the + // startsWith check but fails the bech32 checksum), and that previously threw + // *before* the publish try, escaping the caller. Best-effort — never throw. + // (review CS-3) try { + const recipientHex = recipient.startsWith("npub1") + ? (nip19.decode(recipient).data as string) + : recipient; + const ciphertext = nip04.encrypt(sk, recipientHex, content); + const event = finalizeEvent( + { + kind: 4, + created_at: Math.floor(Date.now() / 1000), + tags: [["p", recipientHex]], + content: ciphertext, + }, + sk, + ); await pool.publish(event); } catch (e) { - console.log(e); + console.log('dmUser failed for', recipient, '-', (e as any)?.message ?? e); } }