Compare commits

..
2 Commits
Author SHA1 Message Date
sneak 30f57b9076 harden: key remembered site permissions by full origin (closes #402)
check / check (push) Failing after 2s
e2e / e2e-chrome (push) Failing after 2s
e2e / e2e-firefox (push) Failing after 2s
allowedSites and deniedSites held the bare hostname, so a grant to
https://dapp.example also authorised http://dapp.example and every port
on that host, and the connection, transaction and signature prompts
named only the hostname. Both lists now store and match the full origin
(scheme://host[:port]), the key the connections approved without
Remember already used. The prompts, the Settings site lists and
AUTISTMASK_REMOVE_SITE use the origin too. Entries saved by hostname
are not migrated (pre-1.0): they match no site.

Model: opus-5-5
2026-10-04 15:02:14 +00:00
clawbot 9f0e88e963 test: load the popup's libraries once per test file, not on every boot (closes #428)
check / check (push) Failing after 2s
e2e / e2e-chrome (push) Failing after 2s
e2e / e2e-firefox (push) Failing after 2s
Every popup boot in the tests resets jest's module registry so that src/
loads fresh, and that also reloaded ethers, libsodium-wrappers-sumo, qrcode
and ethereum-blockies-base64 each time. tests/support/popupBoot.js now loads
those four once per test file and registers them once with jest.doMock(),
which jest.resetModules() keeps, so every boot gets the same copies. No test
or assertion changed. make test takes 8-13s on the shared build host, down
from 17-25s, measured in alternating runs before and after the change.

Model: opus-5-5
2026-10-04 16:59:12 +02:00
10 changed files with 133 additions and 75 deletions
+5 -6
View File
@@ -8,12 +8,11 @@ WORKDIR /app
# image sets it.
ENV AUTISTMASK_LINT_NATIVE=1
# script/test's default 30s bound is the host figure, against a suite that
# takes 23-29s there with three jest workers. In here the same suite starts on
# a cold jest cache and shares the runner with the rest of the build, so 30s
# is too tight — it killed a healthy suite at 30.6s on a cold CI cache. 180s
# still catches a hang in three minutes and cannot be tripped by a suite that
# is merely running on contended hardware.
# script/test's default 30s bound is the host figure. In here the same suite
# starts on a cold jest cache and shares the runner with the rest of the build,
# so 30s is too tight — it killed a healthy suite at 30.6s on a cold CI cache.
# 180s still catches a hang in three minutes and cannot be tripped by a suite
# that is merely running on contended hardware.
ENV AUTISTMASK_TEST_TIMEOUT=180
# script/bootstrap installs all prerequisites (make via apt here; node
+9
View File
@@ -57,6 +57,15 @@ but the review is broader than any of them.
change are not migrated (pre-1.0): they match no site, and Settings lists them
until they are removed.
- 2026-10-04: `make test` takes 8-13s on the shared build host, down from
17-25s, measured in alternating runs before and after the change
([#428](https://git.eeqj.de/sneak/AutistMask/issues/428)). Each popup boot in
the tests (`tests/support/popupBoot.js`) resets jest's module registry so that
everything under `src/` loads fresh, and that also reloaded `ethers`,
`libsodium-wrappers-sumo`, `qrcode` and `ethereum-blockies-base64` every time.
Those four libraries are now loaded once per test file and handed to every
boot. No test or assertion changed.
- 2026-10-04: A token whose scale is unknown reads the same on the Send screen
as on the confirmation screen
([#377](https://git.eeqj.de/sneak/AutistMask/issues/377)). When two addresses'
+6 -6
View File
@@ -5,12 +5,12 @@
# many-core shared host one per core took gigabytes of RAM per run.
#
# The timeout bounds a hung suite; it is not a performance budget. On the busy
# shared build host the suite takes 23-29s with three workers, so
# REPO_POLICIES' 30s cap is tight there, not comfortable. Inside the image the
# same suite also pays a cold jest cache and shares the runner with the rest of
# the build, which is not what that budget describes, so the Dockerfile raises
# the bound through AUTISTMASK_TEST_TIMEOUT. A cap a healthy suite can trip on a
# cold cache produces a red that means nothing, and teaches "just run it again".
# shared build host the suite takes 8-13s with three workers, inside
# REPO_POLICIES' 20s budget. Inside the image the same suite also pays a cold
# jest cache and shares the runner with the rest of the build, which is not what
# that budget describes, so the Dockerfile raises the bound through
# AUTISTMASK_TEST_TIMEOUT. A cap a healthy suite can trip on a cold cache
# produces a red that means nothing, and teaches "just run it again".
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
+11 -5
View File
@@ -140,8 +140,14 @@ function load() {
state.selectedWallet = 0;
state.selectedAddress = 0;
state.activeAddress = A0;
state.allowedSites = { [A0]: ["a.example"], [B0]: ["b.example"] };
state.deniedSites = { [B0]: ["c.example"], [C0]: ["d.example"] };
state.allowedSites = {
[A0]: ["https://a.example"],
[B0]: ["https://b.example"],
};
state.deniedSites = {
[B0]: ["https://c.example"],
[C0]: ["https://d.example"],
};
state.viewStack = ["main", "settings"];
state.currentView = "settings";
@@ -388,8 +394,8 @@ describe("deleting without the password", () => {
await click("btn-delete-wallet-lost-confirm");
const saved = (await storage.get("autistmask")).autistmask;
expect(saved.allowedSites).toEqual({ [A0]: ["a.example"] });
expect(saved.deniedSites).toEqual({ [C0]: ["d.example"] });
expect(saved.allowedSites).toEqual({ [A0]: ["https://a.example"] });
expect(saved.deniedSites).toEqual({ [C0]: ["https://d.example"] });
});
// The route shares finishDelete() with the password route, so the
@@ -437,7 +443,7 @@ describe("deleting without the password", () => {
test("deleting the last wallet lands on Welcome with nothing left", async () => {
const { deleteWallet, state, storage } = load();
state.wallets = [wallet("Wallet 1", "secret-one", [A0])];
state.allowedSites = { [A0]: ["a.example"] };
state.allowedSites = { [A0]: ["https://a.example"] };
state.deniedSites = {};
await openLostPassword(deleteWallet, 0);
+23 -12
View File
@@ -59,24 +59,32 @@ describe("the floor under allowedSites and deniedSites", () => {
}
});
test(`an ${field} entry whose value is not a hostname list is dropped`, () => {
for (const bad of ["dapp.example", 42, null, { a: 1 }, true]) {
test(`an ${field} entry whose value is not an origin list is dropped`, () => {
for (const bad of [
"https://dapp.example",
42,
null,
{ a: 1 },
true,
]) {
expect(
normalizePersisted({ [field]: { [ADDRESS]: bad } })[field],
).toEqual({});
}
});
test(`a hostname that is not text is dropped from an ${field} entry`, () => {
test(`an origin that is not text is dropped from an ${field} entry`, () => {
expect(
normalizePersisted({
[field]: { [ADDRESS]: [42, null, "dapp.example", {}] },
[field]: {
[ADDRESS]: [42, null, "https://dapp.example", {}],
},
})[field],
).toEqual({ [ADDRESS]: ["dapp.example"] });
).toEqual({ [ADDRESS]: ["https://dapp.example"] });
});
test(`a real ${field} map survives, copied not shared`, () => {
const saved = { [field]: { [ADDRESS]: ["dapp.example"] } };
const saved = { [field]: { [ADDRESS]: ["https://dapp.example"] } };
const out = normalizePersisted(saved);
@@ -87,10 +95,13 @@ describe("the floor under allowedSites and deniedSites", () => {
test(`a good ${field} entry beside a malformed one survives`, () => {
const out = normalizePersisted({
[field]: { [ADDRESS]: ["dapp.example"], [TOKEN_ADDRESS]: 42 },
[field]: {
[ADDRESS]: ["https://dapp.example"],
[TOKEN_ADDRESS]: 42,
},
});
expect(out[field]).toEqual({ [ADDRESS]: ["dapp.example"] });
expect(out[field]).toEqual({ [ADDRESS]: ["https://dapp.example"] });
});
test(`a stored own "__proto__" key in ${field} is dropped`, () => {
@@ -100,7 +111,7 @@ describe("the floor under allowedSites and deniedSites", () => {
// saveState()'s merge hands to the prototype setter on the next
// write.
const saved = JSON.parse(
'{"' + field + '":{"__proto__":["evil.invalid"]}}',
'{"' + field + '":{"__proto__":["https://evil.invalid"]}}',
);
const out = normalizePersisted(saved);
@@ -197,7 +208,7 @@ describe("a malformed allowedSites entry", () => {
const MALFORMED = [
{ name: "a string", value: "notalist" },
{ name: "a number", value: 42 },
{ name: "a record", value: { hostnames: ["dapp.example"] } },
{ name: "a record", value: { origins: ["https://dapp.example"] } },
];
for (const { name, value } of MALFORMED) {
@@ -231,7 +242,7 @@ describe("a malformed allowedSites entry", () => {
const env = await bootPopup(
unversionedValidProfile({
allowedSites: {
[ADDRESS]: ["dapp.example"],
[ADDRESS]: ["https://dapp.example"],
[TOKEN_ADDRESS]: "notalist",
},
}),
@@ -239,7 +250,7 @@ describe("a malformed allowedSites entry", () => {
expect(env.pageErrors).toEqual([]);
expect(env.storage.read("autistmask").allowedSites).toEqual({
[ADDRESS]: ["dapp.example"],
[ADDRESS]: ["https://dapp.example"],
});
});
+2 -2
View File
@@ -165,7 +165,7 @@ const CONTRACT = [
[ADDRESS],
{ [ADDRESS]: 42 },
{ [ADDRESS]: [42, null, {}] },
JSON.parse('{"__proto__":["evil.invalid"]}'),
JSON.parse('{"__proto__":["https://evil.invalid"]}'),
],
holds: siteMapHolds,
},
@@ -177,7 +177,7 @@ const CONTRACT = [
[ADDRESS],
{ [ADDRESS]: 42 },
{ [ADDRESS]: [42, null, {}] },
JSON.parse('{"__proto__":["evil.invalid"]}'),
JSON.parse('{"__proto__":["https://evil.invalid"]}'),
],
holds: siteMapHolds,
},
+33 -31
View File
@@ -270,31 +270,32 @@ describe("background refresh racing a wallet deleted on another page", () => {
});
});
// 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.
// allowedSites/deniedSites: { [address]: [origin, ...] }. Mutated in place
// from two different contexts — rememberSiteChoice() in
// src/background/index.js pushes a newly approved origin onto
// state.allowedSites[activeAddress], and the Settings "revoke" button
// (forgetOrigin() in src/popup/views/settings.js) filters an origin 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) {
function approveSite(pageState, address, origin) {
if (!pageState.allowedSites[address]) {
pageState.allowedSites[address] = [];
}
if (!pageState.allowedSites[address].includes(hostname)) {
pageState.allowedSites[address].push(hostname);
if (!pageState.allowedSites[address].includes(origin)) {
pageState.allowedSites[address].push(origin);
}
}
function revokeSite(pageState, hostname) {
function revokeSite(pageState, origin) {
for (const addr of Object.keys(pageState.allowedSites)) {
pageState.allowedSites[addr] = pageState.allowedSites[addr].filter(
(h) => h !== hostname,
(o) => o !== origin,
);
if (pageState.allowedSites[addr].length === 0) {
delete pageState.allowedSites[addr];
@@ -308,7 +309,7 @@ describe("a dApp approval racing a stale Settings page's later save", () => {
await storage.set({
autistmask: {
wallets: [W1],
allowedSites: { [ADDR2]: ["other.example"] },
allowedSites: { [ADDR2]: ["https://other.example"] },
},
});
@@ -318,24 +319,24 @@ describe("a dApp approval racing a stale Settings page's later save", () => {
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.
// different address and saves — the real sequence in
// rememberSiteChoice(), src/background/index.js.
const approval = loadPage(storage);
await approval.state.loadState();
approveSite(approval.state.state, ADDR1, "dapp.example");
approveSite(approval.state.state, ADDR1, "https://dapp.example");
await approval.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.allowedSites[ADDR1],
).toEqual(["dapp.example"]);
).toEqual(["https://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");
// Settings revokes its own, unrelated site — the real sequence in
// forgetOrigin(), src/popup/views/settings.js — and saves from state
// loaded before the dApp approval ever happened.
revokeSite(settings.state.state, "https://other.example");
await settings.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.allowedSites[ADDR1]).toEqual(["dapp.example"]);
expect(persisted.allowedSites[ADDR1]).toEqual(["https://dapp.example"]);
expect(persisted.allowedSites[ADDR2]).toBeUndefined();
});
});
@@ -346,7 +347,7 @@ describe("a revoked site permission against a stale page's later save", () => {
await storage.set({
autistmask: {
wallets: [W1],
allowedSites: { [ADDR1]: ["evil.example"] },
allowedSites: { [ADDR1]: ["https://evil.example"] },
},
});
@@ -354,23 +355,24 @@ describe("a revoked site permission against a stale page's later save", () => {
const stale = loadPage(storage);
await stale.state.loadState();
// Settings revokes it — src/popup/views/settings.js:55-68 — from a
// second page.
// Settings revokes it — forgetOrigin(), src/popup/views/settings.js —
// from a second page.
const settings = loadPage(storage);
await settings.state.loadState();
revokeSite(settings.state.state, "evil.example");
revokeSite(settings.state.state, "https://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");
// for a different address and saves — rememberSiteChoice(),
// src/background/index.js.
approveSite(stale.state.state, ADDR2, "https://good.example");
await stale.state.saveState();
const persisted = (await storage.get("autistmask")).autistmask;
expect(persisted.allowedSites[ADDR2]).toEqual(["good.example"]);
expect(persisted.allowedSites[ADDR2]).toEqual(["https://good.example"]);
expect(persisted.allowedSites[ADDR1]).toBeUndefined();
});
});
+3 -1
View File
@@ -167,7 +167,9 @@ describe("an unversioned profile that is perfectly valid", () => {
expect(stored.wallets[0].encryptedSecret).toBe("encrypted-secret-1");
expect(stored.wallets[0].addresses[0].address).toBe(ADDRESS);
expect(stored.activeAddress).toBe(ADDRESS);
expect(stored.allowedSites).toEqual({ [ADDRESS]: ["dapp.example"] });
expect(stored.allowedSites).toEqual({
[ADDRESS]: ["https://dapp.example"],
});
});
});
+21 -1
View File
@@ -20,6 +20,26 @@ const path = require("path");
const { makeStorageStub } = require("./storageStub");
// The four libraries the popup loads from node_modules, loaded once per test
// file and handed to every boot. jest.resetModules() in bootPopup() empties the
// module cache but keeps what jest.doMock() registered, so these registrations
// hold for every boot in the file and the libraries are not loaded again. None
// of them holds popup state; everything under src/ is still loaded fresh on
// each boot.
//
// A test's own mock of one of them: a jest.doMock() made inside the test
// replaces the registration here, as it would for any module. A top-of-file
// jest.mock() is what the require() below gets, so it is kept, but its factory
// runs once per file and every boot shares the same mock object.
const ethers = require("ethers");
const sodium = require("libsodium-wrappers-sumo");
const QRCode = require("qrcode");
const makeBlockie = require("ethereum-blockies-base64");
jest.doMock("ethers", () => ethers);
jest.doMock("libsodium-wrappers-sumo", () => sodium);
jest.doMock("qrcode", () => QRCode);
jest.doMock("ethereum-blockies-base64", () => makeBlockie);
const POPUP_HTML = fs.readFileSync(
path.join(__dirname, "..", "..", "src", "popup", "index.html"),
"utf8",
@@ -51,7 +71,7 @@ function unversionedValidProfile(extra) {
networkId: "mainnet",
rpcUrl: "https://ethereum-rpc.publicnode.com",
blockscoutUrl: "https://eth.blockscout.com/api/v2",
allowedSites: { [ADDRESS]: ["dapp.example"] },
allowedSites: { [ADDRESS]: ["https://dapp.example"] },
deniedSites: {},
trackedTokens: [],
theme: "system",
+20 -11
View File
@@ -28,11 +28,14 @@ function makeState(overrides = {}) {
selectedAddress: 0,
activeAddress: A0,
allowedSites: {
[A0]: ["a.example"],
[A1]: ["b.example"],
[B0]: ["c.example"],
[A0]: ["https://a.example"],
[A1]: ["https://b.example"],
[B0]: ["https://c.example"],
},
deniedSites: {
[A1]: ["https://d.example"],
[C0]: ["https://e.example"],
},
deniedSites: { [A1]: ["d.example"], [C0]: ["e.example"] },
...overrides,
};
}
@@ -41,7 +44,7 @@ describe("removeWalletFromState", () => {
test("deleting the last wallet clears hasWallet", () => {
const state = makeState({
wallets: [wallet("A", [A0])],
allowedSites: { [A0]: ["a.example"] },
allowedSites: { [A0]: ["https://a.example"] },
deniedSites: {},
});
@@ -109,8 +112,8 @@ describe("removeWalletFromState", () => {
removeWalletFromState(state, 0);
expect(state.allowedSites).toEqual({ [B0]: ["c.example"] });
expect(state.deniedSites).toEqual({ [C0]: ["e.example"] });
expect(state.allowedSites).toEqual({ [B0]: ["https://c.example"] });
expect(state.deniedSites).toEqual({ [C0]: ["https://e.example"] });
});
});
@@ -126,8 +129,14 @@ function makeAddressState(overrides = {}) {
selectedWallet: 0,
selectedAddress: 0,
activeAddress: A0,
allowedSites: { [A0]: ["a.example"], [A1]: ["b.example"] },
deniedSites: { [A1]: ["d.example"], [B0]: ["e.example"] },
allowedSites: {
[A0]: ["https://a.example"],
[A1]: ["https://b.example"],
},
deniedSites: {
[A1]: ["https://d.example"],
[B0]: ["https://e.example"],
},
...overrides,
};
}
@@ -273,8 +282,8 @@ describe("removeAddressFromState", () => {
removeAddressFromState(state, 0, 1);
expect(state.allowedSites).toEqual({ [A0]: ["a.example"] });
expect(state.deniedSites).toEqual({ [B0]: ["e.example"] });
expect(state.allowedSites).toEqual({ [A0]: ["https://a.example"] });
expect(state.deniedSites).toEqual({ [B0]: ["https://e.example"] });
});
// The derivation counter is a high-water mark, never rewound: "+" derives