build: make build can produce an INSECURE debug bundle and verify-build certifies it green #309
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?
Found by the pre-1.0 deployability audit (#303). Blocker: the build-integrity control can be satisfied by the artifact it exists to reject.
script/verify-build:225-231computes its expectation by readingAUTISTMASK_DEBUGout of the verifier's own environment.Makefile:50-54invokes it bare, so it inherits the shell that ran the build.README.md:51states "make buildalways produces a release build". It does not.Reproduction
An operator with
AUTISTMASK_DEBUG=1exported in their shell, running the release target, in a throwaway copy of the tree:Loaded in a real Chrome, that build:
That is the publicly committed phrase from
src/shared/constants.js:25-26. Every wallet such a build creates is drainable by anyone who reads this repo.Separately,
verify-buildhas zero provenance — it is a marker grep, not a build-integrity control. Demonstrated:dist/chrome/src/popup/index.jsreplaced by a 26-byte file containing only the stringautistmask-build-debug=offverifiesok.dist/chrome/src/content/index.js(which runs on<all_urls>) anddist/chrome/manifest.jsonare not inspected at all, so arbitrary attacker content in them passes.dist/unrelated to this tree passes withverify-build: 1 bundle(s) verified.The trust root is
dist/constants-bundles.txt, unsigned, inside the tree it describes.The red
DEBUG / INSECUREbanner does appear, which is what makes this a build-process blocker rather than a certain-loss one.Definition of done
make buildscrubs the flag and the expectation is an explicit argument, never ambient env:env -u AUTISTMASK_DEBUG script/verify-build --expect releaseforbuild,--expect debugforbuild-debug. No argument = fail.AUTISTMASK_DEBUG=1exported runningmake buildfails the build, with ascript/test-verify-buildcase asserting exactly that.verify-buildcovers every emitted file indist/<browser>/, includingmanifest.jsonand the content script, not only bundles listed in a file that lives inside the tree it vouches for.dist/.make checkgreen.Plan.
Part 1.
script/verify-buildstops readingAUTISTMASK_DEBUGentirely. It takes--expect release|debug(required; missing, repeated or unknown arguments fail).Makefilecallsenv -u AUTISTMASK_DEBUG script/verify-build --expect release ...forbuildand--expect debugforbuild-debug.AUTISTMASK_DEBUG=1 make buildtherefore still compiles a debug bundle and then fails at verification, which is the loud outcome the issue asks for rather than a silent release.Part 2. The file list moves out of
dist/.build.jsrecords every path it emits (bundles, popup HTML, bothstyles.csscopies, bothmanifest.json) and writes a receipt — one line per emitted file: sha256, whether the bundle containssrc/shared/constants.js, and the path — to the path inAUTISTMASK_BUILD_RECEIPT. TheMakefilecreates that path withmktempper invocation, outside the repo, and removes it in a trap;build.jsrefuses a receipt path insidedist/.dist/constants-bundles.txtis deleted.build.jsalso clearsdist/first, so the tree holds only what this build wrote.verify-build --receipt PATHthen asserts: every receipt entry exists as a regular file with matching bytes; every audited bundle carries the expected marker; no non-audited file carries one; and every file and symlink found underdist/appears in the receipt. So the 26-byte marker file, a tamperedmanifest.jsonor content script, an extra file, and a wholesale hand-writtendist/all fail.What that does and does not prove goes in the PR body — it is a within-one-
make build-invocation guarantee, not cryptographic provenance for a third party holding adist/(that is #310).make verify-build(the standalone re-check target) goes away: without a receipt from a build that just ran, it cannot honestly assert anything, and keeping it would preserve exactly the trust root this issue is about.script/test-verify-buildis extended in place: the receipt replaces the manifest in the fixture, the existing failure modes (grep exit-2,findstatus, newline/trailing-space paths, symlinkeddist/, the permission cases and their root probe) are kept, and cases are added for each of the four demonstrated bypasses, for a missing--expect, and forAUTISTMASK_DEBUG=1being ignored by the verifier.Built and pushed as #330 (branch
issue-309-build-integrity, basenext). Full rationale is in the PR body; the short version and how it was verified:Part 1.
script/verify-buildreads nothing from its environment. The mode is the required argument--expect release|debug, with no default;make buildpasses--expect releaseunderenv -u AUTISTMASK_DEBUG,make build-debugpasses--expect debug.AUTISTMASK_DEBUG=1is deliberately still allowed to reach the compiler, so that shell compiles a debug bundle and then fails the target instead of quietly getting one. Run by hand on the branch:AUTISTMASK_DEBUG=1 make buildnow exits 2 onis autistmask-build-debug=on but this build was told to expect autistmask-build-debug=off, where before it printedverify-build: 4 bundle(s) verified autistmask-build-debug=onand exited 0.Part 2.
build.jsrecords every file it emits and writes a receipt of them (path, sha256, whether the file bundlesconstants.js) to a path theMakefilemktemps per invocation outside the repo and deletes afterwards; a receipt path insidedist/is refused,dist/is cleared before a build, anddist/constants-bundles.txtis gone.verify-buildchecks the receipt's shape, then thatdist/holds nothing the build did not emit and no symlinks, then every recorded file's bytes and every audited bundle's marker. All four demonstrated bypasses now fail: the 26-byte marker-only file, the tampered content script, the tamperedmanifest.json, and the hand-writtendist/.The guarantee is stated precisely rather than broadly, in README.md and in the script header:
dist/is byte for byte the output of thebuild.jsrun that just finished. It says nothing about the honesty of the source tree or ofbuild.js, and nothing to a third party holding adist/— that is signing, #310. One consequence: the standalonemake verify-buildtarget is removed, because re-verifying adist/from a list inside it is the defect itself.Verified.
make checkgreen (39 suites / 811 tests,test-verify-build: 39 case(s) passed, up from 18 cases), and green inside the pinned image viadocker build --no-cache-filter=check .so the stage demonstrably ran, where the harness runs as root and reports thesetprivrunner rather than skipping the permission cases. Coverage proved non-vacuous by mutation: disabling the digest comparison fails exactly the four bypass cases, removing thedist/walk fails the eight extra-file and symlink cases, restoring the ambient-environment fallback fails the missing---expectcase, breaking theMakefilerecipe fails themake -nwiring cases, and droppingmanifest.jsonfrom the recorded emissions fails a realmake build.