fix: version the stored profile, and give a record that cannot be read a way out (closes #311)
The stored profile carried no version, so nothing could tell a record this build wrote from one a later build did, and loadState() coerced scalars while trusting the structure. A wallets that was a string, an array of nulls, or a later schema's wallet records reached the popup and threw on the first dereference: no view, no message, no control, and every dApp call answering a generic -32603 because getActiveAddress() dereferenced the same record. There was no reset or wipe control anywhere in the product, so the only escape was clearing extension storage through browser internals.
saveState() and updateState() now both stamp STATE_SCHEMA_VERSION, and every read goes through assertStateUsable() on the raw bytes before normalization can paper over them. Version 1 is the shape that shipped unversioned, so the profile every existing install holds loads normally and is migrated in place by being stamped on the first write; an upgrade shows nobody a wipe prompt for a wallet that is fine. A record this build cannot vouch for is refused instead, and refused all the way: not normalized, not written back, not half-loaded, and not overwritten by a save either.
The gate covers what nothing downstream can floor. Everything else is normalizePersisted()'s job, and three separate gaps there let a gate-accepted record reach a dereference and blank the popup anyway. trackedTokens and activeAddress were floored on truthiness rather than on type: trackedTokens: "nope" rendered nothing with "Cannot read properties of undefined (reading 'toLowerCase')", activeAddress: 42 rendered nothing with "address.slice is not a function". A container check is not enough either, because [1, 2] IS a list and the dereference is t.address.toLowerCase() one level below the Array.isArray(): [1,2], [null], [{}], [{address:42}] and ["0xAA..."] each still rendered nothing. And each address's tokenBalances had no floor at all — which matters most, since refreshBalances() writes that field wholesale and the partial write the issue names as the live cause of a corrupt record lands exactly there — so "x", 42, [null] and [42] rendered nothing too. All of them are type-checked now, container AND entries: an entry that is not a record with a text address is dropped, the well-formed entries beside it survive, and an empty list is still a legitimate value. activeAddress's empty string now floors to null rather than surviving, because init() auto-selects the first address only on a strict null, so a kept "" would leave the popup with no address ever selected; that restores what the || null this check replaced already did.
The header of stateSchema.js claimed every non-gated field's floor was a type check. It is not, and now it says so field by field: rpcUrl, blockscoutUrl, lastBalanceRefresh, fraudContracts, tokenHolderCache, theme, currentView, selectedToken and viewData are saved.x || default; every boolean flag, dustThresholdGwei, selectedWallet and selectedAddress are taken verbatim when present; allowedSites and deniedSites are checked as containers only, never per entry. The header and the README now list which field is in which category rather than asserting a rule the module does not follow.
The popup shows a new StateRecovery screen. It names the problem in a sentence, exports the stored record into a text box on the page with no normalization or repair on it (and downloads it where the browser allows), and offers an erase behind a typed ERASE MY WALLET. Both controls are required: an export with no reset leaves the user stuck, and a reset with no export destroys the only copy of possibly recoverable key material. The Settings gear is hidden while it is up, and showView() is not used to raise it, because both read the state singleton that by then refuses to be read. The export is JSON.stringify of the deserialized record, so what JSON cannot carry is stated where the export is written: a cycle or a BigInt throws and fails the export entirely, and a Date, a Map, a Set, an undefined property or a NaN is mangled silently instead, which is the worse residual because the box then looks complete.
The background refuses the same record and answers dApps -32007 with a message saying the saved data cannot be read and that nothing was signed or sent, rather than the -32603 it also answers when a signing attempt breaks. EIP-1474 sets aside -32000..-32099 for implementation-defined server errors but assigns meanings to -32000 through -32006, including -32001 "Resource not found" and the -32002 "Resource unavailable" this wallet already uses for a pending approval; -32007..-32099 are the unassigned ones, and a test pins the code against that table.
networkById() now throws on an id it does not know instead of quietly answering mainnet, which also stops NETWORKS["constructor"] resolving off the prototype chain. Every key test in the gate is an own-property test, because networkId is an object key into networkEndpoints and an unvalidated "__proto__" set that map's prototype instead of an own key, dropping the user's endpoint silently; normalizePersisted() copies endpoint entries with defineProperty for the same reason. That own-property discipline is the gate's alone — normalizePersisted() reads the same fields plainly, and the two agree only because a record from storage has been through structuredClone and carries Object.prototype.
The three corrupt blobs from the issue drive the real popup entry point and the real worker in tests; each rendered nothing at all and answered -32603 before this, and the unversioned-but-valid case is tested too. Every corrupt-field shape that has ever been observed to blank the popup is a row in tests/stateRecovery.test.js, measured through the same entry point; none has been removed. Three test files used fixture wallets the product cannot produce (a bare address string where an address record belongs, a wallet with no address list) and now use whole records. src/popup/restorableViews.js moved to src/shared/restorableViews.js, since persistedState.js requires it and that module is in the background bundle.
This commit is contained in:
423
tests/stateSchema.test.js
Normal file
423
tests/stateSchema.test.js
Normal file
@@ -0,0 +1,423 @@
|
||||
// The stored-profile version stamp and the shape gate in front of it
|
||||
// (https://git.eeqj.de/sneak/AutistMask/issues/311).
|
||||
//
|
||||
// tests/stateRecovery.test.js covers what the USER sees when a record is
|
||||
// refused. This file covers what is refused and what is not, which is the
|
||||
// half that decides whether an upgrade brick or a false alarm ever happens:
|
||||
// a gate that refuses too much sends a perfectly good wallet to a wipe prompt,
|
||||
// and one that refuses too little is the blank popup again.
|
||||
|
||||
const fs = require("fs");
|
||||
const path = require("path");
|
||||
|
||||
const {
|
||||
STATE_SCHEMA_VERSION,
|
||||
StateUnusableError,
|
||||
assertStateUsable,
|
||||
migrationNeeded,
|
||||
stateProblem,
|
||||
} = require("../src/shared/stateSchema");
|
||||
const {
|
||||
NETWORKS,
|
||||
UnknownNetworkError,
|
||||
isKnownNetworkId,
|
||||
networkById,
|
||||
} = require("../src/shared/networks");
|
||||
const { normalizePersisted } = require("../src/shared/persistedState");
|
||||
const { RESET_PHRASE } = require("../src/popup/views/stateRecovery");
|
||||
const { makeStorageStub } = require("./support/storageStub");
|
||||
|
||||
const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
|
||||
|
||||
function validProfile(extra) {
|
||||
return {
|
||||
hasWallet: true,
|
||||
wallets: [
|
||||
{
|
||||
type: "hd",
|
||||
name: "Wallet 1",
|
||||
xpub: "xpub-wallet-1",
|
||||
encryptedSecret: "encrypted-secret-1",
|
||||
nextIndex: 1,
|
||||
addresses: [
|
||||
{ address: ADDRESS, balance: "1.5", tokenBalances: [] },
|
||||
],
|
||||
},
|
||||
],
|
||||
activeAddress: ADDRESS,
|
||||
networkId: "mainnet",
|
||||
...(extra || {}),
|
||||
};
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
delete global.chrome;
|
||||
});
|
||||
|
||||
describe("what the gate accepts", () => {
|
||||
test("nothing stored at all is a first run, not a defect", () => {
|
||||
expect(stateProblem(undefined)).toBeNull();
|
||||
expect(stateProblem(null)).toBeNull();
|
||||
expect(stateProblem({})).toBeNull();
|
||||
});
|
||||
|
||||
test("a valid profile with no version field is accepted and migrated", () => {
|
||||
const saved = validProfile();
|
||||
|
||||
expect(stateProblem(saved)).toBeNull();
|
||||
expect(migrationNeeded(saved)).toBe(true);
|
||||
// The migration IS the stamp: version 1 is the shape that shipped
|
||||
// unversioned, so nothing about the record has to change.
|
||||
expect(normalizePersisted(saved).schemaVersion).toBe(
|
||||
STATE_SCHEMA_VERSION,
|
||||
);
|
||||
expect(normalizePersisted(saved).wallets).toEqual(saved.wallets);
|
||||
});
|
||||
|
||||
test("a profile already at the current version needs no migration", () => {
|
||||
const saved = validProfile({ schemaVersion: STATE_SCHEMA_VERSION });
|
||||
|
||||
expect(stateProblem(saved)).toBeNull();
|
||||
expect(migrationNeeded(saved)).toBe(false);
|
||||
});
|
||||
|
||||
test("an empty wallet list is fine", () => {
|
||||
expect(stateProblem({ hasWallet: false, wallets: [] })).toBeNull();
|
||||
});
|
||||
|
||||
test("unknown extra fields alone are not a defect", () => {
|
||||
// Only a change in the MEANING of a stored field is a version bump, so
|
||||
// a field this build does not know must not be a refusal on its own —
|
||||
// otherwise a downgrade would wipe a working wallet.
|
||||
expect(stateProblem(validProfile({ somethingNew: 42 }))).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("what the gate refuses", () => {
|
||||
test("a record that is not the record AutistMask stores", () => {
|
||||
expect(stateProblem("wallet")).toMatch(/not the record/);
|
||||
expect(stateProblem([1, 2])).toMatch(/not the record/);
|
||||
});
|
||||
|
||||
test("a version from a newer build, naming both versions", () => {
|
||||
const problem = stateProblem(
|
||||
validProfile({ schemaVersion: STATE_SCHEMA_VERSION + 1 }),
|
||||
);
|
||||
|
||||
expect(problem).toMatch(/newer version/);
|
||||
expect(problem).toContain(String(STATE_SCHEMA_VERSION + 1));
|
||||
expect(problem).toContain(String(STATE_SCHEMA_VERSION));
|
||||
});
|
||||
|
||||
test("a version that is not a version at all", () => {
|
||||
for (const version of ["1", 1.5, 0, -1, null, {}]) {
|
||||
expect(
|
||||
stateProblem(validProfile({ schemaVersion: version })),
|
||||
).toMatch(/schema version/);
|
||||
}
|
||||
});
|
||||
|
||||
test("wallets that is not a list", () => {
|
||||
expect(stateProblem({ wallets: ADDRESS })).toMatch(/not a list/);
|
||||
expect(stateProblem({ wallets: { 0: {} } })).toMatch(/not a list/);
|
||||
});
|
||||
|
||||
test("a wallet that is not a wallet record", () => {
|
||||
expect(stateProblem({ wallets: [null] })).toMatch(/wallet record/);
|
||||
expect(stateProblem({ wallets: [42] })).toMatch(/wallet record/);
|
||||
});
|
||||
|
||||
test("a wallet whose addresses are missing or not records", () => {
|
||||
expect(stateProblem({ wallets: [{ name: "Wallet 1" }] })).toMatch(
|
||||
/no list of addresses/,
|
||||
);
|
||||
expect(stateProblem({ wallets: [{ addresses: [ADDRESS] }] })).toMatch(
|
||||
/not a record/,
|
||||
);
|
||||
expect(stateProblem({ wallets: [{ addresses: [{}] }] })).toMatch(
|
||||
/no address/,
|
||||
);
|
||||
});
|
||||
|
||||
test("the problem names WHICH wallet, counting from one", () => {
|
||||
const problem = stateProblem({
|
||||
wallets: [validProfile().wallets[0], { name: "Wallet 2" }],
|
||||
});
|
||||
|
||||
expect(problem).toContain("Wallet 2");
|
||||
});
|
||||
|
||||
test("assertStateUsable throws the sentence, not a generic error", () => {
|
||||
let thrown = null;
|
||||
try {
|
||||
assertStateUsable({ wallets: ADDRESS });
|
||||
} catch (e) {
|
||||
thrown = e;
|
||||
}
|
||||
|
||||
expect(thrown).toBeInstanceOf(StateUnusableError);
|
||||
expect(thrown.problem).toMatch(/not a list/);
|
||||
expect(thrown.message).toBe(thrown.problem);
|
||||
});
|
||||
});
|
||||
|
||||
describe("networkId, which is used as an object key", () => {
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/311#issuecomment-67478:
|
||||
// state.networkId keys state.networkEndpoints, so a corrupt "__proto__"
|
||||
// sets that map's prototype instead of an own key and the user's endpoint
|
||||
// is silently not recorded.
|
||||
test("a network this build does not know is refused", () => {
|
||||
expect(stateProblem(validProfile({ networkId: "base" }))).toMatch(
|
||||
/does not know/,
|
||||
);
|
||||
});
|
||||
|
||||
test('"__proto__" and "constructor" are refused, not resolved', () => {
|
||||
for (const id of ["__proto__", "constructor", "toString"]) {
|
||||
expect(stateProblem(validProfile({ networkId: id }))).toMatch(
|
||||
/does not know/,
|
||||
);
|
||||
expect(isKnownNetworkId(id)).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test("the gate reads own properties only", () => {
|
||||
// A record whose PROTOTYPE carries the fields must not be read as
|
||||
// though it carried them itself: that is how a polluted prototype
|
||||
// would decide whether a profile is refused.
|
||||
const inherited = Object.create({
|
||||
schemaVersion: STATE_SCHEMA_VERSION + 99,
|
||||
networkId: "base",
|
||||
wallets: "not a list",
|
||||
});
|
||||
|
||||
expect(stateProblem(inherited)).toBeNull();
|
||||
});
|
||||
|
||||
test("normalizing never turns a stored key into a prototype", () => {
|
||||
// JSON can carry an own "__proto__" key, and plain assignment would
|
||||
// treat it as the prototype setter rather than storing an entry.
|
||||
const saved = JSON.parse(
|
||||
'{"networkId":"mainnet","networkEndpoints":' +
|
||||
'{"__proto__":{"rpcUrl":"https://evil.invalid"}}}',
|
||||
);
|
||||
|
||||
const out = normalizePersisted(saved);
|
||||
|
||||
expect(Object.prototype.hasOwnProperty.call(out, "rpcUrl")).toBe(true);
|
||||
expect(out.rpcUrl).not.toBe("https://evil.invalid");
|
||||
expect(Object.getPrototypeOf(out.networkEndpoints)).toBe(
|
||||
Object.prototype,
|
||||
);
|
||||
expect({}.rpcUrl).toBeUndefined();
|
||||
});
|
||||
|
||||
test("an unknown stored networkId never reaches the endpoint map", () => {
|
||||
// The floor under the gate: normalization alone must not adopt it.
|
||||
const out = normalizePersisted({ networkId: "__proto__" });
|
||||
|
||||
expect(out.networkId).toBe("mainnet");
|
||||
expect(Object.keys(out.networkEndpoints)).toEqual(["mainnet"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the floors under the gate, for fields the gate does not check", () => {
|
||||
// The gate's scope is what nothing can floor. Everything it lets through
|
||||
// is normalizePersisted()'s to make safe, and a floor written as
|
||||
// `saved.x || default` is not one: a truthy value of the wrong type walks
|
||||
// through it and throws on the first dereference, which produced the blank
|
||||
// popup from the issue. Nor is a container check on its own: [1, 2] is a
|
||||
// list, and the dereference is `t.address.toLowerCase()` one level below
|
||||
// it. Container AND entries, therefore — an empty list still survives.
|
||||
const TOKEN = "0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
|
||||
|
||||
test("trackedTokens that is not a list becomes an empty list", () => {
|
||||
for (const bad of ["nope", 42, true, { a: 1 }]) {
|
||||
expect(
|
||||
normalizePersisted({ trackedTokens: bad }).trackedTokens,
|
||||
).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
test("a trackedTokens entry that is not a token record is dropped", () => {
|
||||
for (const bad of [1, null, {}, { address: 42 }, TOKEN, [], true]) {
|
||||
expect(
|
||||
normalizePersisted({ trackedTokens: [bad] }).trackedTokens,
|
||||
).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
test("a real trackedTokens list survives, copied not shared", () => {
|
||||
const saved = { trackedTokens: [{ address: ADDRESS, symbol: "AM" }] };
|
||||
|
||||
const out = normalizePersisted(saved);
|
||||
|
||||
expect(out.trackedTokens).toEqual(saved.trackedTokens);
|
||||
expect(out.trackedTokens).not.toBe(saved.trackedTokens);
|
||||
expect(normalizePersisted({ trackedTokens: [] }).trackedTokens).toEqual(
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test("a good trackedTokens entry beside a malformed one survives", () => {
|
||||
const good = { address: TOKEN, symbol: "AM", decimals: 18 };
|
||||
|
||||
expect(
|
||||
normalizePersisted({ trackedTokens: [1, null, good, {}] })
|
||||
.trackedTokens,
|
||||
).toEqual([good]);
|
||||
});
|
||||
|
||||
// Below an address record, which the gate walks but does not descend into.
|
||||
// refreshBalances() writes tokenBalances WHOLESALE, so a write that only
|
||||
// partly lands leaves exactly this field malformed.
|
||||
function walletWith(tokenBalances) {
|
||||
return {
|
||||
wallets: [
|
||||
{
|
||||
name: "Wallet 1",
|
||||
addresses: [{ address: ADDRESS, tokenBalances }],
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
function balancesOf(out) {
|
||||
return out.wallets[0].addresses[0].tokenBalances;
|
||||
}
|
||||
|
||||
test("an address's tokenBalances that is not a list becomes an empty list", () => {
|
||||
for (const bad of ["x", 42, true, { a: 1 }, undefined]) {
|
||||
expect(balancesOf(normalizePersisted(walletWith(bad)))).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
test("a tokenBalances entry that is not a token record is dropped", () => {
|
||||
for (const bad of [null, 42, "x", {}, { address: 42 }]) {
|
||||
expect(balancesOf(normalizePersisted(walletWith([bad])))).toEqual(
|
||||
[],
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("a real tokenBalances entry survives, copied not shared", () => {
|
||||
const held = { address: TOKEN, symbol: "AM", balance: "2.0" };
|
||||
const saved = walletWith([held]);
|
||||
|
||||
const out = normalizePersisted(saved);
|
||||
|
||||
expect(balancesOf(out)).toEqual([held]);
|
||||
expect(balancesOf(out)[0]).not.toBe(held);
|
||||
});
|
||||
|
||||
test("activeAddress that is not text becomes null", () => {
|
||||
for (const bad of [42, true, { a: 1 }, [ADDRESS]]) {
|
||||
expect(
|
||||
normalizePersisted({ activeAddress: bad }).activeAddress,
|
||||
).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
test("a real activeAddress survives; the empty string becomes null", () => {
|
||||
expect(
|
||||
normalizePersisted({ activeAddress: ADDRESS }).activeAddress,
|
||||
).toBe(ADDRESS);
|
||||
// "" is text but it is not an address, and src/popup/index.js
|
||||
// auto-selects the first address only on a STRICT null — so keeping
|
||||
// the empty string would leave the popup with none ever selected.
|
||||
expect(
|
||||
normalizePersisted({ activeAddress: "" }).activeAddress,
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("networkById on an unknown id", () => {
|
||||
test("throws instead of quietly answering mainnet", () => {
|
||||
expect(() => networkById("base")).toThrow(UnknownNetworkError);
|
||||
expect(() => networkById(undefined)).toThrow(UnknownNetworkError);
|
||||
// Both of these used to answer with something truthy off the
|
||||
// prototype chain rather than with a network.
|
||||
expect(() => networkById("constructor")).toThrow(UnknownNetworkError);
|
||||
expect(() => networkById("__proto__")).toThrow(UnknownNetworkError);
|
||||
});
|
||||
|
||||
test("still answers every network it does know", () => {
|
||||
for (const id of Object.keys(NETWORKS)) {
|
||||
expect(networkById(id).id).toBe(id);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("the version stamp on the way out", () => {
|
||||
function loadStateModule(persisted) {
|
||||
jest.resetModules();
|
||||
const storage = makeStorageStub(
|
||||
persisted ? { autistmask: persisted } : {},
|
||||
);
|
||||
global.chrome = { storage };
|
||||
return { storage, mod: require("../src/shared/state") };
|
||||
}
|
||||
|
||||
test("the popup stamps it on a profile that had none", async () => {
|
||||
const { storage, mod } = loadStateModule(validProfile());
|
||||
|
||||
await mod.loadState();
|
||||
await mod.saveState();
|
||||
|
||||
expect(storage.read("autistmask").schemaVersion).toBe(
|
||||
STATE_SCHEMA_VERSION,
|
||||
);
|
||||
});
|
||||
|
||||
test("the background stamps it on a profile that had none", async () => {
|
||||
jest.resetModules();
|
||||
const storage = makeStorageStub({ autistmask: validProfile() });
|
||||
global.chrome = { storage };
|
||||
const { updateState } = require("../src/background/state");
|
||||
|
||||
await updateState((s) => {
|
||||
s.lastBalanceRefresh = 1;
|
||||
});
|
||||
|
||||
expect(storage.read("autistmask").schemaVersion).toBe(
|
||||
STATE_SCHEMA_VERSION,
|
||||
);
|
||||
});
|
||||
|
||||
test("a save refuses to write over a record it cannot read", async () => {
|
||||
// Another context — a newer build, or something else entirely — wrote
|
||||
// a record this one does not understand while this page was open. That
|
||||
// record is the only copy of whatever it holds, and normalizing it
|
||||
// back into storage would destroy it.
|
||||
const { storage, mod } = loadStateModule(validProfile());
|
||||
await mod.loadState();
|
||||
|
||||
const hostile = { schemaVersion: STATE_SCHEMA_VERSION + 1 };
|
||||
storage.write("autistmask", hostile);
|
||||
|
||||
// By name rather than by constructor: jest.resetModules() above gives
|
||||
// the module under test its own copy of the error class, so instanceof
|
||||
// across that boundary would be comparing two identical classes.
|
||||
const thrown = await mod.saveState().then(
|
||||
() => null,
|
||||
(e) => e,
|
||||
);
|
||||
expect(thrown && thrown.name).toBe("StateUnusableError");
|
||||
expect(thrown.problem).toMatch(/newer version/);
|
||||
expect(storage.read("autistmask")).toEqual(hostile);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the typed confirmation phrase", () => {
|
||||
test("the markup asks for the phrase the code checks", () => {
|
||||
// The button is behind a phrase typed by hand. A screen that asks for
|
||||
// one phrase while the code compares another is an exit that cannot be
|
||||
// taken, on the one screen that exists to be an exit.
|
||||
const html = fs.readFileSync(
|
||||
path.join(__dirname, "..", "src", "popup", "index.html"),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
expect(html).toContain(RESET_PHRASE);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user