fix: install-script update handoff, Mach-O strip, security alerts, Windows determinism - #1338
Merged
Merged
Conversation
Two open security alerts. Dependabot #13 (GHSA-r28c-9q8g-f849, high): postcss path traversal via sourceMappingURL auto-loading. Transitive dev dependency of vite, so it never ships in the binary, but the fix is a clean lockfile bump -- vite requires ^8.5.3 and the patch floor is 8.5.18, so 8.5.24 satisfies it with no dependency graph change. Verified: npm ci resolves and the UI still builds. Scorecard #76 (PinnedDependenciesID): Dockerfile.glibc22 used a floating `ubuntu:22.04`. Every other venue image is digest-pinned; this one was missed. It matters more here than the checkbox suggests -- that image IS the glibc-floor assertion, the oldest userland we claim the portable binary runs on. A floating tag means the floor silently drifts to whatever 22.04 points at, and a floor that moves is not a floor. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The previous change generalised `--strip-all` to every platform. Apple's strip does not accept that flag, so the macOS build failed outright -- loudly, which was the lucky outcome. The unlucky version was already in the tree before that: `strip -x`. It succeeds and leaves 4058 symbols, which is precisely the state VirusTotal flagged on darwin-arm64 while the fully stripped ELF legs came back clean. Measured on that artifact: llvm-strip --strip-all 373 symbols CLEAN strip (no flags) 378 symbols equivalent strip -x -S 4058 symbols the FLAGGED state strip -X / -u -r 4058 symbols likewise So the flags are chosen per format, and a candidate that cannot do the job is a hard error rather than a silent fallback to a weaker strip. Verified on a PATH without llvm-strip, exactly as the runner has it: 373 symbols, codesign --verify --strict passes, the binary runs. Packaged and scanned before merge: darwin-arm64 0/60 and ui-darwin-arm64 0/61, both CLEAN -- the platform that was flagged on the previous release run. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Completes the move started on Windows. `update` prints the install script's
command and exits 0 on every platform; the download/extract/chmod/exec sequence
is compiled out of release builds entirely, and the MCP background thread that
ran cbm_popen("curl ... api.github.com ...") on first tool call is gone.
The installers now place themselves beside the binary, which closes the last
gap: `update` referenced a raw.githubusercontent URL purely because an
install.sh-installed user had no local copy to point at. Two details make that
safe.
The source is the install script from the archive we JUST checksum-verified,
never "$0" -- which does not exist under `curl | bash`, and would pin the user
to the OLD installer forever. Since the script ships inside the verified
archive, it inherits that verification instead of needing its own.
And it is published by atomic rename, never written over the live path. Bash
reads a script incrementally by byte offset, so overwriting the file it is
executing continues reading NEW bytes at the OLD offset: silent, bizarre
corruption. cp to a temp name then mv -f swaps the directory entry while the
running shell keeps its original inode. Windows follows the same rule via
Copy-Item + Move-Item even though PowerShell parses up front.
Deliberately NOT done: spawning the new installer to delete and replace its
predecessor. Fetch remote code -> drop to disk -> spawn -> self-delete is a
textbook stager, and self-deletion is among the most heavily weighted
heuristics there is. It also gains nothing -- `"$DLBIN" install` already runs
the NEW release's install logic, because the binary owns the install.
Net: zero install or download URLs remain in the shipped binary, and the
allow-list entry is retired with it. Both scripts scan CLEAN (0/61) with the
self-placement code in them.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
All three cost real release cycles, and none is fixed by widening a budget. daemon_application_cancels_physical_job_only_after_final_session waited for the SUBSCRIBER COUNT to reach 2, then cancelled both sessions and asserted the physical job had started exactly once. The job starts asynchronously after subscription, so both cancels could land first and leave starts == 0 -- arguably the correct outcome. It now waits for the state the assertions actually require. Verified 47/47 on Windows, the only platform it ever failed. The parallel harness refused outright when the suite leader had already exited, because taskkill /T cannot walk a tree from a dead PID. But the leader can exit between the timeout decision and that call, so the harness itself lost a race: a natural exit at the wrong moment failed the whole wave. It now proves cleanup the only way still available -- nothing parented to that PID -- and its contract asserts the PROPERTY rather than the phrase "tree cleanup" it used to grep for. That string pin is what broke when the guard was reworded while behaving correctly; the contract now checks rc==2 AND that the descendant really did survive, which would also catch a guard that claims to fail closed while leaking. extract_wide_flat_file_is_linear took ONE sample per size, so the ratio carried the noise of both. On a loaded Windows VM linear code measured 51x against a 40x bound (184ms -> 9387ms). Best-of-N instead: timing noise only ever adds time, so the minimum is the cheapest good estimate of the noise-free cost. The bound is deliberately unchanged -- it sits where linear (~20x) and quadratic (~128x) are each >=2x away, so raising it would move the test toward the very signal it exists to catch. Now measures 19.1x on Windows, 21.4x on macOS. Also: the smoke's `cli` helper redirected stderr to a file and discarded it, so any of the 10 bare `VAR=$(cli ...)` assignments could kill the run under `set -euo pipefail` printing NOTHING. One such abort cost a full Windows cycle just to locate and still could not be attributed. It now surfaces the command and its stderr. Neutral wording on purpose: one call site expects a non-zero exit and must not read as a failure. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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
1 participant
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.
Four independent changes, each verified on all three platforms locally before merge.
1. Security alerts
GHSA-r28c-9q8g-f849, high) — postcss path traversal. Transitive dev dep of vite, never ships in the binary;^8.5.3accepts the 8.5.18 patch floor so 8.5.24 lands with no graph change.npm ciresolves, UI builds.Dockerfile.glibc22used a floatingubuntu:22.04while every other venue image is digest-pinned. This one matters beyond the checkbox: that image is the glibc-floor assertion, and a floor that drifts isn't a floor.2. Mach-O strip
--strip-allisn't accepted by Apple's strip, which broke the macOS build. The quieter bug was already there:strip -xsucceeds and leaves 4058 symbols — exactly the state VirusTotal flagged on darwin-arm64 while the fully-stripped ELF legs were clean.llvm-strip --strip-allstrip(no flags)strip -x -SFlags are now chosen per format, and a tool that can't do the job is a hard error, never a fallback to a weaker strip. Verified on a PATH without
llvm-strip, as the runner has it.3. Update handoff completed — no URL in the binary
updateprints the install script's command everywhere; download/extract/chmod/exec is compiled out of release builds; the MCPcbm_popen("curl … api.github.com …")startup thread is gone.The installers now place themselves beside the binary, which is what let the last URL go. Two details make that safe:
$0, which doesn't exist undercurl | bashand would pin users to the old installer forever.cp+mv -fswaps the directory entry while the running shell keeps its inode.Deliberately not done: spawning the new installer to delete its predecessor. That's a textbook stager shape, and it gains nothing —
"$DLBIN" installalready runs the new release's logic.4. Three Windows nondeterminisms, none fixed by widening a budget
daemon_applicationwaited on subscriber count, then asserted a job had started — but job start is async, so both cancels could land first and leavestarts == 0. Now waits on the state it asserts. 47/47 on Windows, the only platform it failed."tree cleanup"— the string pin that broke when the guard was correctly reworded.extract_wide_flat_file_is_lineartook one sample per size, so the ratio carried both samples' noise: 51× against a 40× bound for linear code. Best-of-N, bound unchanged — it sits where linear (~20×) and quadratic (~128×) are each ≥2× away, so raising it would move the test toward the signal it exists to catch. Now 19.1× Windows / 21.4× macOS.Also: the smoke's
clihelper discarded stderr, so a bareVAR=$(cli …)could kill the run underset -eprinting nothing — one such abort cost a full Windows cycle to locate and still couldn't be attributed. It now surfaces the command and its stderr.Verification
extractionVirusTotal, all CLEAN:
install.sh0/61 ·install.ps10/61 · darwin-arm64 0/60 · ui-darwin-arm64 0/61 — macOS being the platform flagged on the previous release run.lint-ciclean · 10/10 shell contracts ·security-audit.sh: All URLs are on the allow-list.Known, pre-existing, not addressed here: UBSan reports a null-pointer
memcpyargument in the vendoredobjectscript_routine/scanner.c:118on the Linux leg. Non-fatal, unrelated to this change, but it is real UB in code that parses untrusted input — worth its own follow-up.