Compare commits

..

1 Commits

Author SHA1 Message Date
6a01688106 fix: floor the persisted fields a restore dereferences, and make each field's floor an executable claim (closes #362)
All checks were successful
check / check (push) Successful in 40s
e2e / e2e-chrome (push) Successful in 1m45s
e2e / e2e-firefox (push) Successful in 32s
A persisted container was checked while its ENTRIES were dereferenced
unchecked. A stored `{"0x…": "notalist"}` in allowedSites passes the state
gate, renders a working popup, and then throws inside saveState()'s per-
hostname merge, so every save from that moment on fails while the UI looks
entirely healthy. deniedSites has the identical shape; fraudContracts is the
same class with a milder consequence.

The sweep for that class found four more:

- selectedToken, dereferenced as text behind a truthiness-only restore gate.
- rpcUrl, handed whole to `new JsonRpcProvider()` by getProvider(), which
  throws SYNCHRONOUSLY for a non-string — from txStatus.js and addWallet.js,
  neither inside a try, and the first reachable from a stored
  `currentView: "wait-tx"` through the unguarded restoreView().
- The ENTRIES of viewData. Four restore branches gate on one truthy field and
  hand the rest to a renderer that calls address.toLowerCase(): a stored
  `{"currentView":"success-tx","viewData":{"hash":"0x1"}}` throws out of
  restoreView(), skipping the rest of popup init.
- selectedWallet / selectedAddress. `wallets` is a real Array, so a stored
  "map", "length", "constructor" or "__proto__" is TRUTHY: hasValidAddress()'s
  `&&` does not short-circuit and `.addresses[…]` throws. A stale INTEGER index
  is the safe case.

Floors, in src/shared/persistedState.js: allowedSites/deniedSites through
siteMap(), fraudContracts and each hostname list through textList(),
selectedToken and activeAddress as text-or-null, rpcUrl and blockscoutUrl as
non-empty text, selectedWallet and selectedAddress as a non-negative integer
or null, and each networkEndpoints pair's two URL fields — which
applyChainSwitchFields() assigns straight onto s.rpcUrl on the next switch.

Guards, in src/popup/viewRouter.js: the four restore branches that gate on one
truthy field now check the entries their renderer dereferences, as
txStatus.restoreWait() has always done for wait-tx. "confirm-tx" joins
ADDRESS_VIEWS, because its Sign button dereferences
state.wallets[state.selectedWallet] behind no guard of its own.

A stored own "__proto__" key is dropped by siteMap(): it can never be a wallet
address, so it grants nothing, and keeping it only keeps a value the next save
would hand to the prototype setter. networkEndpoints keeps unknown keys by
design, so mergeMapByKey() in src/shared/state.js now writes with
defineProperty as well — the guard in the floor was being undone one layer
downstream.

A save that fails is also told, not merely repaired: onSaveFailure() reports
every failed save, awaited or not (the save queue's own rejection handler is
what made a failure vanish), and the popup raises a persistent "NOT SAVED"
banner naming the reason. doRefreshAndRender() no longer rejects, since every
one of its call sites fires it and walks away.

The per-field justification in the header of src/shared/stateSchema.js is
replaced by tests/persistedFieldContract.test.js. That comment shipped a false
claim in three consecutive changes; the artifact was the problem. The test is
one row per persisted field, declaring the property that field's floor is
claimed to have and PROVING it by driving the real code with hostile values —
the gate for a field the gate refuses, normalizePersisted() for a field it
floors, the real JsonRpcProvider constructor for rpcUrl, and — for every field
whose only defence is that nothing dereferences it structurally — a boot of the
real popup entry point over that value onto EVERY view the popup can reopen
onto.

That last part is what makes the claim falsifiable, and it is why this defect
class is worth a harness at all: it lives on the RESTORE path and not on Home.
So the suite goes red on a field any restorable view dereferences on render, on
a field that gains a floor while its row still claims it has none, and on a
field added to PERSISTED_FIELDS with no row. What it does not reach is what no
stored record reaches by itself: a view only forward navigation opens, and
anything behind a click. The header and the README mirror now point at it
instead of restating it.

The boots are cheap enough to keep by construction rather than by sampling.
Every field the router itself reads is driven onto each view individually,
since a hostile value in one of those legitimately changes which view renders;
every other unfloored field is corrupted on the SAME boot, and that boot has to
land on the view it stored — so a field that does move the routing cannot hide
in the crowd, and the failure path re-boots one field at a time to name it.
That is thirty-three boots instead of six hundred; the suite runs in about 13s
against a 30s cap.

