harden: end a site's unremembered connection when its address or wallet is removed #416
@@ -1608,6 +1608,10 @@ view would leave a wallet one click from deletion.
|
||||
- Either way, the active address moves only if it belonged to the deleted
|
||||
wallet, and `AUTISTMASK_ACTIVE_CHANGED` is broadcast when it does
|
||||
(`src/shared/walletDelete.js`)
|
||||
- Either way, every address the wallet held loses its site permissions of
|
||||
both kinds: the remembered ones in storage, and the connections approved
|
||||
without "Remember", which only the background holds, in memory, and drops
|
||||
on `AUTISTMASK_ADDRESSES_REMOVED`
|
||||
- "Confirm Delete" (wrong password) → "That password is incorrect. Please
|
||||
try again." on the error line, nothing deleted
|
||||
- "I have lost my password" → **DeleteWalletLostPassword**
|
||||
@@ -1704,6 +1708,10 @@ view would leave a wallet one click from deletion.
|
||||
so a connected site stops being told about an address the user removed
|
||||
(`src/shared/walletDelete.js`). A selection in any other wallet is left alone;
|
||||
one in this wallet follows the splice.
|
||||
- The address loses its site permissions of both kinds, whether or not it was
|
||||
the active one: the remembered ones in storage, and any connection approved
|
||||
without "Remember", which only the background holds, in memory, and drops on
|
||||
`AUTISTMASK_ADDRESSES_REMOVED`.
|
||||
- The wallet's derivation counter (`nextIndex`) is not rewound, so "+" derives a
|
||||
fresh address rather than handing back the one just removed.
|
||||
|
||||
|
||||
@@ -45,6 +45,17 @@ but the review is broader than any of them.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-04: Removing an address or deleting a wallet ends every site
|
||||
connection approved without "Remember" for the addresses removed
|
||||
([#245](https://git.eeqj.de/sneak/AutistMask/issues/245)). Such a connection
|
||||
lives only in the background's in-memory `connectedSites` map. Both removal
|
||||
paths dropped the remembered `allowedSites`/`deniedSites` entries, but nothing
|
||||
told the background, so its entry was cleared only as a side effect: every
|
||||
change of active address empties the whole map, and removing the active
|
||||
address changes it. `dropSitePermissions()` in `src/shared/walletDelete.js`,
|
||||
shared by both paths, now also sends `AUTISTMASK_ADDRESSES_REMOVED` with the
|
||||
removed addresses, and the background deletes their `connectedSites` entries;
|
||||
only the extension's own pages may send it.
|
||||
- 2026-10-03: The typed-data signing screen warns for a token permission, and
|
||||
names the primary type ethers signs
|
||||
([#400](https://git.eeqj.de/sneak/AutistMask/issues/400)). A Permit or Permit2
|
||||
|
||||
@@ -1305,6 +1305,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
||||
"AUTISTMASK_GET_APPROVAL",
|
||||
"AUTISTMASK_TX_RESPONSE",
|
||||
"AUTISTMASK_SIGN_RESPONSE",
|
||||
"AUTISTMASK_ADDRESSES_REMOVED",
|
||||
];
|
||||
if (POPUP_ONLY_TYPES.includes(msg.type) && !isExtensionSender(sender)) {
|
||||
sendResponse({ error: "Unauthorized sender" });
|
||||
@@ -1647,6 +1648,20 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
||||
return false;
|
||||
}
|
||||
|
||||
// The popup removed these addresses, so no site stays connected to them.
|
||||
// A connectedSites key is origin + ":" + address, and an origin can carry
|
||||
// a port, so the address is what follows the last colon.
|
||||
if (msg.type === "AUTISTMASK_ADDRESSES_REMOVED") {
|
||||
const removed = Array.isArray(msg.addresses) ? msg.addresses : [];
|
||||
for (const key of Object.keys(connectedSites)) {
|
||||
const address = key.slice(key.lastIndexOf(":") + 1);
|
||||
if (removed.some((a) => sameAddress(a, address))) {
|
||||
delete connectedSites[key];
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
if (msg.type === "AUTISTMASK_REMOVE_SITE") {
|
||||
// Popup already saved state; nothing else needed
|
||||
return false;
|
||||
|
||||
@@ -12,12 +12,15 @@ function sameAddress(a, b) {
|
||||
return String(a).toLowerCase() === String(b).toLowerCase();
|
||||
}
|
||||
|
||||
// Forget every site permission held against the given addresses.
|
||||
// Forget every site permission held against the given addresses: the
|
||||
// remembered ones in `state`, and the connections approved without
|
||||
// "Remember", which only the background holds, in memory.
|
||||
function dropSitePermissions(state, addresses) {
|
||||
for (const addr of addresses) {
|
||||
delete state.allowedSites[addr];
|
||||
delete state.deniedSites[addr];
|
||||
}
|
||||
notify({ type: "AUTISTMASK_ADDRESSES_REMOVED", addresses });
|
||||
}
|
||||
|
||||
// Remove wallet `walletIdx` from `state` and repair the derived state.
|
||||
|
||||
@@ -25,6 +25,10 @@ const { Network, Wallet } = require("ethers");
|
||||
// what the user is actually shown.
|
||||
const { describeSigningFailure } = require("../src/shared/approvalVerify");
|
||||
const { makeStorageStub } = require("./support/storageStub");
|
||||
const {
|
||||
removeAddressFromState,
|
||||
removeWalletFromState,
|
||||
} = require("../src/shared/walletDelete");
|
||||
|
||||
const SIGNER_KEY =
|
||||
"0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d";
|
||||
@@ -2096,3 +2100,95 @@ describe("a site connection decided as the popup closes", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// A site connected without "Remember" is held only in the background's memory,
|
||||
// keyed to the address it was connected to. Removing that address, or the
|
||||
// wallet holding it, must end the connection as part of the removal itself.
|
||||
// The accountsChanged broadcast the views send afterwards also clears it, but
|
||||
// only when the active address moved, and nothing waits for it to arrive.
|
||||
//
|
||||
// Each test asks the background while storage still names the removed address
|
||||
// as active, because the popup has not saved yet, so the answer turns on the
|
||||
// connection alone.
|
||||
describe("removing an address ends a site's connection to it", () => {
|
||||
// FRESH_ORIGIN connected to the active address without "Remember", in a
|
||||
// wallet holding a second address so that one can be removed at all. The
|
||||
// popup's messages reach the background as they would from the popup.
|
||||
async function connectedBackground() {
|
||||
const bg = loadBackground({ actionPopup: true });
|
||||
const stored = bg.storage.read("autistmask");
|
||||
stored.wallets[0].addresses.push({
|
||||
address: other.address,
|
||||
balance: "0",
|
||||
tokenBalances: [],
|
||||
});
|
||||
bg.storage.write("autistmask", stored);
|
||||
|
||||
const pending = bg.requestSite();
|
||||
await settle();
|
||||
bg.connectApproval(pending.id()).decide(true, false);
|
||||
await settle();
|
||||
expect(pending.result()).toEqual({ result: [signer.address] });
|
||||
|
||||
global.chrome.runtime.sendMessage = (msg) => {
|
||||
bg.send(msg, bg.fromPopup);
|
||||
};
|
||||
return bg;
|
||||
}
|
||||
|
||||
// What FRESH_ORIGIN is told when it asks which account it may use.
|
||||
async function siteAccounts(bg) {
|
||||
const { sendResponse } = bg.send(
|
||||
{ type: "AUTISTMASK_RPC", method: "eth_accounts", params: [] },
|
||||
{ origin: FRESH_ORIGIN },
|
||||
);
|
||||
await settle();
|
||||
return sendResponse.mock.calls[0][0];
|
||||
}
|
||||
|
||||
test("removing the connected address ends the connection", async () => {
|
||||
const bg = await connectedBackground();
|
||||
expect(await siteAccounts(bg)).toEqual({ result: [signer.address] });
|
||||
|
||||
const popupState = bg.storage.read("autistmask");
|
||||
expect(removeAddressFromState(popupState, 0, 0).removed).toBe(true);
|
||||
|
||||
expect(await siteAccounts(bg)).toEqual({ result: [] });
|
||||
});
|
||||
|
||||
test("deleting the wallet holding the connected address ends the connection", async () => {
|
||||
const bg = await connectedBackground();
|
||||
expect(await siteAccounts(bg)).toEqual({ result: [signer.address] });
|
||||
|
||||
const popupState = bg.storage.read("autistmask");
|
||||
removeWalletFromState(popupState, 0);
|
||||
|
||||
expect(await siteAccounts(bg)).toEqual({ result: [] });
|
||||
});
|
||||
|
||||
test("removing a different address leaves the connection alone", async () => {
|
||||
const bg = await connectedBackground();
|
||||
|
||||
const popupState = bg.storage.read("autistmask");
|
||||
expect(removeAddressFromState(popupState, 0, 1).removed).toBe(true);
|
||||
|
||||
expect(await siteAccounts(bg)).toEqual({ result: [signer.address] });
|
||||
});
|
||||
|
||||
test("a page cannot end the connection", async () => {
|
||||
const bg = await connectedBackground();
|
||||
|
||||
const spoof = bg.send(
|
||||
{
|
||||
type: "AUTISTMASK_ADDRESSES_REMOVED",
|
||||
addresses: [signer.address],
|
||||
},
|
||||
{ url: FRESH_ORIGIN + "/index.html" },
|
||||
);
|
||||
|
||||
expect(spoof.sendResponse).toHaveBeenCalledWith({
|
||||
error: "Unauthorized sender",
|
||||
});
|
||||
expect(await siteAccounts(bg)).toEqual({ result: [signer.address] });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -407,7 +407,9 @@ describe("deleting without the password", () => {
|
||||
expect(saved.activeAddress).toBe(A0);
|
||||
expect(saved.selectedWallet).toBe(0);
|
||||
expect(saved.selectedAddress).toBe(0);
|
||||
expect(sent).toEqual([]);
|
||||
expect(sent).toEqual([
|
||||
{ type: "AUTISTMASK_ADDRESSES_REMOVED", addresses: [B0] },
|
||||
]);
|
||||
// Settings is stubbed, so this is where the route hands over, not
|
||||
// where it renders.
|
||||
expect(mockSettingsShow).toHaveBeenCalled();
|
||||
@@ -426,7 +428,10 @@ describe("deleting without the password", () => {
|
||||
"Wallet 3",
|
||||
]);
|
||||
expect(saved.activeAddress).toBe(B0);
|
||||
expect(sent).toEqual([{ type: "AUTISTMASK_ACTIVE_CHANGED" }]);
|
||||
expect(sent).toEqual([
|
||||
{ type: "AUTISTMASK_ADDRESSES_REMOVED", addresses: [A0, A1] },
|
||||
{ type: "AUTISTMASK_ACTIVE_CHANGED" },
|
||||
]);
|
||||
});
|
||||
|
||||
test("deleting the last wallet lands on Welcome with nothing left", async () => {
|
||||
|
||||
Reference in New Issue
Block a user