Compare commits

..

1 Commits

Author SHA1 Message Date
18ad93be45 harden: make the background physically unable to read the shared state singleton (closes #324)
All checks were successful
check / check (push) Successful in 54s
e2e / e2e-chrome (push) Successful in 2m0s
e2e / e2e-firefox (push) Successful in 54s
Five defects traced to one fact: src/background/index.js read and wrote the
module-level `state` singleton in src/shared/state.js, which the MV3 service
worker never populates and which answered an unpopulated read out of
DEFAULT_STATE in silence. Every previous fix added a loadState() before the
access, and that is what produced the fifth: a load detaches the objects an
in-flight handler is holding.

So the reachability goes rather than a sixth call site.

The background now has its own storage layer, src/background/state.js:
getState() is a detached, normalized per-call read, and updateState() is a
queued read-modify-write whose read is one storage round trip ahead of its
write. Nothing in the background holds an in-memory copy of the profile.

- Every handler takes one snapshot and answers from it, including the address
  it names: activeAddressOf(s) replaced a second, later storage read that
  could disagree with the first.
- wallet_switchEthereumChain applies applyChainSwitchFields() (split out of
  chainSwitch.js, which keeps the singleton path for the popup) inside
  updateState() instead of calling onChainSwitch() on the singleton.
- The remembered site decision is a read-modify-write, not a load-mutate-save
  around a prompt the user takes seconds to answer.
- backgroundRefresh() refreshes a private copy of the wallets and applies the
  balances that came back by address, so it never publishes an object other
  in-flight work holds, and a wallet added or deleted during the round trip
  survives its write.
- The transaction attempt takes its chain id and its endpoint from the same
  snapshot. They used to come from different moments, so a chain switch
  committed in between moved the endpoint under an artifact already verified
  against the old chain.

getProvider(rpcUrl, networkId) now REQUIRES the network id and validates it
against networks.js. That closes the cold-worker wrong-chain send at its shape
rather than at one call site: the hint used to default to currentNetwork() off
the unpopulated singleton, so the endpoint was the user's chain and ethers
fixed chainId at 0x1, and the wallet's own verifySignedTx then refused every
non-mainnet dApp send. refreshBalances(), lookupTokenInfo(), scanForAddresses()
and resolveEnsName() carry the id through; balances.js no longer requires
state.js at all.

The prohibition is enforced mechanically, not by review, and it is enforced by
the bundler rather than by a guess at what the bundler does. The table of
modules an entry point's bundle may not contain lives in
script/lib/forbiddenBundleInputs.js — one copy, read by both layers that act on
it — and build.js's assertNoForbiddenInputs() fails the build when esbuild's
metafile reports src/shared/state.js as an input of a background bundle, naming
the import chain from the metafile's own graph. That is the resolution the
shipped bundle was built from, so no specifier syntax, no hop and no resolution
rule can slip past it; Dockerfile:42 runs make build, so it holds in CI.

Both halves of the table are checked for rot, because either one turns the
prohibition into a pass that checks nothing: a key no bundled entry point
matched fails, and so does a listed module this build bundled nowhere. The
second is what makes a rename of src/shared/state.js loud instead of silently
disarming the check, and it is stronger than an existsSync() because it also
fails when the module is still there but has dropped out of every bundle. What
the assertion does NOT cover is a COPY of the singleton at another path: it is
keyed by path, so a copy builds and lints clean. That is stated where the table
lives, with the reason it is accepted — a copy carries the singleton's own
guard, so a background read of it throws rather than being served DEFAULT_STATE.

make check does not run make build, so the assertion is unit tested against
synthetic metafiles in tests/buildForbiddenInputs.test.js: build.js runs its
build() only as a program now and exports the checks. Executing a check in CI
is not testing it — without that file, inverting the condition leaves every
check in this repo green with the singleton back in the worker.

A custom ESLint rule walks the CommonJS require graph from every src/background/
file and reports the same thing in the editor, before a full bundle. It reads
the same table, and it matches specifiers textually, so it is best-effort fast
feedback and not the guarantee — two earlier revisions of it shipped holes (a
template literal, a dynamic import(), a comment inside the call, a directory
resolved through package.json main). Those are covered now and pinned by
tests/backgroundStateLintRule.test.js. Two shapes it does not report are
recorded in its header as known divergences the build catches, both measured: a
computed specifier (require("../shared/" + "state"), which esbuild
constant-folds into the bundle) and a symlink to the module (esbuild reports the
real path). Each is make lint exit 0 and make build exit 2.

Reading a persisted field of the singleton before any load now throws
StateNotLoadedError instead of serving DEFAULT_STATE.

Test stubs: chrome.storage.local is a serialization boundary, and eight files
stubbed it with an aliasing get, so the object a module held and the object
"storage" held were one object — an assertion could pass on a build that never
wrote anything. Every test that drives real persistence now goes through
tests/support/storageStub.js, which structured-clones in both directions.

closes #320
2026-08-23 15:04:59 +00:00
9 changed files with 429 additions and 63 deletions

18
TODO.md
View File

@@ -67,13 +67,19 @@ but the review is broader than any of them.
the build when esbuild's own metafile reports `src/shared/state.js` as an the build when esbuild's own metafile reports `src/shared/state.js` as an
input of either background bundle — the resolution the shipped bundle was input of either background bundle — the resolution the shipped bundle was
actually built from, so no specifier syntax and no resolution rule can slip actually built from, so no specifier syntax and no resolution rule can slip
past it, and `make build` runs in CI. An ESLint rule that walks the require past it, and `make build` runs in CI. The assertion itself is pinned by
`tests/buildForbiddenInputs.test.js`, including both ways its table can rot: a
key no bundled entry point matched, and a forbidden module this build bundled
nowhere. Its bound is that it is keyed by path, so a COPY of the singleton at
another path is outside it; that is recorded where the table lives
(`script/lib/forbiddenBundleInputs.js`). An ESLint rule that walks the require
graph textually gives the same answer in the editor, before a full bundle; it graph textually gives the same answer in the editor, before a full bundle; it
is fast feedback rather than the guarantee, and the shapes it is known to reads the same table, and it is fast feedback rather than the guarantee, with
catch are pinned by `tests/backgroundStateLintRule.test.js`. Reading an the shapes it is known to catch — and the two it is known to miss — pinned by
unloaded singleton now throws `StateNotLoadedError` instead of serving `tests/backgroundStateLintRule.test.js`. Reading an unloaded singleton now
defaults. The `chrome.storage.local` stubs in eight test files aliased instead throws `StateNotLoadedError` instead of serving defaults. The
of structured-cloning, which could let an assertion pass on a build that never `chrome.storage.local` stubs in eight test files aliased instead of
structured-cloning, which could let an assertion pass on a build that never
wrote anything; every test that drives real persistence now goes through wrote anything; every test that drives real persistence now goes through
`tests/support/storageStub.js`. `tests/support/storageStub.js`.
- 2026-08-23: A swap always names its output token - 2026-08-23: A swap always names its output token

142
build.js
View File

@@ -4,6 +4,7 @@ const crypto = require("crypto");
const { execSync } = require("child_process"); const { execSync } = require("child_process");
const esbuild = require("esbuild"); const esbuild = require("esbuild");
const { resolveVersion } = require("./script/lib/version"); const { resolveVersion } = require("./script/lib/version");
const { FORBIDDEN_INPUTS } = require("./script/lib/forbiddenBundleInputs");
const DIST = path.join(__dirname, "dist"); const DIST = path.join(__dirname, "dist");
const DIST_CHROME = path.join(DIST, "chrome"); const DIST_CHROME = path.join(DIST, "chrome");
@@ -16,25 +17,15 @@ const SRC = path.join(__dirname, "src");
// rotting with it. // rotting with it.
const AUDITED_MODULE = "src/shared/constants.js"; const AUDITED_MODULE = "src/shared/constants.js";
// Modules a given entry point's bundle may not contain, keyed by the // FORBIDDEN_INPUTS — what each entry point's bundle may not contain, and what
// repo-relative entry point. // that covers — lives in script/lib/forbiddenBundleInputs.js, because the
// // ESLint rule reads the same table and two literal copies of a path drift.
// src/shared/state.js is a module-level object loaded once by loadState() and
// mutated in place from then on. That is the popup's model. The MV3 service
// worker has no "once" — it is killed when idle and revived by the next
// message — so a background read of it is answered out of DEFAULT_STATE. Five
// defects came from that, one of which destroyed a wallet
// (https://git.eeqj.de/sneak/AutistMask/issues/324); the background has its own
// per-call storage layer in src/background/state.js instead.
// //
// This is the authoritative check, and it is here rather than in the linter // This is the authoritative check, and it is here rather than in the linter
// because it consults the resolution esbuild actually performed. Any specifier // because it consults the resolution esbuild actually performed. Any specifier
// syntax, any hop, any resolution rule that puts the module in the bundle fails // syntax, any hop, any resolution rule that puts the module in the bundle fails
// the build, whether or not a text matcher would have recognized it. // the build, whether or not a text matcher would have recognized it.
// Dockerfile:42 runs `make build`, so it is enforced in CI. // Dockerfile:42 runs `make build`, so it is enforced in CI.
const FORBIDDEN_INPUTS = {
"src/background/index.js": ["src/shared/state.js"],
};
// The build receipt: every file this build emits, with its sha256 and whether // The build receipt: every file this build emits, with its sha256 and whether
// it is one of the audited bundles. script/verify-build is handed this and // it is one of the audited bundles. script/verify-build is handed this and
@@ -111,20 +102,41 @@ function importChain(metafile, entryInput, target) {
return null; return null;
} }
// Entry points from FORBIDDEN_INPUTS that this build actually bundled. A key // What the forbidden-input checks accumulate over a whole build: which
// that matches nothing means the table has rotted away from the entry point // FORBIDDEN_INPUTS keys were actually bundled, and every input of every output
// list — the prohibition would then be silently unenforced, so the build fails // this build emitted. Both are read by assertForbiddenTableCovered() at the
// on it rather than passing vacuously. // end — a table entry naming something that is not there any more enforces
const forbiddenEntriesSeen = new Set(); // nothing, and must fail rather than pass quietly.
function newForbiddenRecord() {
return { entriesChecked: new Set(), bundledInputs: new Set() };
}
// Note every input of every output of one esbuild run. Deliberately not
// restricted to the entry points named in FORBIDDEN_INPUTS: it is the POPUP
// that legitimately bundles src/shared/state.js, and that is what makes
// "the forbidden module still exists at this path" checkable at all.
function recordBundledInputs(metafile, record) {
for (const info of Object.values(metafile.outputs)) {
for (const input of Object.keys(info.inputs)) {
record.bundledInputs.add(repoRelative(input));
}
}
}
// Fail the build when an entry point's bundle contains a module it is // Fail the build when an entry point's bundle contains a module it is
// prohibited from reaching. The inputs come from esbuild's metafile, so this is // prohibited from reaching. The inputs come from esbuild's metafile, so this is
// the resolution the shipped bundle was built from and not a guess at it. // the resolution the shipped bundle was built from and not a guess at it.
function assertNoForbiddenInputs(entryPoint, outfile, metafile) { function assertNoForbiddenInputs(
entryPoint,
outfile,
metafile,
record,
table = FORBIDDEN_INPUTS,
) {
const entry = repoRelative(entryPoint); const entry = repoRelative(entryPoint);
const forbidden = FORBIDDEN_INPUTS[entry]; const forbidden = table[entry];
if (!forbidden) return; if (!forbidden) return;
forbiddenEntriesSeen.add(entry); record.entriesChecked.add(entry);
const out = repoRelative(outfile); const out = repoRelative(outfile);
const entryOutput = Object.entries(metafile.outputs).find( const entryOutput = Object.entries(metafile.outputs).find(
@@ -150,6 +162,46 @@ function assertNoForbiddenInputs(entryPoint, outfile, metafile) {
} }
} }
// Fail the build when the table has rotted away from the tree it describes.
// Both halves of an entry rot independently, and either one turns the whole
// prohibition into a pass that checks nothing:
//
// - the KEY, when no bundled entry point matches it: the entry point was
// renamed or is no longer built, and no bundle was ever tested against the
// list;
// - the MODULE, when this build bundled it nowhere: the module was renamed,
// moved or deleted, so "is it an input of the background bundle" is asked
// about a path nothing resolves to and is answered no forever. The popup
// legitimately bundles src/shared/state.js, which is what makes this
// checkable — and it is stronger than an existsSync(), because it also
// fails when the file is still there but has dropped out of every bundle.
//
// This matters concretely: https://git.eeqj.de/sneak/AutistMask/issues/311
// rewrites this persistence layer, and a rename that quietly disarmed the
// guarantee would put the singleton back within reach of the worker with every
// check in the repo still green.
function assertForbiddenTableCovered(record, table = FORBIDDEN_INPUTS) {
for (const [entry, modules] of Object.entries(table)) {
if (!record.entriesChecked.has(entry)) {
throw new Error(
`${entry} is listed in FORBIDDEN_INPUTS but was not bundled, ` +
`so nothing checked it`,
);
}
for (const module of modules) {
if (record.bundledInputs.has(module)) continue;
throw new Error(
`${module} is listed in FORBIDDEN_INPUTS for ${entry}, but ` +
`this build bundled it nowhere, so the prohibition names ` +
`a module that is not in this tree at that path and ` +
`nothing enforces it. If the module moved, move it in ` +
`script/lib/forbiddenBundleInputs.js too, which both ` +
`this check and the ESLint rule read.`,
);
}
}
}
// Every file this build writes under dist/, recorded as it is written. This is // Every file this build writes under dist/, recorded as it is written. This is
// the build's own account of what it emitted; it is never recovered by // the build's own account of what it emitted; it is never recovered by
// listing dist/, because a file that is in dist/ without this build having put // listing dist/, because a file that is in dist/ without this build having put
@@ -337,6 +389,9 @@ async function build() {
// esbuild run below and recorded in the receipt for script/verify-build. // esbuild run below and recorded in the receipt for script/verify-build.
const auditedBundles = []; const auditedBundles = [];
// What the forbidden-input checks accumulate across those same runs.
const forbiddenRecord = newForbiddenRecord();
// compile tailwind CSS // compile tailwind CSS
console.log("Compiling Tailwind CSS..."); console.log("Compiling Tailwind CSS...");
const tailwindInput = path.join(SRC, "popup", "styles", "main.css"); const tailwindInput = path.join(SRC, "popup", "styles", "main.css");
@@ -380,7 +435,13 @@ async function build() {
}); });
// Before the output is recorded as emitted: a bundle that violates a // Before the output is recorded as emitted: a bundle that violates a
// prohibition must abort the build, not be written into a receipt. // prohibition must abort the build, not be written into a receipt.
assertNoForbiddenInputs(entryPoint, outfile, result.metafile); recordBundledInputs(result.metafile, forbiddenRecord);
assertNoForbiddenInputs(
entryPoint,
outfile,
result.metafile,
forbiddenRecord,
);
recordEmitted(outfile); recordEmitted(outfile);
auditedBundles.push(...outputsContainingAuditedModule(result.metafile)); auditedBundles.push(...outputsContainingAuditedModule(result.metafile));
} }
@@ -437,14 +498,7 @@ async function build() {
path.join(DIST_FIREFOX, "manifest.json"), path.join(DIST_FIREFOX, "manifest.json"),
); );
for (const entry of Object.keys(FORBIDDEN_INPUTS)) { assertForbiddenTableCovered(forbiddenRecord);
if (!forbiddenEntriesSeen.has(entry)) {
throw new Error(
`${entry} is listed in FORBIDDEN_INPUTS but was not bundled, ` +
`so nothing checked it`,
);
}
}
// Written last so a build that died partway through leaves no receipt at // Written last so a build that died partway through leaves no receipt at
// all, which script/verify-build treats as a hard failure rather than as // all, which script/verify-build treats as a hard failure rather than as
@@ -456,7 +510,27 @@ async function build() {
console.log("Build complete: dist/chrome/ and dist/firefox/"); console.log("Build complete: dist/chrome/ and dist/firefox/");
} }
build().catch((err) => { // Run only as a program. Required as a module — which is how
console.error(`Build failed: ${err && err.message ? err.message : err}`); // tests/buildForbiddenInputs.test.js reaches the checks below — this file
process.exit(1); // builds nothing and writes nothing.
}); if (require.main === module) {
build().catch((err) => {
console.error(
`Build failed: ${err && err.message ? err.message : err}`,
);
process.exit(1);
});
}
// Exported for tests/buildForbiddenInputs.test.js only. The prohibition these
// three functions enforce is the guarantee behind
// https://git.eeqj.de/sneak/AutistMask/issues/324, and `make check` does not
// run `make build` — so they are unit tested against synthetic metafiles
// rather than being exercised only by CI, where "it ran" is not "it works".
module.exports = {
importChain,
newForbiddenRecord,
recordBundledInputs,
assertNoForbiddenInputs,
assertForbiddenTableCovered,
};

