Repository navigation
fix(build): scripts ship as written, strict directives, verify:linux works on Node 26, checked tool inputs - #311
Merged
Merged
Conversation
splice() and spliceAndCopy() move out of build.mjs unchanged (the moved text diffs clean against the old lines), so the unit tests can run the splicer: build.mjs deletes and rebuilds the build directories as soon as it is loaded. Proven output-neutral: the three build directories (firefox-github, firefox-amo, web) are byte-identical to a build of the parent commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
miniglob took its separator from require('path').sep, but an ES module run
by Node has no require, so it was always '/', even on Windows: a pattern
built with path.join() (backslashes) matched nothing there, silently. Under
vitest, which does define require, it was '\' instead, and build.mjs's own
glob(dir + '/**') found nothing. Its separator is now '/' everywhere (Node
takes it in Windows paths), and on Windows glob() reads a backslash in a
pattern as a separator, as match() already did (no escaping there).
tests/unit/miniglob.test.mjs covers both patterns, plus Windows simulated
on Linux; taking out either half of the fix fails it. The three build
directories are byte-identical to the baseline build.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A misspelt directive (// SPLICER:WEB:REMOVE_LIN) was kept as an ordinary line, so the line it was meant to remove shipped, with no error; a misspelt target was never acted on by any build. And the directive's text was obeyed anywhere in a line: SPLICER:WEB:REMOVE_FILE in a template literal emptied the file. A directive is now a // comment that is exactly SPLICER:<TARGET>:<COMMAND> (alone or after code, as all 12 in the tree are); any other `// SPLICER:` comment, an unknown target or an unknown command fails the build with the file and line. The same text in a string or a block comment is left alone. tests/unit/splicer.test.mjs: five of its tests fail on the old splicer; one checks every directive in the tree parses for every target. The three build directories are byte-identical to the baseline build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The splicer deleted every blank line of every .js/.mjs it copied, the ones inside template literals too, so a multi-line string shipped changed. It did today: hls.js's getSequenceError message lost its blank line. It also meant no shipped library matched the file sync-vendor writes from npm. Blank lines now stay. Measured on this tree, all three targets: only scripts differ from the previous build, each only by blank lines; 177 of the 180 shipped scripts are now byte-identical to their file in chrome/ (the rest carry a directive, or are mp4box's three files, which npm publishes without a final newline and which get one, as before). The zips grow by about 6 KB of 7.35 MB. tests/unit/splicer.test.mjs: "keeps blank lines" and "ships a script without directives exactly as written" fail with the old filter back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ts (#175, #174) #175: the web build's exclusion list was matched as a raw string prefix, so 'background' and 'custom' (directories) would also have dropped a future root-level backgroundAudio.mjs or customElements.mjs, silently; on Windows an entry with a '/' never matched at all (the relative path has '\'). An entry now names a file or a directory, by whole path segments. #174: every file was read as UTF-8 text before the exclusion check and the script test, the wasm, the VAD model and the images included, and the text thrown away. Only scripts are read now; the rest is copied as bytes, as before. Measured: copying chrome/ for one target, median of 5, 290 ms -> 208 ms. tests/unit/splicer.test.mjs: undoing either change fails its test ("leaves out what the exclusion list names...", "reads only the scripts..."). The three build directories are byte-identical to the build of the previous commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both launchers joined pnpm and web-ext's arguments into one string and spawned it with `shell: false` everywhere but Windows, so Node looked for a program named "pnpm exec web-ext run --source-dir ..." and the launch died with spawn ENOENT, after the whole rebuild. The quotes around paths with spaces would also have reached web-ext as part of the path. tools/rebuild.mjs's new webExtSpawn() gives both: on Windows the same quoted command line through a shell as before (pnpm.cmd needs one), and elsewhere pnpm itself with the arguments as they are. A failed spawn now says so instead of throwing an unhandled 'error' event. tests/unit/launchWebExt.test.mjs: the Linux and macOS cases fail with the old joined string, the Windows case checks the command line is unchanged, and a check that both launchers spawn through it fails on the old launcher. (Firefox is not launched by any test.) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With combined-locales.json absent (deleted to regenerate it, or a checkout without it), localescript.mjs read it as undefined and its key check threw "Cannot read properties of undefined (reading 'get')", which failed every build with an unrelated-looking error; `--combine` could not recreate the file either. And `--whitelist` as the last argument threw on .split. A missing file now reads as no combined locales: the build's check runs on the locale files, `--combine` writes a new combined file, and `--split` stops with "Nothing to split" instead of rewriting nothing. `--whitelist` without a list (last, or followed by another option) says what it needs. On this tree the build's run prints exactly what it printed before. tests/unit/localescript.test.mjs runs a copy in a temp tree: all five tests fail on the old script, and taking out any one of the three guards fails its own test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, gives up on a stalled request (#246) `node generate-locales.mjs --lang french` (or zh-cn, the help's own example) ran every batch against the paid API, then wrote "0 translations": the code's shape was checked only when the results were applied. It is now checked with the other arguments, before any request, and the help's example is zh_CN, as chrome/_locales names it. The help also said the default model was gpt-4o-mini (it is gpt-5-mini) and 5 threads (it is 10). A request that never got an answer waited forever: requests now give up after --timeout seconds (default 300) and the batch retries, as for any other failed request. tests/unit/generateLocales.test.mjs (no real API: no key, or a local server that accepts and never answers): the malformed-code, help and timeout tests fail on the old script, and taking out the new check or the timeout fails its test. Against that server the old script was still waiting after 12 s; the new one gives up after its three attempts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#238, #239) #238: setup.sh installed pnpm through corepack, which Node 25 and later no longer ship. .nvmrc moved to 26 on 2026-10-01, so the next verify:linux would have linked a corepack that is not there and stopped at `corepack enable`. pnpm now comes from package.json's packageManager version through that Node's own npm (as pnpm/action-setup gives CI), over the old corepack links. Trying that showed a second break: Node 26's binary needs libatomic.so.1, which a WSL Ubuntu 24.04 lacks (the runner image and 26.04 have it), so every `node` failed to start; libatomic1 joins the apt list. #239: CI's workflows job runs actionlint, then zizmor, then the run: scripts' tests; verify.sh skipped zizmor, so a workflow change only zizmor flags passed verify:linux and failed CI. setup.sh now takes the zizmor binary out of the image .github/zizmor/Dockerfile pins, the way it takes actionlint and shellcheck out of theirs (one digest-checked image_files function for both), and verify.sh runs `zizmor --offline` on .github/. setup.sh also passes shellcheck now: a comment line starting "# shellcheck binaries..." was read as a broken directive, so shellcheck had stopped at line 11. The AMO build doc no longer leads with `corepack enable`. Proven without running setup.sh or verify.sh: bash -n, shellcheck 0.11.0 and actionlint clean; their pieces, taken from setup.sh itself, run in WSL into /tmp only: both images' files came out digest-checked (a wrong digest refused, the old directory kept), zizmor 1.30.1 passed this tree (no findings), Node 26.10.0 did not start without libatomic1, and its npm installed pnpm 11.28.0 with the pnpm and pnpx links. tests/unit/linuxScripts.test.mjs: all four checks fail on the old scripts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… release (#177) profile:setup put AMO's `latest` uBlock Origin into the persistent dev profile: whatever AMO served that day, no version pin, no hash, the one download in the tool set without either. It is now 1.75.0 by its file URL and SHA-256, as .github/mpv-build.json pins mpv (web-ext turns add-on updates off in that profile, so the version stays as installed either way). The file is written under a .part name and kept only when its hash matches, so a bad download is never taken for "already present" next time; the version is printed. The comment says how to move the pin. The pin was read from AMO's API (current_version 1.75.0, file 5034826) and checked: my own SHA-256 of the download matches it, its manifest says 1.75.0, it carries Mozilla's signature files, and AMO's `latest` URL redirects to this same file today. The script now runs only as a command, so tests can import it. tests/unit/setupDevProfile.test.mjs (a local server; no profile is touched, AMO is not reached): without the hash check the wrong-hash test fails, and with the `latest` URL back the pin test fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…plied (#176) sync-vendor.mjs's `patched: true` was documentation only. With a patchedDependencies line lost from pnpm-workspace.yaml (a bad merge), or a library installed at another version than its patch is cut against (a stale node_modules), the stock library was copied and the build stayed green; e2e would likely catch hls.js or dash.js, not gif.js, Coloris or sweetalert2. unappliedPatches() (beside patchedDependencies() in check-patched-updates.mjs) lists such libraries, and the sync stops on any before copying, saying what to do. When recut-patch.mjs finds that every change landed upstream, the sync now stops until the marks come out, which that path already asks for (its pull request also failed the existing "is the list Dependabot is told to ignore" test). tests/unit/syncVendorPatches.test.mjs: each failing case fails with its branch taken out, and one test checks this repository's marks, yaml and installed versions. On the real sync, with hls.js's yaml line removed the old script exits 0 and the new one exits 1 (likewise for a 1.7.2 key); the yaml was restored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…L js/redos)
`[^}]` also took the `$` and `{` that start a `${...}`, so the two alternatives overlapped.
A `$` now counts as plain text only when no `{` follows. Same matches on tools/sync-vendor.mjs
(all six patched libraries), and CodeQL's hostile input takes 0 ms.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ure on its sum (#248) setup.sh checked the Firefox tarball against SHA256SUMS fetched from the same server, which catches a broken download but not a swapped one: whoever can serve the tarball can serve a matching sum. It now fetches SHA512SUMS, SHA512SUMS.asc and KEY for the release, and verify_signed_sum stops setup unless KEY holds exactly one primary key, the pinned one; SHA512SUMS.asc is a good signature by it (GOODSIG: gpg exits 0 for a signature by an expired key, so its exit status alone would not do); and the SHA-512 that list gives the tarball, named exactly once, is the tarball's. gpg works in a throwaway home with --no-autostart: no keyring of the machine's is read or written, and no agent is left running. gpg joins the apt list. Node stays as it is (its signers change too often to pin). The pin, 14F26682D0916CDD81E37B6D61B7B526D98F0353 (Mozilla Software Releases <release@mozilla.com>, rsa4096, 2015), is what gpg --show-keys reads from the KEY files published with Firefox 60.0, 115.0, 140.0esr and 157.0 and from keyserver.ubuntu.com, all the same (in WSL, a throwaway gpg home). Firefox 157.0's real SHA512SUMS.asc verifies with it, signed by the current subkey 678E455D76767AA3. tests/workflows/linux-firefox-signature.test.sh runs setup.sh's own function on releases made with keys made by the test (no network): a good set passes; a wrong pinned fingerprint, another key's KEY and signature, a KEY with an extra key, a list changed after signing, a signature by a key not in KEY, one by an expired key, a tarball that does not match and one the list does not name are refused. Taking out the KEY check, the signature check, the GOODSIG rule, the listed-once rule or the hash check each fails its checks. tests/unit/linuxScripts.test.mjs checks the pin and that the check runs before the tarball is unpacked; it fails on the previous setup.sh. setup.sh's function on the real Firefox 157.0 files, in WSL /tmp: passes with the pinned key, refused with another fingerprint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…process.exit (CodeQL js/user-controlled-bypass) CodeQL counts process.exit() as a security decision, and the argument checks that lead to it as a bypass of it: the --whitelist check this PR added made two new alerts (and main had four on the same loop). The first bad argument is now kept, reported and ends the run with process.exitCode = 1; the work after the arguments is main(), which runs only when they are all good. Nothing else changed: the remaining exits stay as they were. The same in 20 runs, before and after, by exit status, stdout, stderr and the files it leaves: no arguments (the build's run), --combine, --split with and without combined-locales.json, --combine --split, --whitelist with no list, a list or an empty one, invalid arguments first, last and after a list. `node localescript.mjs` on the real tree prints the same. CodeQL 2.27.1 (security-extended, local threat model, as this repository's code scanning): no localescript.mjs alert left, nothing new. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017JiuRmQkMrNTDSUbPnwKsn
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Build and developer tools: fixes from the code review
What was wrong, as you'd notice it:
// SPLICER:WEB:REMOVE_LIN) was ignored without a word, so code meant to be removed shipped. And a directive's text inside a string was obeyed.backgroundAudio.mjswould have been left out silently.pnpm run start:ffcouldn't start Firefox on Linux or macOS.pnpm run verify:linuxwas already broken: Node 26 no longer ships corepack, and it also needs a library WSL Ubuntu 24.04 lacks. It also skipped zizmor, so a workflow change could pass locally and fail on GitHub.node localescript.mjs(part of every build) crashed ifcombined-locales.jsonwas missing.pnpm run gen-localewith a wrong language code paid for every translation and then saved none. Its help named the wrong defaults, and it could wait forever on a stalled request.What changed:
tools/splicer.mjs.How it's proven:
Note: keeping the blank lines changes every shipped script slightly (the zips grow about 6 KB), so merging this makes a release. That one change is its own commit (38f556f), if you'd rather not have it.
Fixes #165
Fixes #168
Fixes #174
Fixes #175
Fixes #176
Fixes #177
Fixes #178
Fixes #238
Fixes #239
Fixes #245
Fixes #246
Fixes #248
Not fixed here:
verify:linuxuntil someone updates it (your decision on tools/linux/setup.sh: Node and Firefox hashes come over the same channel as the files (no signature check), unlike update-local.ps1 #248).pnpm run verify:linux: it will install gpg, libatomic1, Node 26, pnpm through npm, and zizmor.🤖 Generated with Claude Code