harden: end a site's unremembered connection when its address or wallet is removed (closes #245)
A site connected without "Remember" lives only in the background's in-memory connectedSites map. Removing an address or deleting a wallet dropped the remembered permissions but never told the background; the entry went only as a side effect of the accountsChanged broadcast, which empties the whole map when the active address changes. dropSitePermissions(), shared by both removal paths, now sends AUTISTMASK_ADDRESSES_REMOVED with the removed addresses, and the background deletes their entries. Only the extension's own pages may send it. Model: opus-5-5
This commit is contained in:
@@ -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
|
- Either way, the active address moves only if it belonged to the deleted
|
||||||
wallet, and `AUTISTMASK_ACTIVE_CHANGED` is broadcast when it does
|
wallet, and `AUTISTMASK_ACTIVE_CHANGED` is broadcast when it does
|
||||||
(`src/shared/walletDelete.js`)
|
(`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
|
- "Confirm Delete" (wrong password) → "That password is incorrect. Please
|
||||||
try again." on the error line, nothing deleted
|
try again." on the error line, nothing deleted
|
||||||
- "I have lost my password" → **DeleteWalletLostPassword**
|
- "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
|
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;
|
(`src/shared/walletDelete.js`). A selection in any other wallet is left alone;
|
||||||
one in this wallet follows the splice.
|
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
|
- The wallet's derivation counter (`nextIndex`) is not rewound, so "+" derives a
|
||||||
fresh address rather than handing back the one just removed.
|
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
|
# 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
|
- 2026-10-03: The typed-data signing screen warns for a token permission, and
|
||||||
names the primary type ethers signs
|
names the primary type ethers signs
|
||||||
([#400](https://git.eeqj.de/sneak/AutistMask/issues/400)). A Permit or Permit2
|
([#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_GET_APPROVAL",
|
||||||
"AUTISTMASK_TX_RESPONSE",
|
"AUTISTMASK_TX_RESPONSE",
|
||||||
"AUTISTMASK_SIGN_RESPONSE",
|
"AUTISTMASK_SIGN_RESPONSE",
|
||||||
|
"AUTISTMASK_ADDRESSES_REMOVED",
|
||||||
];
|
];
|
||||||
if (POPUP_ONLY_TYPES.includes(msg.type) && !isExtensionSender(sender)) {
|
if (POPUP_ONLY_TYPES.includes(msg.type) && !isExtensionSender(sender)) {
|
||||||
sendResponse({ error: "Unauthorized sender" });
|
sendResponse({ error: "Unauthorized sender" });
|
||||||
@@ -1647,6 +1648,20 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
|||||||
return false;
|
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") {
|
if (msg.type === "AUTISTMASK_REMOVE_SITE") {
|
||||||
// Popup already saved state; nothing else needed
|
// Popup already saved state; nothing else needed
|
||||||
return false;
|
return false;
|
||||||
|
|||||||
@@ -12,12 +12,15 @@ function sameAddress(a, b) {
|
|||||||
return String(a).toLowerCase() === String(b).toLowerCase();
|
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) {
|
function dropSitePermissions(state, addresses) {
|
||||||
for (const addr of addresses) {
|
for (const addr of addresses) {
|
||||||
delete state.allowedSites[addr];
|
delete state.allowedSites[addr];
|
||||||
delete state.deniedSites[addr];
|
delete state.deniedSites[addr];
|
||||||
}
|
}
|
||||||
|
notify({ type: "AUTISTMASK_ADDRESSES_REMOVED", addresses });
|
||||||
}
|
}
|
||||||
|
|
||||||
// Remove wallet `walletIdx` from `state` and repair the derived state.
|
// 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.
|
// what the user is actually shown.
|
||||||
const { describeSigningFailure } = require("../src/shared/approvalVerify");
|
const { describeSigningFailure } = require("../src/shared/approvalVerify");
|
||||||
const { makeStorageStub } = require("./support/storageStub");
|
const { makeStorageStub } = require("./support/storageStub");
|
||||||
|
const {
|
||||||
|
removeAddressFromState,
|
||||||
|
removeWalletFromState,
|
||||||
|
} = require("../src/shared/walletDelete");
|
||||||
|
|
||||||
const SIGNER_KEY =
|
const SIGNER_KEY =
|
||||||
"0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d";
|
"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.activeAddress).toBe(A0);
|
||||||
expect(saved.selectedWallet).toBe(0);
|
expect(saved.selectedWallet).toBe(0);
|
||||||
expect(saved.selectedAddress).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
|
// Settings is stubbed, so this is where the route hands over, not
|
||||||
// where it renders.
|
// where it renders.
|
||||||
expect(mockSettingsShow).toHaveBeenCalled();
|
expect(mockSettingsShow).toHaveBeenCalled();
|
||||||
@@ -426,7 +428,10 @@ describe("deleting without the password", () => {
|
|||||||
"Wallet 3",
|
"Wallet 3",
|
||||||
]);
|
]);
|
||||||
expect(saved.activeAddress).toBe(B0);
|
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 () => {
|
test("deleting the last wallet lands on Welcome with nothing left", async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user