fix(admin): clear pending callbacks on ACL timeout/response — stop the leak (review AD-1/CS-2) #51
2 changed files with 35 additions and 6 deletions
fix(admin): clear pending callbacks on ACL timeout/response — stop the leak (review AD-1/CS-2)
Some checks failed
Docker image / build-and-push-image (push) Has been cancelled
Some checks failed
Docker image / build-and-push-image (push) Has been cancelled
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
commit
edf1ddc7da
|
|
@ -414,13 +414,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,
|
||||
|
|
@ -436,17 +456,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);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 },
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue