harden: connectedSites is never cleared for a removed address or wallet #245

New Issue

clawbot · 2026-08-12T10:25:15+02:00

clawbot commented

2026-08-12 10:25:15 +02:00

The background keeps an in-memory connectedSites map keyed origin + ":" + address. Nothing clears an entry when the address it names stops existing.

removeWalletFromState() has never cleared it, and the address-level removal added by #240 does not either — so this is pre-existing and consistent, not a regression introduced there. Found by the independent review of that PR and filed rather than dropped.

Both removal paths already drop the persisted allowedSites / deniedSites for the address. The in-memory map is the one piece of connection state that survives, so for the life of the service worker a dApp can still be treated as connected to an address the user deleted.

Implementation requirements

Clear connectedSites entries for the removed address on both the wallet-level and address-level removal paths, alongside the existing dropSitePermissions call.
Check what connectedSites actually gates before choosing the fix — if a stale entry only affects an accountsChanged emission, the consequence is different from it gating an approval, and the PR body should say which it is.
Confirm whether the map is rebuilt from storage on service-worker restart. If it is, the exposure window is bounded by the worker lifetime and that bound should be stated; if it is not, say so.

Definition of done

Removing an address clears its connectedSites entries; removing a wallet clears them for every address it held.
A test drives a removal with a live connected site and asserts the entry is gone.
The PR body states what connectedSites gates and the exposure window before the fix.
TODO.md updated in the same commit.
make check passes.

The background keeps an in-memory `connectedSites` map keyed `origin + ":" + address`. Nothing clears an entry when the address it names stops existing. `removeWalletFromState()` has never cleared it, and the address-level removal added by https://git.eeqj.de/sneak/AutistMask/pulls/240 does not either — so this is pre-existing and consistent, not a regression introduced there. Found by the independent review of that PR and filed rather than dropped. Both removal paths already drop the persisted `allowedSites` / `deniedSites` for the address. The in-memory map is the one piece of connection state that survives, so for the life of the service worker a dApp can still be treated as connected to an address the user deleted. ## Implementation requirements - Clear `connectedSites` entries for the removed address on both the wallet-level and address-level removal paths, alongside the existing `dropSitePermissions` call. - Check what `connectedSites` actually gates before choosing the fix — if a stale entry only affects an `accountsChanged` emission, the consequence is different from it gating an approval, and the PR body should say which it is. - Confirm whether the map is rebuilt from storage on service-worker restart. If it is, the exposure window is bounded by the worker lifetime and that bound should be stated; if it is not, say so. ## Definition of done - [ ] Removing an address clears its `connectedSites` entries; removing a wallet clears them for every address it held. - [ ] A test drives a removal with a live connected site and asserts the entry is gone. - [ ] The PR body states what `connectedSites` gates and the exposure window before the fix. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.

clawbot referenced this issue

2026-08-12 10:56:22 +02:00

feat: remove an address from an HD wallet, behind a confirmation (closes #162) #240

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#245