build: a failed release build leaves the complete INSECURE debug bundle loadable in dist/ #333
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Raised by the review of #330 (#330 (comment)) as a design choice rather than a defect. Filing it because the decision should be recorded either way.
After #309, running
make buildin a shell withAUTISTMASK_DEBUG=1exported correctly fails with exit 2. Butdist/is left holding the complete debug bundle — loadable, and every wallet it creates uses the publicly committed test recovery phrase fromsrc/shared/constants.js.The failure is loud and the red
DEBUG / INSECUREbanner is on, so an operator who loads it is warned. Keeping the artifact as evidence of what went wrong is defensible. The counter-argument is that the entire point of #309 was that a debug bundle must not be mistakable for a release one, and a failed release build that leaves a loadable debug build behind is a smaller version of that same trap — particularly in CI, or for anyone who runs the build, sees it fail, and loadsdist/chrome/anyway.Definition of done
dist/, or it keeps it andREADME.mdstates explicitly that a failed build may leave a loadable debug bundle behind.dist/.script/test-verify-buildcase asserts the chosen behaviour, so it cannot silently flip.make checkgreen.Decision, so the implementer does not have to make it: a failed release build removes
dist/. Reversible, internal, and it is the option that preserves what #309 was for — a build that failed must not leave something loadable behind that an operator can mistake for its output. Documenting the trap instead relies on the operator having read the README at the moment they are staring at a failure, which is the weaker control.Implementation requirements:
make build) wipes on failure.make build-debugkeeps itsdist/; a debug build that fails is not producing a mistakable artifact.dist/was removed and why, so the removal is never silent.script/test-verify-buildcase asserts the post-failure state ofdist/directly, not just the exit code.verify-buildinspects — this is about what happens after it says no.