1 Commits
Author SHA1 Message Date
sneak f87c97ba50 harden: key remembered site permissions by full origin (closes #402)
check / check (push) Failing after 10s
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 14:17:28 +00:00
10 changed files with 75 additions and 133 deletions
+6 -5
View File
@@ -8,11 +8,12 @@ WORKDIR /app
# image sets it.
ENV AUTISTMASK_LINT_NATIVE=1
# 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.
# 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.
ENV AUTISTMASK_TEST_TIMEOUT=180
# script/bootstrap installs all prerequisites (make via apt here; node
-9
View File
@@ -57,15 +57,6 @@ 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 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".
# 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".
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
+5 -11
View File
@@ -140,14 +140,8 @@ function load() {
state.selectedWallet = 0;
state.selectedAddress = 0;
state.activeAddress = A0;
state.allowedSites = {
[A0]: ["https://a.example"],
[B0]: ["https://b.example"],
};
state.deniedSites = {
[B0]: ["https://c.example"],
[C0]: ["https://d.example"],
};
state.allowedSites = { [A0]: ["a.example"], [B0]: ["b.example"] };
state.deniedSites = { [B0]: ["c.example"], [C0]: ["d.example"] };
state.viewStack = ["main", "settings"];
state.currentView = "settings";
@@ -394,8 +388,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]: ["https://a.example"] });
expect(saved.deniedSites).toEqual({ [C0]: ["https://d.example"] });
expect(saved.allowedSites).toEqual({ [A0]: ["a.example"] });
expect(saved.deniedSites).toEqual({ [C0]: ["d.example"] });
});
// The route shares finishDelete() with the password route, so the
@@ -443,7 +437,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]: ["https://a.example"] };
state.allowedSites = { [A0]: ["a.example"] };
state.deniedSites = {};
await openLostPassword(deleteWallet, 0);
+12 -23
View File
@@ -59,32 +59,24 @@ describe("the floor under allowedSites and deniedSites", () => {
}
});
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,
]) {
test(`an ${field} entry whose value is not a hostname list is dropped`, () => {
for (const bad of ["dapp.example", 42, null, { a: 1 }, true]) {
expect(
normalizePersisted({ [field]: { [ADDRESS]: bad } })[field],
).toEqual({});
}
});
test(`an origin that is not text is dropped from an ${field} entry`, () => {
test(`a hostname that is not text is dropped from an ${field} entry`, () => {
expect(
normalizePersisted({
[field]: {
[ADDRESS]: [42, null, "https://dapp.example", {}],
},
[field]: { [ADDRESS]: [42, null, "dapp.example", {}] },
})[field],
).toEqual({ [ADDRESS]: ["https://dapp.example"] });
).toEqual({ [ADDRESS]: ["dapp.example"] });
});
test(`a real ${field} map survives, copied not shared`, () => {
const saved = { [field]: { [ADDRESS]: ["https://dapp.example"] } };
const saved = { [field]: { [ADDRESS]: ["dapp.example"] } };
const out = normalizePersisted(saved);
@@ -95,13 +87,10 @@ describe("the floor under allowedSites and deniedSites", () => {
test(`a good ${field} entry beside a malformed one survives`, () => {
const out = normalizePersisted({
[field]: {
[ADDRESS]: ["https://dapp.example"],
[TOKEN_ADDRESS]: 42,
},
[field]: { [ADDRESS]: ["dapp.example"], [TOKEN_ADDRESS]: 42 },
});
expect(out[field]).toEqual({ [ADDRESS]: ["https://dapp.example"] });
expect(out[field]).toEqual({ [ADDRESS]: ["dapp.example"] });
});
test(`a stored own "__proto__" key in ${field} is dropped`, () => {
@@ -111,7 +100,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__":["https://evil.invalid"]}}',
'{"' + field + '":{"__proto__":["evil.invalid"]}}',
);
const out = normalizePersisted(saved);
@@ -208,7 +197,7 @@ describe("a malformed allowedSites entry", () => {
const MALFORMED = [
{ name: "a string", value: "notalist" },
{ name: "a number", value: 42 },
{ name: "a record", value: { origins: ["https://dapp.example"] } },
{ name: "a record", value: { hostnames: ["dapp.example"] } },
];
for (const { name, value } of MALFORMED) {
@@ -242,7 +231,7 @@ describe("a malformed allowedSites entry", () => {
const env = await bootPopup(
unversionedValidProfile({
allowedSites: {
[ADDRESS]: ["https://dapp.example"],
[ADDRESS]: ["dapp.example"],
[TOKEN_ADDRESS]: "notalist",
},
}),
@@ -250,7 +239,7 @@ describe("a malformed allowedSites entry", () => {
expect(env.pageErrors).toEqual([]);
expect(env.storage.read("autistmask").allowedSites).toEqual({
[ADDRESS]: ["https://dapp.example"],
[ADDRESS]: ["dapp.example"],
});
});
+2 -2
View File
@@ -165,7 +165,7 @@ const CONTRACT = [
[ADDRESS],
{ [ADDRESS]: 42 },
{ [ADDRESS]: [42, null, {}] },
JSON.parse('{"__proto__":["https://evil.invalid"]}'),
JSON.parse('{"__proto__":["evil.invalid"]}'),
],
holds: siteMapHolds,
},
@@ -177,7 +177,7 @@ const CONTRACT = [
[ADDRESS],
{ [ADDRESS]: 42 },
{ [ADDRESS]: [42, null, {}] },
JSON.parse('{"__proto__":["https://evil.invalid"]}'),
JSON.parse('{"__proto__":["evil.invalid"]}'),
],
holds: siteMapHolds,
},
+31 -33
View File
@@ -270,32 +270,31 @@ describe("background refresh racing a wallet deleted on another page", () => {
});
});
// 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.
// 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, origin) {
function approveSite(pageState, address, hostname) {
if (!pageState.allowedSites[address]) {
pageState.allowedSites[address] = [];
}
if (!pageState.allowedSites[address].includes(origin)) {
pageState.allowedSites[address].push(origin);
if (!pageState.allowedSites[address].includes(hostname)) {
pageState.allowedSites[address].push(hostname);
}
}
function revokeSite(pageState, origin) {
function revokeSite(pageState, hostname) {
for (const addr of Object.keys(pageState.allowedSites)) {
pageState.allowedSites[addr] = pageState.allowedSites[addr].filter(
(o) => o !== origin,
(h) => h !== hostname,
);
if (pageState.allowedSites[addr].length === 0) {
delete pageState.allowedSites[addr];
@@ -309,7 +308,7 @@ describe("a dApp approval racing a stale Settings page's later save", () => {
await storage.set({
autistmask: {
wallets: [W1],
allowedSites: { [ADDR2]: ["https://other.example"] },
allowedSites: { [ADDR2]: ["other.example"] },
},
});
@@ -319,24 +318,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 in
// rememberSiteChoice(), src/background/index.js.
// 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, "https://dapp.example");
approveSite(approval.state.state, ADDR1, "dapp.example");
await approval.state.saveState();
expect(
(await storage.get("autistmask")).autistmask.allowedSites[ADDR1],
).toEqual(["https://dapp.example"]);
).toEqual(["dapp.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");
// 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(["https://dapp.example"]);
expect(persisted.allowedSites[ADDR1]).toEqual(["dapp.example"]);
expect(persisted.allowedSites[ADDR2]).toBeUndefined();
});
});
@@ -347,7 +346,7 @@ describe("a revoked site permission against a stale page's later save", () => {
await storage.set({
autistmask: {
wallets: [W1],
allowedSites: { [ADDR1]: ["https://evil.example"] },
allowedSites: { [ADDR1]: ["evil.example"] },
},
});
@@ -355,24 +354,23 @@ describe("a revoked site permission against a stale page's later save", () => {
const stale = loadPage(storage);
await stale.state.loadState();
// Settings revokes it — forgetOrigin(), src/popup/views/settings.js —
// from a second page.
// 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, "https://evil.example");
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 — rememberSiteChoice(),
// src/background/index.js.
approveSite(stale.state.state, ADDR2, "https://good.example");
// 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(["https://good.example"]);
expect(persisted.allowedSites[ADDR2]).toEqual(["good.example"]);
expect(persisted.allowedSites[ADDR1]).toBeUndefined();
});
});
+1 -3
View File
@@ -167,9 +167,7 @@ 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]: ["https://dapp.example"],
});
expect(stored.allowedSites).toEqual({ [ADDRESS]: ["dapp.example"] });
});
});
+1 -21
View File
@@ -20,26 +20,6 @@ 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",
@@ -71,7 +51,7 @@ function unversionedValidProfile(extra) {
networkId: "mainnet",
rpcUrl: "https://ethereum-rpc.publicnode.com",
blockscoutUrl: "https://eth.blockscout.com/api/v2",
allowedSites: { [ADDRESS]: ["https://dapp.example"] },
allowedSites: { [ADDRESS]: ["dapp.example"] },
deniedSites: {},
trackedTokens: [],
theme: "system",
+11 -20
View File
@@ -28,14 +28,11 @@ function makeState(overrides = {}) {
selectedAddress: 0,
activeAddress: A0,
allowedSites: {
[A0]: ["https://a.example"],
[A1]: ["https://b.example"],
[B0]: ["https://c.example"],
},
deniedSites: {
[A1]: ["https://d.example"],
[C0]: ["https://e.example"],
[A0]: ["a.example"],
[A1]: ["b.example"],
[B0]: ["c.example"],
},
deniedSites: { [A1]: ["d.example"], [C0]: ["e.example"] },
...overrides,
};
}
@@ -44,7 +41,7 @@ describe("removeWalletFromState", () => {
test("deleting the last wallet clears hasWallet", () => {
const state = makeState({
wallets: [wallet("A", [A0])],
allowedSites: { [A0]: ["https://a.example"] },
allowedSites: { [A0]: ["a.example"] },
deniedSites: {},
});
@@ -112,8 +109,8 @@ describe("removeWalletFromState", () => {
removeWalletFromState(state, 0);
expect(state.allowedSites).toEqual({ [B0]: ["https://c.example"] });
expect(state.deniedSites).toEqual({ [C0]: ["https://e.example"] });
expect(state.allowedSites).toEqual({ [B0]: ["c.example"] });
expect(state.deniedSites).toEqual({ [C0]: ["e.example"] });
});
});
@@ -129,14 +126,8 @@ function makeAddressState(overrides = {}) {
selectedWallet: 0,
selectedAddress: 0,
activeAddress: A0,
allowedSites: {
[A0]: ["https://a.example"],
[A1]: ["https://b.example"],
},
deniedSites: {
[A1]: ["https://d.example"],
[B0]: ["https://e.example"],
},
allowedSites: { [A0]: ["a.example"], [A1]: ["b.example"] },
deniedSites: { [A1]: ["d.example"], [B0]: ["e.example"] },
...overrides,
};
}
@@ -282,8 +273,8 @@ describe("removeAddressFromState", () => {
removeAddressFromState(state, 0, 1);
expect(state.allowedSites).toEqual({ [A0]: ["https://a.example"] });
expect(state.deniedSites).toEqual({ [B0]: ["https://e.example"] });
expect(state.allowedSites).toEqual({ [A0]: ["a.example"] });
expect(state.deniedSites).toEqual({ [B0]: ["e.example"] });
});
// The derivation counter is a high-water mark, never rewound: "+" derives