diff --git a/Makefile b/Makefile index 5f272ea..6be020f 100644 --- a/Makefile +++ b/Makefile @@ -60,19 +60,31 @@ hooks: # scrubbed from the build itself: with AUTISTMASK_DEBUG=1 exported, this target # compiles a debug bundle and then fails on it, loudly, rather than quietly # handing back something other than the release build that was asked for. +# +# Every step of this target is wrapped in script/discard-dist-on-failure, so a +# release build that fails removes dist/ instead of leaving a complete, loadable +# debug bundle there for whoever runs the build, sees it fail, and loads +# dist/chrome/ anyway. A step that succeeds removes nothing, and build-debug is +# deliberately not wrapped. build: @echo "Building extension..." @set -eu; \ receipt="$$(mktemp "$${TMPDIR:-/tmp}/autistmask-build-receipt.XXXXXX")"; \ trap 'rm -f "$$receipt"' EXIT INT TERM; \ - AUTISTMASK_BUILD_RECEIPT="$$receipt" yarn run build 2>&1; \ - env -u AUTISTMASK_DEBUG script/verify-build --expect release \ + script/discard-dist-on-failure \ + env AUTISTMASK_BUILD_RECEIPT="$$receipt" yarn run build 2>&1; \ + script/discard-dist-on-failure \ + env -u AUTISTMASK_DEBUG script/verify-build --expect release \ --receipt "$$receipt" - @script/check-censored --require-dist + @script/discard-dist-on-failure script/check-censored --require-dist # Development-only build: enables the red DEBUG / INSECURE banner and makes # the hardcoded test recovery phrase the output of wallet creation. Never # distribute the artifacts this produces. +# +# No discard-dist-on-failure here, on purpose: a debug build that fails is not +# producing an artifact anyone could mistake for a release one, and its dist/ is +# the evidence of what went wrong. build-debug: @echo "Building extension (DEBUG)..." @set -eu; \ diff --git a/README.md b/README.md index 8335d58..ed60c3f 100644 --- a/README.md +++ b/README.md @@ -63,8 +63,12 @@ that runs `make build`, that target compiles a debug bundle and then **fails**, because it tells `script/verify-build` in so many words that it was supposed to produce a release build. It used to be that the verifier read the same variable out of its own environment, agreed with itself, and reported a debug artifact as -verified. The build prints which mode it used. See the -[DEBUG Mode Policy](#debug-mode-policy) for what the flag changes. **Never +verified. The build prints which mode it used. A release build that fails also +**removes `dist/`**, and says so: the bundle it had already written is loadable, +and a loud failure is no protection against someone loading `dist/chrome/` +anyway. `make build-debug` keeps its `dist/` on failure — that output is not +mistakable for a release build, and it is the evidence of what went wrong. See +the [DEBUG Mode Policy](#debug-mode-policy) for what the flag changes. **Never distribute a debug build** — every wallet it creates gets the same publicly known test recovery phrase. @@ -158,16 +162,24 @@ provide: `make build` and `make build-debug`; fails loudly rather than passing whenever it cannot determine something. Not part of `make check`, which does not depend on build artifacts existing. +- `script/discard-dist-on-failure COMMAND [ARG...]` — run one step of the + **release** build and, if it fails, remove `dist/` before returning that + step's exit status, saying on stderr that it did and why. Every step of + `make build` runs through it; `make build-debug` runs none of them through it. + A step that succeeds removes nothing, and a removal that cannot be completed + is reported as loudly as one that was - `script/test-verify-build` — exercise every failure mode of `script/verify-build` against a fixture tree in a temp dir, asserting the exit - status and the message of each, and read the `make build` and + status and the message of each, assert the state of `dist/` on disk after a + failing and a succeeding release build step, and read the `make build` and `make build-debug` recipes back out of `make -n` to check that they pass the - mode as an argument on a scrubbed environment. Part of `make check`; it reads - no build artifacts and writes nothing under `dist/`. The cases that depend on - file permissions cannot mean anything for a process that is not subject to - them, so the harness proves its runner against a mode-000 file before counting - them, dropping to an unprivileged user when run as root; if it cannot, it - skips those cases and says so in a banner rather than passing them. + mode as an argument on a scrubbed environment and wrap only the release path. + Part of `make check`; it reads no build artifacts and writes nothing under + `dist/`. The cases that depend on file permissions cannot mean anything for a + process that is not subject to them, so the harness proves its runner against + a mode-000 file before counting them, dropping to an unprivileged user when + run as root; if it cannot, it skips those cases and says so in a banner rather + than passing them. - `script/docker` — build the Docker image tagged via `script/projectname` - `script/cibuild` — CI entrypoint: plain `docker build .` - `script/precommit` — run by the git pre-commit hook; runs `script/check` @@ -181,9 +193,11 @@ The Makefile shims to those. It also carries a few targets that have no silently rewritten. Use `make setup` for a fresh clone. - `make hooks` — shims to `script/install-precommit` - `make build` — build the extension into `dist/chrome/` and `dist/firefox/`, - then verify the result against the build's receipt as a release build + then verify the result against the build's receipt as a release build. A + failure at any step removes `dist/` - `make build-debug` — the same build with `AUTISTMASK_DEBUG=1`, verified as a - debug build (see [Debug Builds](#debug-builds)) + debug build, and keeping its `dist/` on failure (see + [Debug Builds](#debug-builds)) - `make clean` — remove `dist/` - `make dev` — build in watch mode diff --git a/TODO.md b/TODO.md index f0a0cbf..f82e2cf 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,19 @@ but the review is broader than any of them. # Completed Steps +- 2026-08-23: A failed release build no longer leaves a loadable debug bundle in + `dist/` ([#333](https://git.eeqj.de/sneak/AutistMask/issues/333)). With + `AUTISTMASK_DEBUG=1` exported, `make build` compiled a debug bundle and failed + on it in `script/verify-build` — but the bundle stayed on disk, loadable, with + every wallet it creates using the publicly committed test recovery phrase. + Every step of `make build` now runs through `script/discard-dist-on-failure`, + which removes `dist/` when a step fails and says on stderr that it did and + why; a removal it cannot complete is reported just as loudly. + `make build-debug` is deliberately not wrapped: its output is not mistakable + for a release build and is the evidence of the failure. + `script/test-verify-build` asserts the state of `dist/` on disk after a + failing and a succeeding step, not just the exit status, and reads `make -n` + to check the wrapper is on the release path and only there. - 2026-08-23: `README.md` and `script/verify-build`'s own comments now state the emitted-tree guarantee at the width the code actually enforces ([#331](https://git.eeqj.de/sneak/AutistMask/issues/331)). The tree walk is diff --git a/script/discard-dist-on-failure b/script/discard-dist-on-failure new file mode 100755 index 0000000..35d439c --- /dev/null +++ b/script/discard-dist-on-failure @@ -0,0 +1,78 @@ +#!/bin/sh +# script/discard-dist-on-failure: run one step of the RELEASE build, and if that +# step fails, remove dist/ before returning its exit status. Our own extension +# to scripts-to-rule-them-all, wrapped around every step of make build. +# +# Why: with AUTISTMASK_DEBUG=1 exported in the calling shell, make build +# compiles a debug bundle and then fails on it in script/verify-build — but the +# bundle is already written. It is loadable, and every wallet it creates gets +# the publicly committed test recovery phrase from src/shared/constants.js. A +# failed release build that leaves that behind is a smaller version of the trap +# the verifier exists to close, and "the failure was loud" only works on an +# operator who does not load dist/chrome/ anyway. Removing the artifact does not +# depend on that. +# +# Two things this deliberately does not do. It does not wrap make build-debug: a +# debug build that failed is not a mistakable artifact, and its output is the +# evidence of what went wrong. And it never removes anything on a step that +# SUCCEEDS, including the final check-censored --require-dist pass. +# +# The removal is never silent: it says dist/ is gone and why, on stderr, above +# the build's own failure. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" +DIST="$ROOT/dist" + +usage() { + echo "usage: discard-dist-on-failure COMMAND [ARG...]" >&2 +} + +# Remove dist/, and say so. A removal that could not be completed is reported as +# loudly as one that was: the artifact is still on disk, and reporting nothing +# would leave the operator believing it is not. +discard_dist() { + if [ ! -e "$DIST" ] && [ ! -h "$DIST" ]; then + echo "discard-dist-on-failure: the release build failed. There was no" \ + "dist/ to remove." >&2 + return 0 + fi + + rm -rf "$DIST" || true + + if [ -e "$DIST" ] || [ -h "$DIST" ]; then + echo "discard-dist-on-failure: the release build failed and dist/" \ + "COULD NOT BE REMOVED, so it is still on disk. Do not load it:" \ + "a release build that failed may hold a complete debug bundle," \ + "whose wallets all use the publicly committed test recovery" \ + "phrase. Remove it by hand (make clean)." >&2 + return 0 + fi + + echo "discard-dist-on-failure: the release build failed, so dist/ WAS" \ + "REMOVED and no longer exists. A release build that fails has often" \ + "already emitted a complete, loadable debug bundle — every wallet it" \ + "creates gets the publicly committed test recovery phrase — so the" \ + "failed build is not left behind to be loaded. Fix the failure and" \ + "re-run make build, or run make build-debug if a debug build is what" \ + "was wanted; that target keeps its output." >&2 +} + +main() { + [ "$#" -ge 1 ] || { + usage + echo "discard-dist-on-failure: no command given, so no build step ran" \ + "and nothing was removed." >&2 + exit 1 + } + + _status=0 + "$@" || _status=$? + + [ "$_status" -ne 0 ] || return 0 + + discard_dist + exit "$_status" +} + +main "$@" diff --git a/script/test-verify-build b/script/test-verify-build index 6ffa058..04d8f94 100755 --- a/script/test-verify-build +++ b/script/test-verify-build @@ -1,7 +1,8 @@ #!/bin/sh # script/test-verify-build: exercise every failure mode of -# script/verify-build. Our own extension to scripts-to-rule-them-all, run -# from script/check so make check covers it. +# script/verify-build, and what make build does with dist/ after one of them +# (script/discard-dist-on-failure). Our own extension to +# scripts-to-rule-them-all, run from script/check so make check covers it. # # Why this exists: verify-build is the build-integrity guard, and four separate # reviews of it each found a fresh vacuous pass — the grep exit-2 conflation, @@ -31,6 +32,7 @@ set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" VERIFY_BUILD="$ROOT/script/verify-build" +DISCARD_DIST="$ROOT/script/discard-dist-on-failure" MARKER_ON="autistmask-build-debug=on" MARKER_OFF="autistmask-build-debug=off" @@ -150,6 +152,7 @@ build_fixture() { mkdir -p "$FIXTURE/script" ln -s "$VERIFY_BUILD" "$FIXTURE/script/verify-build" + ln -s "$DISCARD_DIST" "$FIXTURE/script/discard-dist-on-failure" mkdir -p "$FIXTURE/dist/chrome/src/popup" \ "$FIXTURE/dist/chrome/src/content" \ @@ -513,12 +516,125 @@ c_debug_build() { write_receipt } +c_no_dist() { rm -rf dist; } + +# --- dist discard ----------------------------------------------------------- +# +# make build wraps every step of the release path in +# script/discard-dist-on-failure, so a release build that fails removes dist/: +# with AUTISTMASK_DEBUG=1 exported it has already emitted a complete, loadable +# debug bundle whose every wallet uses the publicly committed test recovery +# phrase, and a loud failure alone does not stop someone loading dist/chrome/ +# anyway. make build-debug is deliberately not wrapped. +# +# Both directions are asserted against the state of dist/ ON DISK after the run, +# not against the exit status: a case reading only the status would keep passing +# if the removal quietly stopped happening, which is the flip this exists to +# catch. The wrapper runs against the fixture — its ROOT is the fixture, via the +# symlink in the fixture's script/ — with trivial commands standing in for the +# build steps, because what is under test is what happens after a step says no, +# not the step. + +# discard_case [cmd...] +discard_case() { + _dc_name="$1" + _dc_setup="$2" + _dc_want_status="$3" + _dc_want_dist="$4" + _dc_want="$5" + _dc_unwanted="$6" + shift 6 + + build_fixture + if ! (cd "$FIXTURE" && "$_dc_setup") >/dev/null 2>&1; then + FAILED=$((FAILED + 1)) + echo " FAIL: $_dc_name" + echo " the case's own setup failed, so nothing was tested." + return 0 + fi + + _dc_status=0 + _dc_out="$(cd "$FIXTURE" && + "$FIXTURE/script/discard-dist-on-failure" "$@" 2>&1)" || _dc_status=$? + + _ok=yes + _why="" + + if [ "$_dc_status" -ne "$_dc_want_status" ]; then + _ok=no + _why="exit status $_dc_status, wanted $_dc_want_status" + fi + + # The assertion this case exists for: what is on disk now. + if [ -e "$FIXTURE/dist" ] || [ -h "$FIXTURE/dist" ]; then + _dc_dist=kept + else + _dc_dist=gone + fi + if [ "$_dc_dist" != "$_dc_want_dist" ]; then + _ok=no + _why="${_why:+$_why; }dist/ is $_dc_dist after the run, wanted" + _why="$_why $_dc_want_dist" + elif [ "$_dc_want_dist" = kept ] && + [ ! -f "$FIXTURE/dist/chrome/src/popup/index.js" ]; then + # Kept has to mean intact: a dist/ emptied out is not one left alone. + _ok=no + _why="${_why:+$_why; }dist/ survived but its emitted bundle did not" + fi + + _dc_check_message "$_dc_want" want + _dc_check_message "$_dc_unwanted" unwanted + + if [ "$_ok" = yes ]; then + PASSED=$((PASSED + 1)) + echo " ok: $_dc_name" + return 0 + fi + + FAILED=$((FAILED + 1)) + echo " FAIL: $_dc_name" + echo " $_why" + echo " --- discard-dist-on-failure output ---" + printf '%s\n' "$_dc_out" | sed 's/^/ /' + echo " --- end output ---" +} + +# Require ($2 = want) or forbid ($2 = unwanted) a substring in the wrapper's +# output, updating _ok and _why. An empty substring asserts nothing. Same grep +# discipline as everywhere else here: 0 and 1 are answers, anything else means +# the message was never checked. +_dc_check_message() { + [ -n "$1" ] || return 0 + + _dcm_g=0 + printf '%s\n' "$_dc_out" | grep -q -F -e "$1" || _dcm_g=$? + case "$_dcm_g" in + 0) + [ "$2" = unwanted ] || return 0 + _ok=no + _why="${_why:+$_why; }message contained: $1" + ;; + 1) + [ "$2" = want ] || return 0 + _ok=no + _why="${_why:+$_why; }message did not contain: $1" + ;; + *) + _ok=no + _why="${_why:+$_why; }grep exited $_dcm_g matching the message, so the + message was never checked" + ;; + esac +} + # --- Makefile wiring -------------------------------------------------------- # The verifier cases above prove what verify-build does when it is told what to -# expect. This proves the Makefile tells it — with the mode as an argument, on -# a scrubbed environment, and identically whether or not AUTISTMASK_DEBUG is -# exported in the shell that ran make. Read off `make -n`, so no build runs. +# expect, and the discard cases prove what the wrapper does with dist/. This +# proves the Makefile wires both up — the mode as an argument, on a scrubbed +# environment, identically whether or not AUTISTMASK_DEBUG is exported in the +# shell that ran make, and the wrapper on the release path only. Read off +# `make -n`, so no build runs. check_makefile_wiring() { if ! command -v make >/dev/null 2>&1; then SKIPPED=$((SKIPPED + 1)) @@ -537,6 +653,34 @@ check_makefile_wiring() { build-debug "verify-build --expect debug" _wiring_case "make build-debug scrubs AUTISTMASK_DEBUG for the verifier" \ build-debug "env -u AUTISTMASK_DEBUG" + + # The release path runs its steps through the wrapper, including the final + # check-censored pass; the debug path runs none of them through it, which is + # what keeps a failed debug build's dist/ on disk. + _wiring_case "make build wraps its steps in discard-dist-on-failure" \ + build "script/discard-dist-on-failure" + _wiring_case "make build wraps check-censored --require-dist too" \ + build "script/discard-dist-on-failure script/check-censored" + _wiring_case_absent "make build-debug never discards its dist/" \ + build-debug "discard-dist-on-failure" +} + +# Run `make -n TARGET` with AUTISTMASK_DEBUG=1 exported, into _wc_out. Returns +# non-zero, having already reported the failure, when make itself failed: a +# recipe that could not be printed was never checked. +_wiring_make_n() { + AUTISTMASK_DEBUG=1 + export AUTISTMASK_DEBUG + _wc_status=0 + _wc_out="$(cd "$ROOT" && make -n "$_wc_target" 2>&1)" || _wc_status=$? + unset AUTISTMASK_DEBUG + + [ "$_wc_status" -ne 0 ] || return 0 + + FAILED=$((FAILED + 1)) + echo " FAIL: $_wc_name" + echo " make -n $_wc_target exited $_wc_status" + return 1 } _wiring_case() { @@ -544,18 +688,7 @@ _wiring_case() { _wc_target="$2" _wc_want="$3" - AUTISTMASK_DEBUG=1 - export AUTISTMASK_DEBUG - _wc_status=0 - _wc_out="$(cd "$ROOT" && make -n "$_wc_target" 2>&1)" || _wc_status=$? - unset AUTISTMASK_DEBUG - - if [ "$_wc_status" -ne 0 ]; then - FAILED=$((FAILED + 1)) - echo " FAIL: $_wc_name" - echo " make -n $_wc_target exited $_wc_status" - return 0 - fi + _wiring_make_n || return 0 _wc_g=0 printf '%s\n' "$_wc_out" | grep -q -F -e "$_wc_want" || _wc_g=$? @@ -577,6 +710,34 @@ _wiring_case() { esac } +# The inverse: the recipe must NOT run something. +_wiring_case_absent() { + _wc_name="$1" + _wc_target="$2" + _wc_want="$3" + + _wiring_make_n || return 0 + + _wc_g=0 + printf '%s\n' "$_wc_out" | grep -q -F -e "$_wc_want" || _wc_g=$? + case "$_wc_g" in + 1) + PASSED=$((PASSED + 1)) + echo " ok: $_wc_name" + ;; + 0) + FAILED=$((FAILED + 1)) + echo " FAIL: $_wc_name" + echo " make -n $_wc_target runs: $_wc_want" + ;; + *) + FAILED=$((FAILED + 1)) + echo " FAIL: $_wc_name" + echo " grep exited $_wc_g, so the recipe was never checked" + ;; + esac +} + run_cases() { check_case "control: untouched dist passes" \ no release 0 "2 bundle(s) $MARKER_OFF" c_control @@ -712,6 +873,18 @@ run_cases() { no release 1 "carries a debug marker but the build did not" \ c_marker_on_plain_file + discard_case "a failed release build step removes dist/" \ + c_control 3 gone "dist/ WAS REMOVED" "" sh -c 'exit 3' + + discard_case "a successful release build step leaves dist/ alone" \ + c_control 0 kept "" "REMOVED" true + + discard_case "a failed release build step with no dist/ says there was none" \ + c_no_dist 3 gone "There was no dist/ to remove" "" sh -c 'exit 3' + + discard_case "the wrapper given no command removes nothing" \ + c_control 1 kept "no command given" "" + check_makefile_wiring } @@ -740,6 +913,10 @@ main() { echo "test-verify-build: $VERIFY_BUILD is missing or not executable" >&2 exit 1 } + [ -x "$DISCARD_DIST" ] || { + echo "test-verify-build: $DISCARD_DIST is missing or not executable" >&2 + exit 1 + } echo "Testing script/verify-build failure modes..." pick_sha256_tool