View File

@@ -91,6 +91,10 @@ module.exports = [
// the direct require, because a re-export from any shared module the // the direct require, because a re-export from any shared module the
// background already pulls in would put the singleton back in the bundle // background already pulls in would put the singleton back in the bundle
// with no background file naming it. // with no background file naming it.
//
// It is not the guarantee: build.js asserts the same prohibition against
// esbuild's own metafile, from the shared table in
// script/lib/forbiddenBundleInputs.js. This is the early report.
{ {
files: ["src/background/**/*.js"], files: ["src/background/**/*.js"],
plugins: { background: backgroundState }, plugins: { background: backgroundState },

View File

@@ -35,15 +35,31 @@
// string counts — which is the safe direction here: the failure mode is a // string counts — which is the safe direction here: the failure mode is a
// spurious error naming an exact file and line, not a silent hole. // spurious error naming an exact file and line, not a silent hole.
// //
// Deliberately not matched: a computed specifier, `require("../shared/" + // Two shapes this rule does NOT report, both of which the build does fail on
// "state")`. esbuild cannot resolve that statically either, so it never // (each measured with `make lint` and `make build` on the branch that added
// reaches the bundle. // this note):
//
// - a computed specifier, `require("../shared/" + "state")` — esbuild
// constant-folds it, so it is in the bundle and `make build` is exit 2
// naming src/shared/state.js, while `make lint` is exit 0. Same for
// `import("../shared/" + variable)`, which esbuild resolves as a glob.
// - a symlink to the module — esbuild reports the real path and fails the
// build; this rule resolves the link's own path and sees a different file.
//
// They are listed as known divergences, not as things that cannot happen. A
// matcher will keep diverging from a bundler; that is why the guarantee is the
// build's and this rule is not widened again to chase them.
const fs = require("fs"); const fs = require("fs");
const path = require("path"); const path = require("path");
// The module this rule exists to keep out, relative to the repo root. const { FORBIDDEN_INPUTS } = require("../forbiddenBundleInputs");
const FORBIDDEN = path.join("src", "shared", "state.js");
// The modules to keep out, repo-relative, taken from the same table build.js
// asserts against so that the two layers cannot name different paths. A second
// literal copy here is how a rename disarms one of them while the other still
// looks enforced.
const FORBIDDEN = [...new Set(Object.values(FORBIDDEN_INPUTS).flat())];
// Whatever may sit between a keyword, a paren and a specifier: whitespace and // Whatever may sit between a keyword, a paren and a specifier: whitespace and
// comments. `import(/* webpackChunkName: "x" */ "./x")` is a standard bundler // comments. `import(/* webpackChunkName: "x" */ "./x")` is a standard bundler
@@ -119,14 +135,14 @@ function requiresOf(file) {
} }
// Breadth-first from `entry`, returning the shortest chain of files that ends // Breadth-first from `entry`, returning the shortest chain of files that ends
// at the forbidden module, or null when it is not reachable. // at one of the forbidden modules, or null when none is reachable.
function chainToForbidden(entry, forbidden) { function chainToForbidden(entry, forbidden) {
const seen = new Set([entry]); const seen = new Set([entry]);
const queue = [[entry]]; const queue = [[entry]];
while (queue.length > 0) { while (queue.length > 0) {
const chain = queue.shift(); const chain = queue.shift();
for (const next of requiresOf(chain[chain.length - 1])) { for (const next of requiresOf(chain[chain.length - 1])) {
if (next === forbidden) return chain.concat([next]); if (forbidden.has(next)) return chain.concat([next]);
if (seen.has(next)) continue; if (seen.has(next)) continue;
seen.add(next); seen.add(next);
queue.push(chain.concat([next])); queue.push(chain.concat([next]));
@@ -158,8 +174,12 @@ const rule = {
"Program:exit"(node) { "Program:exit"(node) {
const filename = context.filename; const filename = context.filename;
// ESLint lints from the repo root, which is also where the // ESLint lints from the repo root, which is also where the
// forbidden path is anchored. // forbidden paths are anchored.
const forbidden = path.resolve(context.cwd, FORBIDDEN); const forbidden = new Set(
FORBIDDEN.map((module) =>
path.resolve(context.cwd, module),
),
);
const chain = chainToForbidden( const chain = chainToForbidden(
path.resolve(filename), path.resolve(filename),
forbidden, forbidden,

View File

@@ -0,0 +1,54 @@
// The modules a given entry point's bundle may not contain, keyed by the
// repo-relative entry point.
//
// ONE table, read by both layers that act on it: build.js asserts it against
// esbuild's own metafile (the guarantee), and
// script/lib/eslint/noStateSingletonInBackground.js reports the same
// prohibition in the editor (fast feedback). It lives here because a second
// literal copy of the path is exactly how a rename disarms one layer while the
// other still looks enforced.
//
// src/shared/state.js holds a module-level `state` object, loaded once by
// loadState() and mutated in place from then on. That is the popup's model:
// one page, one load at boot, one lifetime. The MV3 service worker has no
// "once" — it is killed when idle and revived by the next message, nothing
// loads state at module scope, and an unpopulated read was answered out of
// DEFAULT_STATE in silence. Five defects came from that, one of which
// destroyed a wallet (https://git.eeqj.de/sneak/AutistMask/issues/324). The
// background has its own per-call storage layer in src/background/state.js
// instead.
//
// What build.js's assertion covers, measured rather than assumed:
//
// - Any import of a listed module, at any hop, in any specifier syntax,
// however esbuild resolved it. The check reads the input list esbuild
// reported for the emitted bundle, so it is the resolution the shipped
// file was built from and not a model of it. Measured on a computed
// specifier that esbuild constant-folds (`require("../shared/" +
// "state")`), on a computed specifier it resolves as a glob
// (`import("../shared/" + variable)`), and on a symlink to the module
// (esbuild reports the real path): each is `make build` exit 2.
//
// - NOT covered: a COPY of a listed module at another path. The table is
// keyed by path, so `cp src/shared/state.js src/shared/stateCopy.js` plus
// a background require of the copy is `make build` exit 0 and `make lint`
// exit 0 (measured). That is deliberate rather than an oversight: a copy
// carries the singleton's own guard, so a background read of an unloaded
// field throws StateNotLoadedError instead of being served DEFAULT_STATE
// — loud, which is the opposite of the failure this table exists to
// prevent. A newly WRITTEN singleton would carry no such backstop, and
// nothing mechanical catches that one.
//
// - The table protects the entry points it names. A second background-side
// entry point added later needs its own line here; the ESLint rule's
// src/background/** glob would cover it, the build assertion would not.
//
// Both halves are checked for rot at the end of a build: a key no bundled
// entry point matched, and a listed module this build bundled nowhere, each
// fail the build rather than passing vacuously
// (assertForbiddenTableCovered(), pinned by tests/buildForbiddenInputs.test.js).
const FORBIDDEN_INPUTS = {
"src/background/index.js": ["src/shared/state.js"],
};
module.exports = { FORBIDDEN_INPUTS };

View File

@@ -10,8 +10,11 @@ const {
const { applyChainSwitchFields } = require("../shared/chainSwitchFields"); const { applyChainSwitchFields } = require("../shared/chainSwitchFields");
// The background's own storage layer. src/shared/state.js — the module-level // The background's own storage layer. src/shared/state.js — the module-level
// `state` singleton, loadState() and saveState() — is deliberately NOT // `state` singleton, loadState() and saveState() — is deliberately NOT
// imported here and must never be: see the header of src/background/state.js, // imported here and must never be: see the header of src/background/state.js.
// and the lint rule that enforces it in eslint.config.js. // The build enforces it, not review: build.js fails when esbuild's metafile
// reports that module as an input of this bundle (FORBIDDEN_INPUTS in
// script/lib/forbiddenBundleInputs.js). The ESLint rule of the same name is
// the same prohibition reported early, not the guarantee.
const { getState, updateState } = require("./state"); const { getState, updateState } = require("./state");
const { refreshBalances, getProvider } = require("../shared/balances"); const { refreshBalances, getProvider } = require("../shared/balances");
const { debugFetch, log } = require("../shared/log"); const { debugFetch, log } = require("../shared/log");

View File

@@ -10,11 +10,17 @@
// nothing loads state at module scope, and an unpopulated read used to hand // nothing loads state at module scope, and an unpopulated read used to hand
// back DEFAULT_STATE with no complaint — five defects came out of that one // back DEFAULT_STATE with no complaint — five defects came out of that one
// fact (https://git.eeqj.de/sneak/AutistMask/issues/324). Two things close it: // fact (https://git.eeqj.de/sneak/AutistMask/issues/324). Two things close it:
// this module is unreachable from the background bundle (enforced by the // this module is unreachable from the background bundle, and reading a
// ESLint rule in eslint.config.js, and by the background having its own // persisted field of the singleton before a load now THROWS instead of quietly
// per-call storage layer in src/background/state.js), and reading a persisted // serving a default.
// field of the singleton before a load now THROWS instead of quietly serving //
// a default. // The unreachability is enforced by the BUILD. build.js fails when esbuild's
// own metafile reports this module as an input of a background bundle — the
// resolution the shipped file was built from, so no specifier syntax gets past
// it — from the table in script/lib/forbiddenBundleInputs.js, which also
// records what that does and does not cover. The ESLint rule that reports the
// same thing in the editor is fast feedback in front of the build, not the
// guarantee.
const { networkById } = require("./networks"); const { networkById } = require("./networks");
const { const {
@@ -58,7 +64,10 @@ let loaded = false;
// The cost of that is honest and worth naming: a context that writes one field // The cost of that is honest and worth naming: a context that writes one field
// and then reads a different, untouched one is still served that field's // and then reads a different, untouched one is still served that field's
// default. Nothing closes that here — what closes it for the background is // default. Nothing closes that here — what closes it for the background is
// that the background cannot reach this module at all (eslint.config.js). // that the background cannot reach this module at all, which build.js asserts
// against esbuild's metafile on every build (FORBIDDEN_INPUTS in
// script/lib/forbiddenBundleInputs.js, pinned by
// tests/buildForbiddenInputs.test.js).
let adopted = false; let adopted = false;
// Every field whose pre-load value would be a plausible-looking default rather // Every field whose pre-load value would be a plausible-looking default rather

View File

@@ -8,9 +8,10 @@
// What this file does NOT do is establish that the singleton cannot reach the // What this file does NOT do is establish that the singleton cannot reach the
// background bundle. That is build.js's FORBIDDEN_INPUTS assertion, which reads // background bundle. That is build.js's FORBIDDEN_INPUTS assertion, which reads
// esbuild's metafile and so cannot be evaded by a syntax a matcher does not // esbuild's metafile and so cannot be evaded by a syntax a matcher does not
// know. The rule under test here is fast local feedback in front of that, and // know; it is pinned by tests/buildForbiddenInputs.test.js. The rule under test
// these cases pin the shapes it is known to catch, so a regression in the // here is fast local feedback in front of that, and these cases pin the shapes
// matcher is a failing test rather than a quietly narrower rule. // it is known to catch, so a regression in the matcher is a failing test rather
// than a quietly narrower rule.
// //
// Every shape below was measured against a real `make build`: each one puts // Every shape below was measured against a real `make build`: each one puts
// state.js in the shipped background bundles, and each one was invisible to // state.js in the shipped background bundles, and each one was invisible to
@@ -18,9 +19,12 @@
// backtick, the dynamic import and the `from` clause; its successor missed a // backtick, the dynamic import and the `from` clause; its successor missed a
// comment inside the call and a directory resolved through package.json `main`. // comment inside the call and a directory resolved through package.json `main`.
// //
// NOT covered, deliberately: a computed specifier such as // NOT covered, and pinned nowhere below because the rule genuinely does not
// `require("../shared/" + "state")`. esbuild cannot resolve that statically // report it: a computed specifier such as `require("../shared/" + "state")`.
// either, so it never reaches the bundle — there is nothing to block. // esbuild constant-folds that and puts state.js in the bundle — measured,
// `make lint` exit 0 and `make build` exit 2 — so it is a known divergence
// that the build catches, not a shape that cannot occur. Same for a symlink to
// the module.
const fs = require("fs"); const fs = require("fs");
const os = require("os"); const os = require("os");

View File

@@ -0,0 +1,192 @@
// build.js's FORBIDDEN_INPUTS assertion — the mechanical guarantee that the
// MV3 background bundle cannot contain src/shared/state.js
// (https://git.eeqj.de/sneak/AutistMask/issues/324).
//
// Why this file exists: `make check` does not run `make build`. CI executes
// the assertion (Dockerfile runs `make build`), but executing is not testing —
// invert its condition, or make the table lookup always come back undefined,
// and every check in this repo stays green while the singleton walks back into
// the worker. Five defects, one destroyed wallet, and the whole argument for
// the scoped loud-read guard rest on this assertion, so it is pinned here.
//
// The subject is build.js's exported helpers, driven against SYNTHETIC
// metafiles in esbuild's shape. Nothing here shells out to a build or writes
// dist/: the assertion's job is to read a metafile correctly, and a metafile is
// data. That the real shapes reach it is the build's own business and is
// measured in the PR that introduced it.
//
// Paths are absolute on the way in, because the helpers normalize whatever
// esbuild gave them to repo-relative and this file should not depend on the
// working directory jest was started from.
const path = require("path");
const {
importChain,
newForbiddenRecord,
recordBundledInputs,
assertNoForbiddenInputs,
assertForbiddenTableCovered,
} = require("../build");
const ROOT = path.resolve(__dirname, "..");
const abs = (p) => path.join(ROOT, p);
const ENTRY = "src/background/index.js";
const OUT = "dist/chrome/src/background/index.js";
const STATE = "src/shared/state.js";
const HOP = "src/shared/chainSwitchFields.js";
// The real table's shape: entry point -> modules its bundle may not contain.
const TABLE = { [ENTRY]: [STATE] };
// A metafile as esbuild emits one: `outputs[out].inputs` is the flat list of
// every input that contributed to that output, and `inputs[file].imports` is
// the edge list, which is what the chain walk follows.
function metafile({ outputs = {}, imports = {} } = {}) {
return {
outputs: Object.fromEntries(
Object.entries(outputs).map(([out, inputs]) => [
abs(out),
{
inputs: Object.fromEntries(
inputs.map((input) => [
abs(input),
{ bytesInOutput: 1 },
]),
),
},
]),
),
inputs: Object.fromEntries(
Object.entries(imports).map(([file, targets]) => [
abs(file),
{ imports: targets.map((target) => ({ path: abs(target) })) },
]),
),
};
}
function check(mf, table = TABLE, record = newForbiddenRecord()) {
recordBundledInputs(mf, record);
assertNoForbiddenInputs(abs(ENTRY), abs(OUT), mf, record, table);
return record;
}
describe("assertNoForbiddenInputs()", () => {
test("a forbidden module in the bundle fails, naming the import chain", () => {
const mf = metafile({
outputs: { [OUT]: [ENTRY, HOP, STATE] },
imports: {
[ENTRY]: [HOP],
[HOP]: [STATE],
},
});
expect(() => check(mf)).toThrow(
`${OUT} bundles ${STATE}, which ${ENTRY} must not reach: ` +
`${ENTRY} -> ${HOP} -> ${STATE}.`,
);
});
test("a bundle without the forbidden module passes, and is recorded as checked", () => {
const mf = metafile({
outputs: { [OUT]: [ENTRY, HOP, "src/background/state.js"] },
imports: { [ENTRY]: [HOP, "src/background/state.js"] },
});
const record = check(mf);
expect([...record.entriesChecked]).toEqual([ENTRY]);
expect(record.bundledInputs.has(STATE)).toBe(false);
});
test("the failure still names the bundle when no import chain can be shown", () => {
// esbuild resolves `import("../shared/" + variable)` as a glob: the
// module is an input of the output, but no single edge leads to it.
// The message must degrade to no chain rather than crash.
const mf = metafile({
outputs: { [OUT]: [ENTRY, STATE] },
imports: { [ENTRY]: [] },
});
expect(() => check(mf)).toThrow(
`${OUT} bundles ${STATE}, which ${ENTRY} must not reach.`,
);
});
});
describe("importChain()", () => {
test("terminates on a cyclic input graph, and still finds the module", () => {
const mf = metafile({
imports: {
[ENTRY]: [HOP],
[HOP]: ["src/shared/log.js"],
// The cycle: log <-> hop, with the target one hop past it.
"src/shared/log.js": [HOP, STATE],
},
});
expect(importChain(mf, abs(ENTRY), STATE)).toEqual([
ENTRY,
HOP,
"src/shared/log.js",
STATE,
]);
});
test("terminates and returns null when a cycle cannot reach the module", () => {
const mf = metafile({
imports: {
[ENTRY]: [HOP],
[HOP]: ["src/shared/log.js"],
"src/shared/log.js": [HOP, ENTRY],
},
});
expect(importChain(mf, abs(ENTRY), STATE)).toBeNull();
});
});
describe("assertForbiddenTableCovered()", () => {
test("the shipped table is satisfied by a build that checked it", () => {
const record = newForbiddenRecord();
record.entriesChecked.add(ENTRY);
// The popup bundle is what legitimately contains the singleton.
record.bundledInputs.add(STATE);
expect(() => assertForbiddenTableCovered(record, TABLE)).not.toThrow();
});
test("a key no bundled entry point matched fails", () => {
const mf = metafile({
outputs: { [OUT]: [ENTRY] },
imports: { [ENTRY]: [] },
});
const stale = { "src/background/renamed.js": [STATE] };
const record = check(mf, stale);
record.bundledInputs.add(STATE);
expect(() => assertForbiddenTableCovered(record, stale)).toThrow(
"src/background/renamed.js is listed in FORBIDDEN_INPUTS but was" +
" not bundled, so nothing checked it",
);
});
test("a forbidden module this build bundled nowhere fails", () => {
// The other half of the same rot: renaming or moving the singleton
// leaves a table that names a path nothing resolves to any more, and
// every bundle then passes it vacuously.
const mf = metafile({
outputs: { [OUT]: [ENTRY] },
imports: { [ENTRY]: [] },
});
const stale = { [ENTRY]: ["src/shared/stateRenamed.js"] };
const record = check(mf, stale);
record.bundledInputs.add(STATE);
expect(() => assertForbiddenTableCovered(record, stale)).toThrow(
"src/shared/stateRenamed.js is listed in FORBIDDEN_INPUTS for" +
` ${ENTRY}, but this build bundled it nowhere`,
);
});
});