Compare commits

...

2 Commits

Author SHA1 Message Date
b9ae64d396 fix: correct verify-build diagnostics and close two robustness gaps (closes #180)
Some checks failed
check / check (push) Has been cancelled
The both-markers diagnostic claimed the debug branch was still live. It is
not: with the __BUILD_DEBUG__ define removed, the emitted bundle carries
`typeof __BUILD_DEBUG__<"u"?__BUILD_DEBUG__:!1`, and in extension context the
identifier is undeclared, so DEBUG evaluates to false at runtime. The message
now states what the check does prove -- DEBUG was not resolved at build time,
so the release/debug distinction is no longer enforced and which way the
unresolved fallback evaluates is an accident a refactor can flip -- and it
remains a hard failure. The other seven failure messages were reviewed and
none needed rewording.

has_marker no longer swallows grep's exit 2 with 2>/dev/null. Match and
no-match are answers about the emitted output; an unreadable file is not, and
is now reported as a permissions or I/O fault instead of as "the emitted
output changed shape". Both paths still fail hard.

The unlisted-bundle scan no longer filters by extension, so the endsWith(".js")
test in build.js is the only place that assumption lives. A bundle emitted
under another extension previously escaped the manifest and the cross-check at
once; it now fails as unlisted. Both sites carry a comment naming the other.

Also: the manifest must be readable and a listed bundle must be non-empty,
so a vacuous input fails loudly rather than reaching a marker check that
cannot prove anything.
2026-08-11 12:23:59 +00:00
b882cede9f fix: repair wallet state on delete (closes #156)
Some checks failed
check / check (push) Has been cancelled
2026-08-11 14:23:06 +02:00
7 changed files with 271 additions and 32 deletions

View File

@@ -980,7 +980,7 @@ Currently supported:
### Wallet Management ### Wallet Management
- [ ] Delete wallet (with confirmation) - [x] Delete wallet (with confirmation)
- [ ] Delete address from HD wallet (with confirmation) - [ ] Delete address from HD wallet (with confirmation)
- [ ] Show wallet's recovery phrase (requires password) - [ ] Show wallet's recovery phrase (requires password)

View File

@@ -44,6 +44,15 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 2026-08-11: `script/verify-build` diagnostics corrected: the both-markers
message now states what is and is not proven, an unreadable bundle is
diagnosed as an I/O fault rather than as changed output, and the `*.js`
assumption lives only in `build.js`
([#180](https://git.eeqj.de/sneak/AutistMask/issues/180)).
- 2026-08-11: Wallet deletion repairs its own state — `hasWallet` follows the
remaining wallets, the selection only moves when it was deleted, and the
active-address change is broadcast to connected sites
([#156](https://git.eeqj.de/sneak/AutistMask/issues/156)).
- 2026-08-11: `TODO.md` Workflow rewritten to the branch-and-PR-per-issue model - 2026-08-11: `TODO.md` Workflow rewritten to the branch-and-PR-per-issue model
on `next`, with Status and Next Step refreshed on `next`, with Status and Next Step refreshed
([#191](https://git.eeqj.de/sneak/AutistMask/issues/191)). ([#191](https://git.eeqj.de/sneak/AutistMask/issues/191)).

View File

@@ -29,6 +29,11 @@ function repoRelative(p) {
// reports every input that contributed to an output in the metafile, which is // reports every input that contributed to an output in the metafile, which is
// the authoritative answer to "is constants.js in this bundle" — unlike // the authoritative answer to "is constants.js in this bundle" — unlike
// searching the minified text, it does not depend on what survived minification. // searching the minified text, it does not depend on what survived minification.
//
// The ".js" filter below is the only place that assumption lives:
// script/verify-build searches every file under dist/ for a marker, without
// filtering by extension, so a bundle emitted under some other extension fails
// there as unlisted rather than escaping both checks at once.
function outputsContainingAuditedModule(metafile) { function outputsContainingAuditedModule(metafile) {
return Object.entries(metafile.outputs) return Object.entries(metafile.outputs)
.filter(([outFile, info]) => { .filter(([outFile, info]) => {

View File

@@ -34,16 +34,31 @@ fail() {
exit 1 exit 1
} }
# Is the literal $1 present in the file $2? Match (grep exit 0) and no-match
# (exit 1) are answers about the emitted output. Anything else (exit 2: the
# file could not be read) is not an answer at all, and must not be reported as
# "no marker" — that would blame the bundle for a permissions or I/O fault.
has_marker() { has_marker() {
grep -q -F "$1" "$2" 2>/dev/null _hm_status=0
grep -q -F -e "$1" -- "$2" || _hm_status=$?
case "$_hm_status" in
0) return 0 ;;
1) return 1 ;;
*)
fail "grep exited $_hm_status reading $2, so the file could not be
searched and its DEBUG state was not checked at all. That is a permissions
or I/O fault on the artifact, not a change in the emitted output. Refusing
to report success."
;;
esac
} }
# Read one bundle's DEBUG state into MARKER. Exactly one marker must be # Read one bundle's DEBUG state into MARKER. Exactly one marker must be
# present. Both means the ternary in constants.js was never folded, which is # present. Both means the ternary in constants.js was never folded, which is
# what happens when the __BUILD_DEBUG__ define goes missing from build.js: # what happens when the __BUILD_DEBUG__ define goes missing from build.js:
# DEBUG stops being known at build time and the debug branch is live again. # DEBUG stops being known at build time. Neither means we are reading output
# Neither means we are reading output we do not understand. Both are hard # we do not understand. Both are hard failures; neither is ever treated as
# failures; neither is ever treated as absence of a problem. # absence of a problem.
read_marker() { read_marker() {
_file="$1" _file="$1"
_on=no _on=no
@@ -52,9 +67,14 @@ read_marker() {
if has_marker "$MARKER_OFF" "$_file"; then _off=yes; fi if has_marker "$MARKER_OFF" "$_file"; then _off=yes; fi
if [ "$_on" = yes ] && [ "$_off" = yes ]; then if [ "$_on" = yes ] && [ "$_off" = yes ]; then
fail "$_file carries both debug markers, so the build-time DEBUG value fail "$_file carries both debug markers, so DEBUG was not resolved at
was never resolved and the debug branch is still live. Check that build.js build time: the ternary in src/shared/constants.js survived into the
still defines __BUILD_DEBUG__." emitted output. This does not mean the debug branch is live in this
artifact: an unresolved __BUILD_DEBUG__ is undeclared in extension
context, so DEBUG evaluates to false at runtime. It does mean the
release/debug distinction is no longer enforced at build time, and which
way that fallback happens to evaluate is then an accident a refactor can
flip. Check that build.js still defines __BUILD_DEBUG__."
fi fi
if [ "$_on" = no ] && [ "$_off" = no ]; then if [ "$_on" = no ] && [ "$_off" = no ]; then
fail "$_file carries no debug marker, so its DEBUG state cannot be fail "$_file carries no debug marker, so its DEBUG state cannot be
@@ -70,13 +90,20 @@ read_marker() {
} }
# The manifest says which bundles must carry a marker. This says no other # The manifest says which bundles must carry a marker. This says no other
# emitted bundle may carry one, which catches a manifest that has gone stale # emitted file may carry one, which catches a manifest that has gone stale
# or short rather than trusting whatever it happens to list. # or short rather than trusting whatever it happens to list.
#
# Deliberately unfiltered by extension. build.js selects manifest entries with
# an endsWith(".js") test; repeating that literal here would mean a bundle
# emitted under some other extension escaped the manifest AND this check at
# once, which is the correlated blind spot the two-source design exists to
# avoid. Every file under dist/ is searched, so build.js's filter is the only
# place the assumption lives and this check is what catches it being wrong.
check_unlisted_bundles() { check_unlisted_bundles() {
_listing="$(find dist -type f -name '*.js' | sort)" _listing="$(find dist -type f | sort)"
while read -r _file; do while read -r _file; do
[ -n "$_file" ] || continue [ -n "$_file" ] || continue
if grep -q -x -F "$_file" "$MANIFEST"; then if grep -q -x -F -e "$_file" -- "$MANIFEST"; then
continue continue
fi fi
if has_marker "$MARKER_ON" "$_file" || if has_marker "$MARKER_ON" "$_file" ||
@@ -113,12 +140,18 @@ main() {
fail "$MANIFEST is empty, so no emitted bundle was found to contain fail "$MANIFEST is empty, so no emitted bundle was found to contain
src/shared/constants.js. That is never correct, so it is a failure and not src/shared/constants.js. That is never correct, so it is a failure and not
a pass." a pass."
[ -r "$MANIFEST" ] ||
fail "$MANIFEST is not readable, so nothing was inspected. That is a
permissions or I/O fault, not a pass."
count=0 count=0
while read -r file; do while read -r file; do
[ -n "$file" ] || continue [ -n "$file" ] || continue
[ -f "$file" ] || [ -f "$file" ] ||
fail "$MANIFEST lists $file, which does not exist." fail "$MANIFEST lists $file, which does not exist."
[ -s "$file" ] ||
fail "$MANIFEST lists $file, which is empty. An empty bundle
carries no marker and proves nothing, so this is a failure and not a pass."
read_marker "$file" read_marker "$file"
[ "$MARKER" = "$expected" ] || [ "$MARKER" = "$expected" ] ||
fail "$file is $MARKER but this build expects $expected." fail "$file is $MARKER but this build expects $expected."

View File

@@ -1,6 +1,10 @@
const { $, showView, showFlash, goBack, clearViewStack } = require("./helpers"); const { $, showView, showFlash, goBack, clearViewStack } = require("./helpers");
const { state, saveState } = require("../../shared/state"); const { state, saveState } = require("../../shared/state");
const { decryptWithPassword } = require("../../shared/vault"); const { decryptWithPassword } = require("../../shared/vault");
const {
removeWalletFromState,
broadcastActiveChanged,
} = require("../../shared/walletDelete");
let deleteWalletIndex = null; let deleteWalletIndex = null;
let ctx = null; let ctx = null;
@@ -58,35 +62,24 @@ function init(_ctx) {
return; return;
} }
// Collect addresses to clean up from allowedSites/deniedSites // Remove the wallet and repair selection, permissions and hasWallet
const addresses = (wallet.addresses || []).map((a) => a.address); const { activeAddressChanged } = removeWalletFromState(
state,
// Remove wallet walletIdx,
state.wallets.splice(walletIdx, 1); );
// Clean up site permissions for deleted addresses
for (const addr of addresses) {
delete state.allowedSites[addr];
delete state.deniedSites[addr];
}
deleteWalletIndex = null; deleteWalletIndex = null;
if (state.wallets.length === 0) { if (!state.hasWallet) {
// No wallets left — reset selection and show welcome
state.selectedWallet = null;
state.selectedAddress = null;
state.activeAddress = null;
clearViewStack(); clearViewStack();
await saveState(); await saveState();
// Save before broadcasting: the background reads the active
// address back out of storage to build accountsChanged.
if (activeAddressChanged) broadcastActiveChanged();
showView("welcome"); showView("welcome");
} else { } else {
// Switch to first wallet if deleted wallet was active
state.selectedWallet = 0;
state.selectedAddress = 0;
state.activeAddress =
state.wallets[0].addresses[0]?.address || null;
await saveState(); await saveState();
if (activeAddressChanged) broadcastActiveChanged();
// Reset stack to [main] so Settings back goes home. // Reset stack to [main] so Settings back goes home.
// Use require() lazily to avoid circular dependency // Use require() lazily to avoid circular dependency
// (settings.js requires deleteWallet.js). // (settings.js requires deleteWallet.js).

View File

@@ -0,0 +1,70 @@
// Wallet deletion state transition, kept out of the view so the selection
// and broadcast rules are testable without a DOM.
// Remove wallet `walletIdx` from `state` and repair the derived state.
//
// Rules:
// - `hasWallet` tracks whether any wallet remains.
// - Site permissions are dropped for every address of the deleted wallet.
// - `selectedWallet` follows the splice: it is decremented when a wallet
// before it was removed, and falls back to the first remaining wallet's
// first address only when the selection itself was deleted.
// - `activeAddress` is only moved when it belonged to the deleted wallet;
// the fallback is the first remaining wallet's first address, or null
// when no wallet remains.
//
// Returns whether `activeAddress` changed, so the caller can broadcast it.
function removeWalletFromState(state, walletIdx) {
const wallet = state.wallets[walletIdx];
const addresses = (wallet.addresses || []).map((a) => a.address);
const previousActive = state.activeAddress;
const activeWasDeleted =
previousActive !== null &&
previousActive !== undefined &&
addresses.some(
(a) => a.toLowerCase() === String(previousActive).toLowerCase(),
);
state.wallets.splice(walletIdx, 1);
for (const addr of addresses) {
delete state.allowedSites[addr];
delete state.deniedSites[addr];
}
state.hasWallet = state.wallets.length > 0;
const fallbackAddress = state.hasWallet
? state.wallets[0].addresses[0]?.address || null
: null;
if (!state.hasWallet) {
state.selectedWallet = null;
state.selectedAddress = null;
} else if (state.selectedWallet === walletIdx) {
state.selectedWallet = 0;
state.selectedAddress = 0;
} else if (
typeof state.selectedWallet === "number" &&
state.selectedWallet > walletIdx
) {
state.selectedWallet -= 1;
}
if (activeWasDeleted || !state.hasWallet) {
state.activeAddress = fallbackAddress;
}
return { activeAddressChanged: state.activeAddress !== previousActive };
}
// Tell the background the active address changed, so it re-emits
// accountsChanged to connected sites. Same call shape as the address
// switch in the home view.
function broadcastActiveChanged() {
const runtime =
typeof browser !== "undefined" ? browser.runtime : chrome.runtime;
runtime.sendMessage({ type: "AUTISTMASK_ACTIVE_CHANGED" });
}
module.exports = { removeWalletFromState, broadcastActiveChanged };

129
tests/walletDelete.test.js Normal file
View File

@@ -0,0 +1,129 @@
const {
removeWalletFromState,
broadcastActiveChanged,
} = require("../src/shared/walletDelete");
// Fixed addresses — never used for anything but these tests.
const A0 = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const A1 = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
const B0 = "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599";
const C0 = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
function wallet(name, addresses) {
return {
name,
addresses: addresses.map((address) => ({ address })),
};
}
// A three-wallet state; wallet A is an HD wallet with two addresses.
function makeState(overrides = {}) {
return {
hasWallet: true,
wallets: [wallet("A", [A0, A1]), wallet("B", [B0]), wallet("C", [C0])],
selectedWallet: 0,
selectedAddress: 0,
activeAddress: A0,
allowedSites: {
[A0]: ["a.example"],
[A1]: ["b.example"],
[B0]: ["c.example"],
},
deniedSites: { [A1]: ["d.example"], [C0]: ["e.example"] },
...overrides,
};
}
describe("removeWalletFromState", () => {
test("deleting the last wallet clears hasWallet", () => {
const state = makeState({
wallets: [wallet("A", [A0])],
allowedSites: { [A0]: ["a.example"] },
deniedSites: {},
});
const { activeAddressChanged } = removeWalletFromState(state, 0);
expect(state.hasWallet).toBe(false);
expect(state.wallets).toEqual([]);
expect(state.selectedWallet).toBeNull();
expect(state.selectedAddress).toBeNull();
expect(state.activeAddress).toBeNull();
expect(activeAddressChanged).toBe(true);
});
test("deleting a non-selected wallet leaves the selection intact", () => {
const state = makeState({
selectedWallet: 2,
selectedAddress: 0,
activeAddress: C0,
});
const { activeAddressChanged } = removeWalletFromState(state, 1);
// Wallet C moved from index 2 to index 1 by the splice.
expect(state.wallets.map((w) => w.name)).toEqual(["A", "C"]);
expect(state.selectedWallet).toBe(1);
expect(state.selectedAddress).toBe(0);
expect(state.activeAddress).toBe(C0);
expect(activeAddressChanged).toBe(false);
expect(state.hasWallet).toBe(true);
});
test("deleting a wallet after the selection does not shift it", () => {
const state = makeState({
selectedWallet: 1,
selectedAddress: 0,
activeAddress: B0,
});
const { activeAddressChanged } = removeWalletFromState(state, 2);
expect(state.selectedWallet).toBe(1);
expect(state.activeAddress).toBe(B0);
expect(activeAddressChanged).toBe(false);
});
test("deleting the active wallet falls back to the first remaining address", () => {
const state = makeState({
selectedWallet: 0,
selectedAddress: 1,
activeAddress: A1,
});
const { activeAddressChanged } = removeWalletFromState(state, 0);
expect(state.wallets.map((w) => w.name)).toEqual(["B", "C"]);
expect(state.selectedWallet).toBe(0);
expect(state.selectedAddress).toBe(0);
expect(state.activeAddress).toBe(B0);
expect(activeAddressChanged).toBe(true);
expect(state.hasWallet).toBe(true);
});
test("site permissions are dropped for every address of the wallet", () => {
const state = makeState();
removeWalletFromState(state, 0);
expect(state.allowedSites).toEqual({ [B0]: ["c.example"] });
expect(state.deniedSites).toEqual({ [C0]: ["e.example"] });
});
});
describe("broadcastActiveChanged", () => {
afterEach(() => {
delete global.chrome;
});
test("sends AUTISTMASK_ACTIVE_CHANGED to the background", () => {
const sendMessage = jest.fn();
global.chrome = { runtime: { sendMessage } };
broadcastActiveChanged();
expect(sendMessage).toHaveBeenCalledWith({
type: "AUTISTMASK_ACTIVE_CHANGED",
});
});
});