fix(admin): harden the boot DM — guard teardown + unhandled rejection (review CS-3)
Some checks failed
Docker image / build-and-push-image (push) Has been cancelled

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
This commit is contained in:
Padreug 2026-06-27 12:22:24 +02:00
commit 434817c899
2 changed files with 33 additions and 19 deletions

View file

@ -110,9 +110,13 @@ class AdminInterface {
this.config().then((config) => { this.config().then((config) => {
if (config.admin?.notifyAdminsOnBoot) { 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<IConfig> { public async config(): Promise<IConfig> {
@ -137,11 +141,17 @@ class AdminInterface {
await new Promise((r) => setTimeout(r, 100)); await new Promise((r) => setTimeout(r, 100));
} }
// 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 || []) { 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); await dmUser(sk, npub, `nsecBunker has started; use ${connectionString} to connect to it and unlock your key(s)`, pool);
} }
} finally {
pool.stop(); pool.stop();
} }
}
/** /**
* Get the npub of the admin interface. * Get the npub of the admin interface.

View file

@ -12,6 +12,11 @@ export async function dmUser(
content: string, content: string,
pool: RelayPool, pool: RelayPool,
): Promise<void> { ): Promise<void> {
// 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") const recipientHex = recipient.startsWith("npub1")
? (nip19.decode(recipient).data as string) ? (nip19.decode(recipient).data as string)
: recipient; : recipient;
@ -25,9 +30,8 @@ export async function dmUser(
}, },
sk, sk,
); );
try {
await pool.publish(event); await pool.publish(event);
} catch (e) { } catch (e) {
console.log(e); console.log('dmUser failed for', recipient, '-', (e as any)?.message ?? e);
} }
} }