The DOM stub in tests/support/popupBoot.js gained one thing to make any of that
possible: an element's parentElement. Without it success-tx and transaction
threw on the first line that hides a field's wrapper, so neither renderer could
be booted onto at all — every boot aimed at them fell back to Home instead, and
the base profile the sweep starts from is now asserted to render each view
rather than fall back, so that cannot go unnoticed again.
2026-08-23 19:37:25 +00:00
4 changed files with 51 additions and 211 deletions

View File

@@ -1052,20 +1052,14 @@ driving the real code with hostile values — and, for every field whose only
defence is that nothing dereferences it, by booting the real popup entry point
over that value onto every view the popup can reopen onto. That last part is
what makes the claim falsifiable, because this defect class lives on the restore
path rather than on the home screen. Read the claim narrowly, as that file
states it: what those boots prove is no structural dereference on the code paths
a WHOLLY-CORRUPTED PROFILE takes, which is not every path a stored record takes.
Not driven: any pairing of values the four slots do not produce, a view only
forward navigation opens, anything behind a click, and everything a healthy
profile reaches. Within that boundary the verdict is unconditional — if one of
those boots leaves the popup unhealthy or off the view it stored, `make check`
fails, including when it takes two corrupted fields at once, because the verdict
is the combined boot and the per-field re-boot that names a culprit can only
decorate the message. So does a field that gains a floor while its row still
claims it has none, and so does a field added to `PERSISTED_FIELDS` with no row
at all. The per-field justification that used to live in the header of
`src/shared/stateSchema.js` shipped a false claim in three consecutive changes,
each caught only by a reviewer re-deriving thirty fields by hand.
path rather than on the home screen: a field that gains a structural dereference
in any restorable view's render fails `make check`, as does a field that gains a
floor while its row still claims it has none, and as does a field added to
`PERSISTED_FIELDS` with no row at all. What the boot does not reach is what no
stored record reaches by itself — a view only forward navigation opens, and
anything behind a click. The per-field justification that used to live in the
header of `src/shared/stateSchema.js` shipped a false claim in three consecutive
changes, each caught only by a reviewer re-deriving thirty fields by hand.
The `allowedSites` case is why the entry check is not optional. A stored
`{"0x…": "notalist"}` is a well-formed object holding a malformed entry: it

15
TODO.md
View File

