Compare commits

...
2 Commits
Author SHA1 Message Date
sneak 3616472f1f fix: say a second wallet's password is separate when one is chosen (closes #374)
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 1s
check / check (push) Successful in 1m21s
The add-wallet screen offered only "Choose a password" while each wallet
keeps its own encrypted secret, so a second wallet silently accepted a
password different from the first with nothing marking it as separate. A
note now appears on that screen when the profile already holds a wallet,
saying each wallet has its own password and this one need not match any
already in use. It is shown only then — the first wallet has no other
password to differ from — and is decided on screen entry, so it does not
move the password fields. It promises no recovery or reset, staying
consistent with the no-password-reset design.

Model: opus-4-8
2026-09-21 19:01:19 +00:00
clawbot 9ac7df0128 harden: keep the test recovery phrase out of release bundles, match committed keys by content (closes #351)
check / check (push) Failing after 1s
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 1s
The 12-word BIP-39 test phrase survived in every release bundle as dead
text: module.exports keeps DEBUG_MNEMONIC live even though wallet.js's only
use of it folds away in a release build, so it could not be tree-shaken.
Putting the value itself behind the __BUILD_DEBUG__ define makes esbuild fold
it to null, so no distributed bundle carries it. script/verify-build now
fails a release build if the phrase appears in any emitted file, so the fold
cannot silently regress; test-verify-build covers both the release failure
and the debug allowance.

tests/extensionId.test.js now scans the content of every tracked file for a
PEM private-key header instead of matching filename extensions alone, so a
key committed under an unexpected name is caught.

Model: opus-4-8
2026-09-21 18:29:39 +02:00
9 changed files with 200 additions and 10 deletions
+22
View File
@@ -45,6 +45,16 @@ but the review is broader than any of them.
# Completed Steps
- 2026-09-21: The test recovery phrase no longer survives in a release bundle,
and the committed-key guard matches by content
([#351](https://git.eeqj.de/sneak/AutistMask/issues/351)). `DEBUG_MNEMONIC` in
`src/shared/constants.js` is now behind the `__BUILD_DEBUG__` define, so a
release build folds the phrase to `null` and no emitted bundle carries it; it
used to survive as dead text because `module.exports` keeps the const alive.
`script/verify-build` now fails a release build if the phrase appears in any
emitted file, so the fold cannot silently regress. `tests/extensionId.test.js`
scans the content of every tracked file for a PEM private-key header instead
of matching filename extensions alone.
- 2026-09-21: A transaction response is honoured only for a transaction
approval, and the three remaining approval-settlement paths are pinned
([#262](https://git.eeqj.de/sneak/AutistMask/issues/262)). The liveness fix
@@ -59,6 +69,18 @@ but the review is broader than any of them.
broadcasting behind them: each is declined and the dApp still receives its
broadcast result.
- 2026-09-21: Adding a second wallet no longer accepts a different password with
nothing saying it is a separate one
([#374](https://git.eeqj.de/sneak/AutistMask/issues/374)). Each wallet has its
own encrypted secret, so per-wallet passwords are by design; the add-wallet
screen said only "Choose a password". A note now appears on that screen when
the profile already holds a wallet, stating that each wallet has its own
password and this one need not match any already in use. It is shown only
then, since the first wallet has no other password to differ from, and it
stays consistent with the no-reset reality of
[#312](https://git.eeqj.de/sneak/AutistMask/issues/312) by promising no
recovery or reset.
- 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the
common views ([#380](https://git.eeqj.de/sneak/AutistMask/issues/380)). The
wallet list was the reported case: the address shared one row with the
+29
View File
@@ -37,6 +37,10 @@ DISCARD_DIST="$ROOT/script/discard-dist-on-failure"
MARKER_ON="autistmask-build-debug=on"
MARKER_OFF="autistmask-build-debug=off"
# The same test recovery phrase verify-build searches release bundles for. Held
# here too, the way the markers above are, so a case can plant it in a bundle.
TEST_MNEMONIC="cube evolve unfold result inch risk jealous skill hotel bulb night wreck"
RECEIPT_HEADER="autistmask-build-receipt v1"
NEWLINE='
@@ -516,6 +520,24 @@ c_debug_build() {
write_receipt
}
# A release bundle that still carries the test recovery phrase — the regression
# verify-build guards against, and the reason DEBUG_MNEMONIC is behind the
# __BUILD_DEBUG__ define in src/shared/constants.js. The receipt is regenerated
# so the phrase is caught as bundle content, not incidentally as a stale digest.
c_release_bundle_with_mnemonic() {
printf '/* %s */\n' "$TEST_MNEMONIC" >>dist/chrome/src/popup/index.js
write_receipt
}
# The same phrase in a debug build is expected: make build-debug ships it on
# purpose, so the phrase check must stay quiet under --expect debug.
c_debug_bundle_with_mnemonic() {
write_bundle dist/chrome/src/popup/index.js "$MARKER_ON"
write_bundle dist/firefox/src/popup/index.js "$MARKER_ON"
printf '/* %s */\n' "$TEST_MNEMONIC" >>dist/chrome/src/popup/index.js
write_receipt
}
c_no_dist() { rm -rf dist; }
# --- dist discard -----------------------------------------------------------
@@ -753,6 +775,13 @@ run_cases() {
check_case "debug bundles under --expect debug pass" \
no debug 0 "2 bundle(s) $MARKER_ON" c_debug_build
check_case "release bundle carrying the test recovery phrase fails" \
no release 1 \
"carries the BIP-39 test recovery phrase" c_release_bundle_with_mnemonic
check_case "debug bundle carrying the test recovery phrase passes" \
no debug 0 "2 bundle(s) $MARKER_ON" c_debug_bundle_with_mnemonic
check_case "no --expect argument" \
no no-expect 1 "no --expect argument." c_control
+27
View File
@@ -13,6 +13,12 @@
# fallback branch; the property only exists in the emitted output, so it has to
# be asserted against the emitted output.
#
# The DEBUG half also checks the phrase directly: a release build must not carry
# the test recovery phrase in any emitted file. The phrase is behind the
# __BUILD_DEBUG__ define in src/shared/constants.js and folds away in a release
# build, but the marker only proves DEBUG compiled off, not that the fold
# removed the string; the phrase grep is the assertion that it did.
#
# Which mode to expect is an ARGUMENT (--expect release|debug) and is never
# taken from this script's environment. It used to be read from
# AUTISTMASK_DEBUG here, which meant an operator with AUTISTMASK_DEBUG=1
@@ -67,6 +73,15 @@ TAB=' '
MARKER_ON="autistmask-build-debug=on"
MARKER_OFF="autistmask-build-debug=off"
# The 12-word BIP-39 test recovery phrase from src/shared/constants.js. It is
# behind the __BUILD_DEBUG__ define there, so a release build folds it out of
# every bundle; this is the assertion that it stayed out. The phrase is a
# publicly committed test value rather than a secret, but a BIP-39 phrase in a
# distributed wallet artifact is exactly the string a scanner or auditor has to
# stop and reason about, so a release build must not ship it. A debug build
# ships it on purpose, so this is checked only when release is expected.
TEST_MNEMONIC="cube evolve unfold result inch risk jealous skill hotel bulb night wreck"
RECEIPT_HEADER="autistmask-build-receipt v1"
# Set by the arguments.
@@ -283,6 +298,18 @@ check_entry() {
receipt records $ENTRY_HASH and the file on disk is $SHA. Something wrote
to dist/ after the build, so this artifact is not the one that was built."
# No emitted file of a release build may carry the test recovery phrase.
# Checked on every file, not only the audited bundles, so a copy that
# reached some other emitted file fails here too. A debug build ships the
# phrase deliberately, so this runs only when release was expected.
if [ "$EXPECT" = "$MARKER_OFF" ] && has_marker "$TEST_MNEMONIC" "$ENTRY_PATH"; then
fail "$ENTRY_PATH carries the BIP-39 test recovery phrase, which a
release build must fold out. The __BUILD_DEBUG__ define in build.js is what
drops it from src/shared/constants.js; check that DEBUG_MNEMONIC is still
behind that flag. A recovery phrase in a distributed bundle is exactly the
string an auditor or scanner has to stop on, so this is a hard failure."
fi
if [ "$ENTRY_FLAG" = A ]; then
read_marker "$ENTRY_PATH"
[ "$MARKER" = "$EXPECT" ] ||
+15
View File
@@ -152,6 +152,21 @@
<!-- Shared password fields -->
<div class="mb-2" id="add-wallet-password-section">
<!-- Shown only when the profile already holds a wallet:
each wallet has its own password (its own
encryptedSecret), so a second wallet does not reuse
the first one's. addWallet.js toggles this on screen
entry from state.wallets.length, so it is constant
while the screen is up and moves nothing. -->
<p
class="text-xs mb-2 border border-border border-dashed p-2 hidden"
id="add-wallet-separate-password-note"
>
You already have a wallet. Each wallet has its own
password: the one you choose here is only for this new
wallet, and it need not match any password you already
use.
</p>
<label class="block mb-1">Choose a password</label>
<!-- The hint is swapped in place when the import tab
changes, and it sits directly above the password
+15
View File
@@ -100,9 +100,24 @@ function clear() {
$("add-wallet-phrase-warning").style.visibility = "hidden";
}
// Each wallet has its own password (its own encryptedSecret), so adding a
// second wallet does not reuse the first one's. The note that says so is
// only meaningful once a wallet exists — on the first wallet there is no
// other password to be separate from — so it is shown only then. This is
// decided on entry and stays put while the screen is up, so it does not
// move the password fields the way a per-tab hint would.
function updateSeparatePasswordNote() {
const hasExistingWallet = state.wallets.length > 0;
$("add-wallet-separate-password-note").classList.toggle(
"hidden",
!hasExistingWallet,
);
}
function show() {
clear();
switchMode("mnemonic");
updateSeparatePasswordNote();
showView("add-wallet");
}
+9 -2
View File
@@ -22,8 +22,15 @@ const BUILD_DEBUG_MARKER = DEBUG
? "autistmask-build-debug=on"
: "autistmask-build-debug=off";
const DEBUG_MNEMONIC =
"cube evolve unfold result inch risk jealous skill hotel bulb night wreck";
// Behind DEBUG for the same reason BUILD_DEBUG_MARKER is above: in a release
// build __BUILD_DEBUG__ is a compile-time false, esbuild drops this branch, and
// the phrase never reaches a distributed bundle. The literal used to survive as
// dead text because module.exports keeps this const live even though wallet.js's
// only use of it is folded away; making the value itself fold to null removes
// it. script/verify-build fails a release build if the phrase appears anyway.
const DEBUG_MNEMONIC = DEBUG
? "cube evolve unfold result inch risk jealous skill hotel bulb night wreck"
: null;
const ETHEREUM_MAINNET_CHAIN_ID = "0x1";
const ETHEREUM_SEPOLIA_CHAIN_ID = "0xaa36a7";
+58
View File
@@ -0,0 +1,58 @@
// Adding a second wallet accepts a password different from the first one's
// with nothing on screen saying the two are separate — each wallet has its
// own encryptedSecret, so per-wallet passwords are by design, but the add
// screen said only "Choose a password"
// (https://git.eeqj.de/sneak/AutistMask/issues/374).
//
// The fix is copy: a note on the password screen that says each wallet has
// its own password and this one need not match. It is only meaningful once
// a wallet exists — on the very first wallet there is no other password to
// be separate from — so it is shown then and hidden otherwise. These boot
// the real popup and reach the add-wallet screen through the same button a
// user presses, so the note's visibility is decided by the real show().
const {
bootPopup,
cleanupPopup,
unversionedValidProfile,
POPUP_HTML,
} = require("./support/popupBoot");
const NOTE = "add-wallet-separate-password-note";
afterEach(() => {
cleanupPopup();
});
describe("second-wallet password note", () => {
test("hidden while onboarding the first wallet", async () => {
const page = await bootPopup(undefined);
expect(page.pageErrors).toEqual([]);
await page.click("btn-welcome-add");
expect(page.visibleViews()).toContain("add-wallet");
expect(page.hidden(NOTE)).toBe(true);
});
test("shown when a wallet already exists", async () => {
const page = await bootPopup(unversionedValidProfile());
expect(page.pageErrors).toEqual([]);
await page.click("btn-main-add-wallet");
expect(page.visibleViews()).toContain("add-wallet");
expect(page.hidden(NOTE)).toBe(false);
});
// The copy states the two facts the definition of done asks for — each
// wallet has its own password, and this one need not match — and stays
// consistent with the no-password-reset reality of
// https://git.eeqj.de/sneak/AutistMask/issues/312 by not promising any
// recovery or reset here.
test("the note says the password is per-wallet and need not match", () => {
const note = /id="add-wallet-separate-password-note"[^>]*>([^]*?)<\/p>/
.exec(POPUP_HTML)[1]
.replace(/\s+/g, " ")
.trim();
expect(note).toContain("its own");
expect(note).toContain("need not match");
expect(note).not.toMatch(/recover|reset/i);
});
});
+17 -8
View File
@@ -89,19 +89,28 @@ describe("chrome extension identity", () => {
// The private half is a credential. It has never been in this repo and no
// target generates one into the working tree; this fails loudly if that
// ever changes, because a committed .pem is a key anyone can sign a CRX
// with under this extension's id.
// ever changes, because a committed private key is one anyone can sign a
// CRX with under this extension's id.
//
// Matched by CONTENT, not by filename: a key committed as notes.txt or with
// no extension carries the same risk as one named key.pem, and a
// filename-only check waves it through. The PEM header a private key opens
// with is the signature searched for. The pattern does not trip on its own
// source: the bracket-expression characters between the two anchors are not
// in the character class, so this file is not a match for it.
const PRIVATE_KEY_HEADER = /-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----/;
test("no private key is committed anywhere in the tree", () => {
const root = path.join(__dirname, "..");
const tracked = require("child_process")
.execSync("git ls-files", {
cwd: path.join(__dirname, ".."),
encoding: "utf8",
})
.execSync("git ls-files", { cwd: root, encoding: "utf8" })
.split("\n")
.filter(Boolean);
expect(tracked.filter((f) => /\.(pem|key|p12|pfx)$/i.test(f))).toEqual(
[],
const offenders = tracked.filter((f) =>
PRIVATE_KEY_HEADER.test(
fs.readFileSync(path.join(root, f), "latin1"),
),
);
expect(offenders).toEqual([]);
});
});
+8
View File
@@ -27,6 +27,14 @@ describe("generateMnemonic in a release build", () => {
expect(constants.DEBUG).toBe(false);
});
test("the test phrase folds away when DEBUG is false", () => {
// The release bundle is what must not carry the phrase; here, with the
// define absent, DEBUG_MNEMONIC is the null branch the bundler keeps,
// and the literal only exists in the branch it drops.
const { constants } = loadWallet();
expect(constants.DEBUG_MNEMONIC).toBeNull();
});
test("returns fresh, valid 12-word phrases that are not the test phrase", () => {
const { constants, wallet } = loadWallet();