fix(admin): clear pending callbacks on ACL timeout/response — stop the leak (review AD-1/CS-2) #51

Merged

padreug merged 1 commit from fix/admin-pending-leak into dev

2026-07-04 13:32:43 +00:00

padreug commented

2026-06-27 11:09:02 +00:00

Owner

Closes the AD-1/CS-2 (and folds in AD-2) findings from the transport review.

The leak

requestPermission (the interactive admin-approval flow) sends a sendRequest to each admin npub and registers a response callback in transport.pending. Two gaps:

the 10s timeout called resolve(undefined) but never clearPending → every timed-out approval orphaned its pending entry permanently;
a single admin's response cleared only its own id → with N admins, the other N-1 leaked.

Each orphaned entry retains a closure over remotePubkey / keyName / method / the JSON-serialized sign_event payload, so growth tracked signing traffic. It's a memory leak, not a key/signing/data issue, and scoped to the admin-DM auth path (requestPermission only runs when no web baseUrl is configured — the demo uses baseUrl, so it's dormant there).

Fix

Collect every issued request id; one finish() resolves once and clears all of them — on both timeout and the first response.
A settled latch makes late/duplicate responses no-ops. This also fixes AD-2: a late 'always' approval can no longer run allowAllRequestsFromKey (persisting an ACL allow) after the request already timed out.
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 (an evicted entry would time out client-side anyway).

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

Refs: transport review AD-1 / CS-2 / AD-2; #42

🤖 Generated with Claude Code

Closes the **AD-1/CS-2** (and folds in **AD-2**) findings from the transport review. ## The leak `requestPermission` (the interactive admin-approval flow) sends a `sendRequest` to each admin npub and registers a response callback in `transport.pending`. Two gaps: - the **10s timeout** called `resolve(undefined)` but **never `clearPending`** → every timed-out approval orphaned its pending entry permanently; - a single admin's response cleared **only its own** id → with N admins, the other N-1 leaked. Each orphaned entry retains a closure over `remotePubkey` / `keyName` / `method` / the JSON-serialized `sign_event` payload, so growth tracked signing traffic. It's a **memory leak, not a key/signing/data issue**, and scoped to the admin-DM auth path (`requestPermission` only runs when no web `baseUrl` is configured — the demo uses `baseUrl`, so it's dormant there). ## Fix - Collect every issued request id; one `finish()` resolves once and **clears all of them** — on both timeout and the first response. - A `settled` latch makes late/duplicate responses no-ops. This also fixes **AD-2**: a late `'always'` approval can no longer run `allowAllRequestsFromKey` (persisting an ACL allow) after the request already timed out. - 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 (an evicted entry would time out client-side anyway). `tsc` at baseline; daemon bundles; admin + nip46 suites green. Refs: transport review AD-1 / CS-2 / AD-2; #42 🤖 Generated with [Claude Code](https://claude.com/claude-code)

padreug added 1 commit

2026-06-27 11:09:02 +00:00

fix(admin): clear pending callbacks on ACL timeout/response — stop the leak (review AD-1/CS-2)

Docker image / build-and-push-image (push) Has been cancelled

Details

47a5071b4a

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

padreug referenced this pull request

2026-06-27 11:20:09 +00:00

Transport-refactor review — deferred findings (CLI + low/info backlog) #52

padreug force-pushed fix/admin-pending-leak from 47a5071b4a

Docker image / build-and-push-image (push) Has been cancelled

Details

to edf1ddc7da

Docker image / build-and-push-image (push) Has been cancelled

Details

2026-07-04 13:32:34 +00:00

Compare

padreug merged commit 1cf24ff2ef into dev