@@ -82,18 +82,9 @@ but the review is broader than any of them.
proven by driving the real code with hostile values — and, for a field whose
only defence is that nothing dereferences it, by booting the real popup entry
point over that value onto every view the popup can reopen onto, since that is
the path this whole class of defect lives on. Each such field is driven at
both polarities — a value nothing writes is wrong-typed and so truthy, so a
falsy slot is driven too, or the field is proven unable to be falsy after the
floor. The claim is narrow and stated as such: no structural dereference on
the code paths a wholly-corrupted profile takes, which is not every path a
stored record takes — a pairing of values the four slots do not produce, a
view only forward navigation opens, anything behind a click, and everything a
healthy profile reaches are all undriven. Within that boundary the verdict is
unconditional, including a dereference that takes two corrupted fields at
once, since the assertion is on the combined boot and the per-field re-boot
can only decorate the message. A field with no row and a field that gains a
floor while its row still claims it has none also fail `make check`.
the path this whole class of defect lives on. A field with no row, a field
that gains a floor while its row still claims it has none, and a field any
restorable view dereferences on render now all fail `make check`.
- 2026-08-23: A swap amount and the token it is counted in now always come from
the same hop, on both sides of the approval screen
([#359](https://git.eeqj.de/sneak/AutistMask/issues/359) and

View File

@@ -38,16 +38,12 @@
// real popup entry point onto EVERY view the popup can reopen onto.
//
// That last part is the whole point, because this defect class lives on the
// RESTORE path and not on Home. Take the claim NARROWLY, exactly as that file
// states it: what those boots prove is no structural dereference on the code
// paths a WHOLLY-CORRUPTED PROFILE takes — which is not every path a stored
// record takes. Not driven: any pairing of values the four slots do not
// produce, a view only forward navigation opens, anything behind a click, and
// everything a healthy profile reaches. Within that boundary the verdict is
// unconditional, including a dereference that takes two corrupted fields at
// once. That suite also goes red on a field that gains a floor while its row
// still claims it has none, and on a field added to PERSISTED_FIELDS with no
// row at all.
// RESTORE path and not on Home: a field that gains a structural dereference
// in any restorable view's render turns that suite red, as does a field that
// gains a floor while its row still claims it has none. What the boot does not
// reach is what no stored record reaches by itself — a view only forward
// navigation opens, and anything behind a click. A field added to
// PERSISTED_FIELDS with no row fails the suite too.
//
// That test exists because this comment did not work. It carried a
// hand-written justification per field, and it shipped a false one in three

View File

@@ -36,40 +36,11 @@
// before trusting it. A row the ROUTER reads (`routes`) gets its own boot per
// view, because a hostile value in it legitimately changes which view renders.
// Every other swept field is corrupted on the SAME boot, one boot per view per
// slot, and that boot has to land on the view it stored — so a field that does
// move the routing cannot hide in the crowd. Every swept field is driven at
// BOTH POLARITIES: a value nothing in src/ writes is a wrong-typed one and so
// always truthy, which leaves `if (!state.x) { state.y.deref() }` unentered on
// the very boot that corrupts x. The last slot is the falsy one for that
// reason, and a field that cannot be falsy after the floor says so in its row
// and is proven so.
//
// READ THE CLAIM NARROWLY. What this file proves is: NO STRUCTURAL
// DEREFERENCE ON THE CODE PATHS A WHOLLY-CORRUPTED PROFILE TAKES. That is not
// every path a stored record takes, and the difference is the whole of what
// this file does not cover:
//
// - Only the values in the table, in the SLOT arrangement below: four value
// combinations per view, not the product of twelve fields. A dereference
// reached only under a pairing no slot produces is not driven at all.
// - Only what a stored record reaches by ITSELF. A view only forward
// navigation opens, and anything behind a click, is not driven.
// - Nothing about the paths a HEALTHY profile takes, which is most of the
// popup. This file is a floor under one defect class, not a proof about
// the renderers.
//
// Within that boundary it is unconditional: if one of these boots leaves the
// popup unhealthy or off the view it stored, this file goes red — including
// when it takes two corrupted fields at once, because the verdict is the
// combined boot itself and the per-field re-boot below can only decorate the
// message. That last part is the one thing an earlier version got wrong: it
// asserted on the per-field list, so an observed dead popup that no single
// field reproduced was reported green.
//
// Booting every field separately at every value would be several hundred boots
// and most of the suite's budget; this is forty-four. Widening it further is
// out of scope — proving no field is dereferenced on any reachable render path
// is exhaustive verification of the popup, not a floor under a stored record.
// hostile slot, and that boot has to land on the view it stored — so a field
// that does move the routing cannot hide in the crowd, and a field that is
// dereferenced by any renderer reachable from a stored record turns this file
// red. Booting each of them separately would be about six hundred boots and
// half a minute; this is thirty-three.
//
// The three claims this replaced, all false, all caught here by construction:
// rpcUrl reaching `new JsonRpcProvider()` (a synchronous throw, not a caught
@@ -112,14 +83,6 @@ const everyEntry = (v, fn) => Array.isArray(v) && v.every(fn);
// floor and the sweep is the regression guard on it.
const swept = (row) => row.kind === KIND.LOOSE || Boolean(row.alsoSweep);
// Every value a swept row drives through a boot: the hostile set, plus the
// falsy slot that gives the field its other polarity. `hostile` values are all
// TRUTHY by nature — a value nothing in src/ writes is a wrong-typed one, and
// wrong-typed values are objects, non-empty strings and non-zero numbers. A
// field that is only ever truthy on the boot that corrupts it cannot falsify
// `if (!state.x) { state.y.deref() }`, so the falsy slot is not optional.
const sweptValues = (row) => [...row.hostile, ...(row.falsy || [])];
// ------------------------------------------------------------------ the table
//
// `hostile` is values a stored record can carry that nothing in src/ ever
@@ -129,15 +92,6 @@ const sweptValues = (row) => [...row.hostile, ...(row.falsy || [])];
// floor alone, which is pure and free. `hostileRestore` is extra values driven
// through the restore path only, for a value that means nothing until a
// particular branch's gate has let it past.
//
// `falsy` is the other POLARITY of a swept field, driven for the same reason.
// It is not a value src/ never writes — for three of these fields it is the
// DEFAULT_STATE default, which is the branch every ordinary install takes —
// and that is the point: without it, a dereference behind `if (!state.x)` is
// unreachable on the one boot that corrupts x. A swept row that cannot supply
// one says `neverFalsy` instead, which is proven rather than asserted: every
// falsy value stored under that field comes back TRUTHY from the floor, so no
// `!state.x` branch is reachable from a stored record at all.
const CONTRACT = [
{
@@ -312,9 +266,6 @@ const CONTRACT = [
// coerces. Nothing renders FROM it without RESTORABLE_VIEWS.has()
// first, and Set.has() answers false for any value.
hostile: [42, "no-such-view", { a: 1 }],
// `saved.currentView || null`: the falsy polarity is the popup landing
// on Home, which every boot in "booting onto Home" below also drives.
falsy: [""],
},
{
field: "viewData",
@@ -327,10 +278,6 @@ const CONTRACT = [
// renderer something it dereferences, which is where the entries are
// actually decided.
hostile: [42, "notarecord", { a: 1 }, [1, 2]],
// `structuredClone(saved.viewData || {})`: the container is never falsy
// in state whatever was stored, so no `!state.viewData` branch exists to
// drive.
neverFalsy: true,
hostileRestore: [
// success-tx passes on `data.hash`, and renderSuccess() then calls
// toAddressHtml(d.to) -> addressTitle() -> address.toLowerCase().
@@ -410,10 +357,6 @@ const CONTRACT = [
// Arithmetic only: `now - (s.lastBalanceRefresh || 0)` compares false
// for a non-number and forces a refresh.
hostile: [true, "notatime", { a: 1 }],
// `|| 0` collapses every falsy stored value to 0, so 0 IS the whole
// falsy polarity of this field — and it is the DEFAULT_STATE default,
// the value a profile carries until its first refresh lands.
falsy: [0],
},
{
field: "tokenHolderCache",
@@ -423,8 +366,6 @@ const CONTRACT = [
// src/shared/state.js, which are safe for any value, and otherwise
// only reset wholesale in src/shared/chainSwitchFields.js.
hostile: [42, "notarecord", [1, 2]],
// `structuredClone(saved.tokenHolderCache || {})`.
neverFalsy: true,
},
{
field: "theme",
@@ -432,16 +373,11 @@ const CONTRACT = [
// Compared against "dark"/"light" in applyTheme() and otherwise falls
// to the system branch; assigned into an input .value, which coerces.
hostile: [42, "chartreuse", { a: 1 }],
// `saved.theme || "system"`.
neverFalsy: true,
},
{
field: "dustThresholdGwei",
kind: KIND.LOOSE,
hostile: ["notanumber", true, { a: 1 }],
// Survives verbatim, so the falsy slot is also wrong-typed: "" reaches
// filterTransactions() as a comparand and a settings input .value.
falsy: [""],
},
...[
"rememberSiteChoice",
@@ -457,10 +393,6 @@ const CONTRACT = [
kind: KIND.LOOSE,
// A flag: only ever tested for truthiness, and written back verbatim.
hostile: [42, "notabool", { a: 1 }],
// Both answers to that truthiness test have to be driven, and 0 is a
// value src/ never writes for a flag. For utcTimestamps and debugMode
// the falsy answer is also the DEFAULT_STATE default.
falsy: [0],
})),
];
@@ -502,10 +434,6 @@ function profileWith(field, value) {
}
describe("the floor each row claims", () => {
// The falsy slot is deliberately NOT in here. `saved.x || default` is a
// floor on falsy values and on nothing else, so a falsy value is the one
// thing a LOOSE field need not carry through verbatim; what it has to carry
// through is being falsy, which "both polarities" below asserts.
for (const row of CONTRACT) {
const values = [...row.hostile, ...(row.floorOnly || [])];
@@ -578,50 +506,9 @@ const HEALTHY = { errors: [], blank: false };
// unversionedValidProfile() stores no currentView, so every boot in here lands
// on Home. That is the cheap half of the proof; the restore path below is the
// half that matters.
// Both polarities of every swept field are driven, or the field is proven
// unable to take one of them. This is the guard on the sweep itself: a hostile
// set is all-truthy by construction, so without a falsy slot a dereference
// behind `if (!state.x)` is never reached on the boot that corrupts x — the
// same falsy-collapse blind spot the fields below were floored for.
describe("both polarities of every swept field are driven", () => {
const FALSY_STORED = [0, "", false, null];
const floored = (field, value) =>
normalizePersisted(profileWith(field, value))[field];
for (const row of CONTRACT) {
if (!swept(row)) continue;
if (row.neverFalsy) {
test(`${row.field}: cannot be falsy in state at all`, () => {
for (const value of FALSY_STORED) {
expect({
stored: value,
truthy: Boolean(floored(row.field, value)),
}).toEqual({ stored: value, truthy: true });
}
});
continue;
}
test(`${row.field}: truthy and falsy`, () => {
// What the boots below actually drive, floored the way a renderer
// sees it — not what the row says it drives.
const driven = [
...sweptValues(row),
...(row.hostileRestore || []).map((entry) => entry.value),
].map((value) => floored(row.field, value));
expect({
truthy: driven.some((value) => Boolean(value)),
falsy: driven.some((value) => !value),
}).toEqual({ truthy: true, falsy: true });
});
}
});
describe("a hostile value for one field, booting onto Home", () => {
for (const row of CONTRACT) {
for (const value of sweptValues(row)) {
for (const value of row.hostile) {
test(`${row.field} = ${JSON.stringify(value)}`, async () => {
await expect(
bootHealth(profileWith(row.field, value)),
@@ -735,7 +622,7 @@ describe("a hostile routing value restoring onto", () => {
for (const row of CONTRACT) {
if (!swept(row) || !routes(row)) continue;
const seen = new Set();
for (const value of sweptValues(row)) {
for (const value of row.hostile) {
for (const view of RESTORABLE_VIEWS) {
const profile = restoringOnto(view, { [row.field]: value });
const key = JSON.stringify(profile);
@@ -758,33 +645,18 @@ describe("a hostile routing value restoring onto", () => {
// routing turns this red and has to declare `routes` and take the individual
// sweep above.
//
// Combining hides one thing, and the last slot is what stops it. A hostile
// value is wrong-typed and therefore TRUTHY, so on a boot where every swept
// field is hostile, no `if (!state.x)` branch is entered — and a dereference
// inside such a branch would go unseen however loudly it throws. The last slot
// is the falsy one: every swept field that CAN be falsy is falsy on it, which
// is also the state an ordinary install boots in for three of them, while the
// fields that cannot be falsy stay hostile-truthy. That makes it a MIX, and a
// deliberate one — the interaction between a falsy flag and a still-hostile
// theme is a shape a stored record really produces.
//
// The verdict is the combined boot, always. When it goes red the same view is
// re-booted one field at a time, so the failure NAMES a culprit instead of
// leaving a reader to bisect twelve fields — but that loop only decorates the
// message. It cannot clear the failure. A dereference that needs two corrupted
// fields at once is reproduced by neither field alone, and a version of this
// file that asserted on the named list reported exactly that case green while
// watching the popup die.
// Nothing is masked by combining: a throw fails the boot whichever field threw,
// and the only other way a dereference could go unseen is the renderer not
// running at all, which is exactly what `restored` forbids. When it does go
// red, the same view is re-booted one field at a time so the failure names the
// fields rather than leaving a reader to bisect twelve of them.
const UNROUTED = CONTRACT.filter((row) => swept(row) && !routes(row));
const HOSTILE_SLOTS = Math.max(
...UNROUTED.map((row) => sweptValues(row).length),
);
const HOSTILE_SLOTS = Math.max(...UNROUTED.map((row) => row.hostile.length));
function unroutedValues(slot) {
const fields = {};
for (const row of UNROUTED) {
const values = sweptValues(row);
fields[row.field] = values[slot % values.length];
fields[row.field] = row.hostile[slot % row.hostile.length];
}
return fields;
}
@@ -800,42 +672,29 @@ describe("every field the router does not read, corrupted at once, onto", () =>
restoringOnto(view, fields),
view,
);
// The per-field re-boot only DECORATES the message. The
// verdict is `together`, unconditionally: a dereference that
// needs two corrupted fields at once is reproduced by NEITHER
// field alone, so an assertion on the named list would report
// an observed dead popup as green.
const named = [];
if (together.errors.length > 0 || !together.restored) {
for (const row of UNROUTED) {
const one = await restoredHealth(
restoringOnto(view, {
[row.field]: fields[row.field],
}),
view,
);
if (one.errors.length === 0 && one.restored) continue;
named.push(
`${row.field}=${JSON.stringify(
fields[row.field],
)}: ` +
(one.errors.join("; ") || `fell off ${view}`),
);
}
if (named.length === 0) {
named.push(
"no single field reproduces it; it takes two or " +
`more of ${JSON.stringify(fields)}`,
);
}
if (together.errors.length === 0 && together.restored) {
expect(together).toEqual(RESTORED);
return;
}
expect({
const named = [];
for (const row of UNROUTED) {
const one = await restoredHealth(
restoringOnto(view, {
[row.field]: fields[row.field],
}),
view,
);
if (one.errors.length === 0 && one.restored) continue;
named.push(
`${row.field}=${JSON.stringify(fields[row.field])}: ` +
(one.errors.join("; ") || `fell off ${view}`),
);
}
expect({ view: view, fields: named }).toEqual({
view: view,
together: together,
fields: named,
}).toEqual({ view: view, together: RESTORED, fields: [] });
fields: [],
});
});
}
}