From 84be04d6e9392cc88e0ea8e567cb0b7ca059f26b Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Sep 2026 07:54:27 +0000 Subject: [PATCH] harden: keep the test recovery phrase out of release bundles, match committed keys by content (closes #351) 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 --- TODO.md | 11 ++++++++++- script/test-verify-build | 29 +++++++++++++++++++++++++++++ script/verify-build | 27 +++++++++++++++++++++++++++ src/shared/constants.js | 11 +++++++++-- tests/extensionId.test.js | 25 +++++++++++++++++-------- tests/wallet.test.js | 8 ++++++++ 6 files changed, 100 insertions(+), 11 deletions(-) diff --git a/TODO.md b/TODO.md index 55adffe..53e2ad2 100644 --- a/TODO.md +++ b/TODO.md @@ -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 @@ -58,7 +68,6 @@ but the review is broader than any of them. approve, reject and disconnect paths against a transaction approval broadcasting behind them: each is declined and the dApp still receives its broadcast result. - - 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 diff --git a/script/test-verify-build b/script/test-verify-build index 04d8f94..88180f2 100755 --- a/script/test-verify-build +++ b/script/test-verify-build @@ -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 diff --git a/script/verify-build b/script/verify-build index eed1a45..0b564a8 100755 --- a/script/verify-build +++ b/script/verify-build @@ -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" ] || diff --git a/src/shared/constants.js b/src/shared/constants.js index b16e4af..6a36273 100644 --- a/src/shared/constants.js +++ b/src/shared/constants.js @@ -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"; diff --git a/tests/extensionId.test.js b/tests/extensionId.test.js index 58b5c32..220d437 100644 --- a/tests/extensionId.test.js +++ b/tests/extensionId.test.js @@ -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([]); }); }); diff --git a/tests/wallet.test.js b/tests/wallet.test.js index 0adf376..b9035df 100644 --- a/tests/wallet.test.js +++ b/tests/wallet.test.js @@ -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();