fix: verify-build's dist walk is line-delimited, so a path with trailing whitespace escapes the unlisted-bundle check #223

New Issue

2026-08-11T14:55:25+02:00

clawbot commented

2026-08-11 14:55:25 +02:00

check_unlisted_bundles in script/verify-build walks dist/ through while read -r _file. Two paths escape the check entirely and the script still reports success:

dist/chrome/src/popup/index.js with a trailing space — unlisted and carrying a marker. read -r strips the trailing blank, the remnant matches a manifest line, is_listed returns 0, and the real file is never checked. Exit 0, verify-build: 4 bundle(s) verified autistmask-build-debug=off.
A filename containing a trailing newline — find prints two lines; line 1 is a listed path and line 2 is empty, hitting the [ -n "$_file" ] || continue guard. Exit 0, same message.

Both are pre-existing — they reproduce identically against the pre-fix script — and #180 explicitly recorded whitespace-bearing paths as considered and rejected, so the review of #203 correctly did not fail on them.

Filing anyway for one reason: that PR added a comment asserting "Every file under dist/ is searched", and made the whole-tree walk load-bearing by removing the *.js filter. The claim is now untrue for these paths, and a future reader will rely on it. Either the walk becomes exhaustive or the comment stops promising that it is.

Not reachable from the failure mode the guard defends — esbuild will not emit such a name — so this is about the guard being honest, not about a live exposure.

Implementation requirements

Convert the walk to NUL delimiting: find dist $ -type f -o -type l $ -print0 fed through xargs -0 or -exec sh -c, so no path can be reshaped by field splitting.
Preserve everything the current version fails closed on: the find non-zero status guard, the symlink handling, and has_marker/is_listed's exit-2 treatment.
While in here, note a second undocumented dependency the review surfaced: replacing dist/ itself with a symlink reduces the walk to a single entry and skips the cross-check — it currently fails closed only because GNU grep returns 2 on a directory. That holds on node:22-slim but is nowhere asserted. Make it explicit rather than incidental.

Definition of done

A file named dist/chrome/src/popup/index.js (trailing space) carrying a marker makes script/verify-build exit non-zero.
A filename containing a newline likewise fails rather than passing.
dist/ replaced by a symlink fails for a stated reason the script itself checks, not as a side effect of grep's directory behaviour.
All existing failure modes still fail: missing/empty/unreadable manifest, missing/empty/unreadable listed bundle, unlisted extension, unwalkable subtree, the symlink cases.
POSIX sh, sh -n and dash -n clean.
TODO.md updated in the same commit.
make check passes.

`check_unlisted_bundles` in `script/verify-build` walks `dist/` through `while read -r _file`. Two paths escape the check entirely and the script still reports success: - `dist/chrome/src/popup/index.js` with a **trailing space** — unlisted and carrying a marker. `read -r` strips the trailing blank, the remnant matches a manifest line, `is_listed` returns 0, and the real file is never checked. Exit 0, `verify-build: 4 bundle(s) verified autistmask-build-debug=off`. - A filename containing a **trailing newline** — `find` prints two lines; line 1 is a listed path and line 2 is empty, hitting the `[ -n "$_file" ] || continue` guard. Exit 0, same message. Both are pre-existing — they reproduce identically against the pre-fix script — and https://git.eeqj.de/sneak/AutistMask/issues/180 explicitly recorded whitespace-bearing paths as considered and rejected, so the review of https://git.eeqj.de/sneak/AutistMask/pulls/203 correctly did not fail on them. Filing anyway for one reason: that PR added a comment asserting **"Every file under `dist/` is searched"**, and made the whole-tree walk load-bearing by removing the `*.js` filter. The claim is now untrue for these paths, and a future reader will rely on it. Either the walk becomes exhaustive or the comment stops promising that it is. Not reachable from the failure mode the guard defends — esbuild will not emit such a name — so this is about the guard being honest, not about a live exposure. ## Implementation requirements - Convert the walk to NUL delimiting: `find dist $ -type f -o -type l $ -print0` fed through `xargs -0` or `-exec sh -c`, so no path can be reshaped by field splitting. - Preserve everything the current version fails closed on: the `find` non-zero status guard, the symlink handling, and `has_marker`/`is_listed`'s exit-2 treatment. - While in here, note a second undocumented dependency the review surfaced: replacing `dist/` itself with a symlink reduces the walk to a single entry and skips the cross-check — it currently fails closed **only** because GNU `grep` returns 2 on a directory. That holds on `node:22-slim` but is nowhere asserted. Make it explicit rather than incidental. ## Definition of done - [ ] A file named `dist/chrome/src/popup/index.js ` (trailing space) carrying a marker makes `script/verify-build` exit non-zero. - [ ] A filename containing a newline likewise fails rather than passing. - [ ] `dist/` replaced by a symlink fails for a stated reason the script itself checks, not as a side effect of `grep`'s directory behaviour. - [ ] All existing failure modes still fail: missing/empty/unreadable manifest, missing/empty/unreadable listed bundle, unlisted extension, unwalkable subtree, the symlink cases. - [ ] POSIX `sh`, `sh -n` and `dash -n` clean. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.

clawbot added this to the 1.0.0 milestone 2026-08-11 14:55:25 +02:00

clawbot referenced this issue from a commit

2026-08-11 15:05:51 +02:00

fix: NUL-delimit verify-build's dist walk so no path escapes the check (closes #223)

clawbot referenced a pull request that will close this issue

2026-08-11 15:06:38 +02:00

fix: NUL-delimit verify-build's dist walk so no path escapes the check (closes #223) #225

clawbot referenced this issue

2026-08-11 15:08:53 +02:00

test: verify-build's failure modes are only ever proven by hand — make the battery a committed target #227

clawbot referenced this issue from a commit

2026-08-11 15:24:45 +02:00