sneak/AutistMask
1
0
Fork 0
Code Issues 50 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix: merge state per field instead of overwriting the whole blob (closes #304)
All checks were successful
check / check (push) Successful in 33s
Details
e2e / e2e-chrome (push) Successful in 1m13s
Details
e2e / e2e-firefox (push) Successful in 24s
Details

Browse Source 
saveState() is now a read-modify-write that merges only the fields this page
changed, diffed against a deep-cloned per-page baseline. wallets, allowedSites,
deniedSites and networkEndpoints merge structurally by identity, so membership
comes from fresh storage except for this page's own adds and deletes.

Fixes a second extension page silently deleting a wallet, the background balance
refresh clobbering a concurrent add or resurrecting a delete, and a stale page
resurrecting a revoked site permission.

Colliding wallet identities keep both records and log rather than silently
dropping an encryptedSecret. Concurrent writers of the same leaf remain
last-writer-wins by design.
This commit was merged in pull request #337.
This commit is contained in:
clawbot
2026-08-20 16:41:19 +02:00
parent 20e911059a
commit cef6aaab11
3 changed files with 987 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files

443
tests/stateMerge.test.js Normal file
View File

@@ -0,0 +1,443 @@
// saveState() used to write the entire state blob every time
// (src/shared/state.js). Every extension page — the toolbar popup, a dApp
// approval window opened by the background, backgroundRefresh() in
// src/background/index.js — holds its own in-memory `state`, loaded once,
// and src/popup/views/helpers.js showView() saves on EVERY navigation. So
// any second page that saved after a first page had written something new
// overwrote it, with no attacker and no unusual input: a whole wallet, name,
// addresses and encrypted secret included, silently gone
// (https://git.eeqj.de/sneak/AutistMask/issues/304).
//
// Both cases below drive the real state.js module through two independent
// module registries sharing one storage backend, the way two real extension
// pages share one chrome.storage.local. The storage stub structured-clones
// on both get and set — a stub that hands back the object it was given
// aliases the caller's own mutation and would make this entire defect class
// invisible (see https://git.eeqj.de/sneak/AutistMask/issues/324).
function makeStorage() {
let store = {};
return {
get: async (keys) => {
const wanted =
keys === undefined || keys === null
? Object.keys(store)
: [].concat(keys);
const out = {};
for (const key of wanted) {
if (key in store) out[key] = structuredClone(store[key]);
}
return out;
},
set: async (items) => {
for (const [key, value] of Object.entries(items)) {
store[key] = structuredClone(value);
}
},
};
}
// One extension page: a fresh module registry over the shared storage.
// state.js resolves the storage API at require time, so the stub has to be
// installed before the module is loaded, and `state` is a module-level
// singleton, so each page needs its own registry to hold its own copy.
function loadPage(storage) {
jest.resetModules();
globalThis.chrome = { storage: { local: storage } };
return {
state: require("../src/shared/state"),
helpers: require("../src/popup/views/helpers"),
};
}
function wallet(name, secret, address) {
return {
type: "hd",
name,
xpub: "xpub-" + name,
encryptedSecret: secret,
nextIndex: 1,
addresses: [{ address, balance: "0", tokenBalances: [] }],
};
}
const W1 = wallet(
"Wallet 1",
"secret-one",
"0x66133E8ea0f5D1d612D2502a968757D1048c214a",
);
const W2 = wallet(
"Wallet 2",
"secret-two",
"0xdAC17F958D2ee523a2206206994597C13D831ec7",
);
// Minimal DOM: showView() toggles view elements, clears the flash line and
// creates/removes the debug banner. Nothing here is asserted; it only has to
// answer without throwing, the way the popup's own index.html would.
function makeElement(id) {
const classes = new Set();
return {
id,
textContent: "",
style: {},
classList: {
add: (...n) => n.forEach((c) => classes.add(c)),
remove: (...n) => n.forEach((c) => classes.delete(c)),
toggle: (c, force) => {
const on = force === undefined ? !classes.has(c) : force;
if (on) classes.add(c);
else classes.delete(c);
return on;
},
},
remove: () => {},
};
}
function makeDocument() {
const els = new Map();
return {
getElementById(id) {
if (id === "debug-banner") return null;
if (!els.has(id)) els.set(id, makeElement(id));
return els.get(id);
},
createElement: () => makeElement("created"),
body: { prepend: () => {} },
};
}
afterEach(() => {
delete globalThis.chrome;
delete globalThis.document;
});
describe("a save from a page that never saw a wallet another page added", () => {
// The first DoD case on the issue: add a wallet in one page, then force
// a save from a second page loaded before that wallet existed. Both
// wallets must survive.
test("both wallets are in storage afterwards", async () => {
const storage = makeStorage();
await storage.set({ autistmask: { wallets: [W1] } });
// Loaded while storage held only Wallet 1, and never reloads —
// the approval window in the reproduction, or a second popup that
// has been open for a while.
const stale = loadPage(storage);
await stale.state.loadState();
expect(stale.state.state.wallets).toHaveLength(1);
// A second page, loaded after, adds a wallet — the exact sequence
// src/popup/views/addWallet.js uses.
const fresh = loadPage(storage);
await fresh.state.loadState();
fresh.state.state.wallets.push(W2);
fresh.state.state.hasWallet = true;
await fresh.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.wallets,
).toHaveLength(2);
// The stale page saves something that has nothing to do with
// wallets — exactly what showView() does on every navigation, and
// what backgroundRefresh() does after a balance poll.
stale.state.state.currentView = "settings";
await stale.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.wallets.map((w) => w.name)).toEqual([
"Wallet 1",
"Wallet 2",
]);
expect(persisted.wallets.map((w) => w.encryptedSecret)).toEqual([
"secret-one",
"secret-two",
]);
});
});
describe("the approval-window reproduction", () => {
// approval window open, add a wallet in the popup, confirm the approval
// — the exact sequence from the issue. The approval window and the
// popup are the same popup code with a different starting view, so
// showView() is the real save path in both: src/popup/views/approval.js
// showTxApproval() calls showView("approve-tx") when the window opens,
// and a successful confirm calls
// src/popup/views/txStatus.js showWait() -> startWait(), which calls
// showView("wait-tx") — the save that clobbered the second wallet in
// the reproduction on the issue.
test("the wallet added in the popup survives confirming the approval", async () => {
globalThis.document = makeDocument();
const storage = makeStorage();
await storage.set({ autistmask: { wallets: [W1] } });
// The background opens the approval window on the approve-tx
// screen; nothing else has happened yet.
const approvalWindow = loadPage(storage);
await approvalWindow.state.loadState();
approvalWindow.helpers.showView("approve-tx");
// showView() does not await its own saveState(); an extra save
// joins the same queue and only resolves once that one has too,
// which is the black-box way to know it landed.
await approvalWindow.state.saveState();
// The user adds a wallet in the popup — a separate page, loaded
// after the approval window.
const popup = loadPage(storage);
await popup.state.loadState();
popup.state.state.wallets.push(W2);
popup.state.state.hasWallet = true;
await popup.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.wallets,
).toHaveLength(2);
// The user confirms the approval. The approval window navigates
// approve-tx -> wait-tx, saving again from state it loaded before
// Wallet 2 ever existed.
approvalWindow.helpers.showView("wait-tx");
await approvalWindow.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.wallets.map((w) => w.name)).toEqual([
"Wallet 1",
"Wallet 2",
]);
expect(persisted.wallets.map((w) => w.encryptedSecret)).toEqual([
"secret-one",
"secret-two",
]);
});
});
// backgroundRefresh() (src/background/index.js) loads state, spends seconds
// on network I/O in refreshBalances() (src/shared/balances.js) mutating
// addr.balance/ensName/tokenBalances IN PLACE on the wallets it already
// knew about, then saves. Precondition 2 on the issue: that refresh window
// overlapping a membership change (add or delete) on another page must not
// clobber or resurrect a wallet — a whole-field diff on `wallets` failed
// this, because "background changed a balance" and "another page changed
// membership" collided as the same field.
describe("background refresh racing a wallet added on another page", () => {
test("the wallet added elsewhere survives background's stale balance save", async () => {
const storage = makeStorage();
await storage.set({ autistmask: { wallets: [W1] } });
// "background": loads first, and its save is the one that lands
// last, modeling the multi-second network round trip in between.
const background = loadPage(storage);
await background.state.loadState();
background.state.state.wallets[0].addresses[0].balance = "1.2345";
// A second page, loaded after, adds a wallet while background's
// refresh is still in flight.
const popup = loadPage(storage);
await popup.state.loadState();
popup.state.state.wallets.push(W2);
popup.state.state.hasWallet = true;
await popup.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.wallets,
).toHaveLength(2);
// background's save lands last, carrying only its balance update.
await background.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.wallets.map((w) => w.name)).toEqual([
"Wallet 1",
"Wallet 2",
]);
expect(persisted.wallets.map((w) => w.encryptedSecret)).toEqual([
"secret-one",
"secret-two",
]);
// The balance update itself must not be lost either — this is a
// merge, not deletion-always-wins.
expect(persisted.wallets[0].addresses[0].balance).toBe("1.2345");
});
});
describe("background refresh racing a wallet deleted on another page", () => {
test("the wallet deleted elsewhere stays deleted after background's stale balance save", async () => {
const storage = makeStorage();
await storage.set({ autistmask: { wallets: [W1, W2] } });
const background = loadPage(storage);
await background.state.loadState();
background.state.state.wallets[0].addresses[0].balance = "1.2345";
// A second page deletes Wallet 2 while background's refresh is in
// flight — the same splice deleteWallet.js's removeWalletFromState()
// does.
const popup = loadPage(storage);
await popup.state.loadState();
popup.state.state.wallets.splice(1, 1);
popup.state.state.hasWallet = popup.state.state.wallets.length > 0;
await popup.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.wallets,
).toHaveLength(1);
await background.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.wallets.map((w) => w.name)).toEqual(["Wallet 1"]);
expect(persisted.wallets[0].addresses[0].balance).toBe("1.2345");
});
});
// allowedSites/deniedSites: { [address]: [hostname, ...] }. Mutated in place
// from two different contexts — src/background/index.js:592-599 pushes a
// newly approved hostname onto state.allowedSites[activeAddress], and the
// Settings "revoke" button (src/popup/views/settings.js:55-68) filters a
// hostname out of state[key][addr] in place, deleting the address key
// entirely once its list is empty — the exact membership-vs-whole-field
// pattern that made the whole-field `wallets` diff unsafe, on a
// security-relevant field: a stale whole-field save here can resurrect a
// revoked permission or wipe a freshly granted one.
const ADDR1 = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const ADDR2 = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
function approveSite(pageState, address, hostname) {
if (!pageState.allowedSites[address]) {
pageState.allowedSites[address] = [];
}
if (!pageState.allowedSites[address].includes(hostname)) {
pageState.allowedSites[address].push(hostname);
}
}
function revokeSite(pageState, hostname) {
for (const addr of Object.keys(pageState.allowedSites)) {
pageState.allowedSites[addr] = pageState.allowedSites[addr].filter(
(h) => h !== hostname,
);
if (pageState.allowedSites[addr].length === 0) {
delete pageState.allowedSites[addr];
}
}
}
describe("a dApp approval racing a stale Settings page's later save", () => {
test("the fresh approval survives Settings revoking an unrelated site", async () => {
const storage = makeStorage();
await storage.set({
autistmask: {
wallets: [W1],
allowedSites: { [ADDR2]: ["other.example"] },
},
});
// Settings loads first, and its save lands last — before either has
// any idea a dApp approval happened elsewhere in between.
const settings = loadPage(storage);
await settings.state.loadState();
// A dApp approval window, opened later, approves a new site for a
// different address and saves — the real sequence at
// src/background/index.js:592-599.
const approval = loadPage(storage);
await approval.state.loadState();
approveSite(approval.state.state, ADDR1, "dapp.example");
await approval.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.allowedSites[ADDR1],
).toEqual(["dapp.example"]);
// Settings revokes its own, unrelated site — the real sequence at
// src/popup/views/settings.js:55-68 — and saves from state loaded
// before the dApp approval ever happened.
revokeSite(settings.state.state, "other.example");
await settings.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.allowedSites[ADDR1]).toEqual(["dapp.example"]);
expect(persisted.allowedSites[ADDR2]).toBeUndefined();
});
});
describe("a revoked site permission against a stale page's later save", () => {
test("the revocation holds even when the stale page approves something else", async () => {
const storage = makeStorage();
await storage.set({
autistmask: {
wallets: [W1],
allowedSites: { [ADDR1]: ["evil.example"] },
},
});
// A stale page loads while the permission still stands.
const stale = loadPage(storage);
await stale.state.loadState();
// Settings revokes it — src/popup/views/settings.js:55-68 — from a
// second page.
const settings = loadPage(storage);
await settings.state.loadState();
revokeSite(settings.state.state, "evil.example");
await settings.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.allowedSites[ADDR1],
).toBeUndefined();
// The stale page, unaware of the revoke, approves an unrelated site
// for a different address and saves — src/background/index.js:592-599.
approveSite(stale.state.state, ADDR2, "good.example");
await stale.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.allowedSites[ADDR2]).toEqual(["good.example"]);
expect(persisted.allowedSites[ADDR1]).toBeUndefined();
});
});
// mergeListByIdentity()'s identity function is not guaranteed collision-free
// — walletIdentity() falls back to one shared "addr:" value for any wallet
// with neither an xpub nor a populated first address (a legacy or corrupt
// record). Two such records created independently on two different pages
// must not silently collapse into one, dropping the loser's
// encryptedSecret with no error and no log.
function legacyWallet(name, secret) {
return {
type: "legacy",
name,
encryptedSecret: secret,
nextIndex: 0,
addresses: [],
};
}
describe("two wallets independently created with a colliding identity", () => {
test("both survive, encryptedSecret included, instead of one silently replacing the other", async () => {
const storage = makeStorage();
await storage.set({ autistmask: { wallets: [W1] } });
// Both pages load before either has created their malformed wallet,
// so neither has baseline knowledge of the other's.
const pageA = loadPage(storage);
await pageA.state.loadState();
const pageB = loadPage(storage);
await pageB.state.loadState();
pageA.state.state.wallets.push(legacyWallet("Legacy A", "secret-a"));
pageA.state.state.hasWallet = true;
await pageA.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.wallets,
).toHaveLength(2);
pageB.state.state.wallets.push(legacyWallet("Legacy B", "secret-b"));
pageB.state.state.hasWallet = true;
await pageB.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
const secrets = persisted.wallets.map((w) => w.encryptedSecret);
expect(secrets).toContain("secret-one");
expect(secrets).toContain("secret-a");
expect(secrets).toContain("secret-b");
});
});