fix: prevent ssh bootstrap from installing a 0-byte/truncated codemux-remote#157
Merged
Conversation
…-remote Closes #133. Two holes let the atomic install ship a broken binary over a working one, plus a probe misclassification that hid the damage: 1. bundled_binary_path gated all three lookup tiers on .exists() only, so a 0-byte dev placeholder sidecar qualified as "bundled" and the background upgrade poller silently installed it over a host's good binary. Every tier now requires a regular file of at least 1 MB (plausible_remote_binary); an empty/tiny candidate is treated as not bundled and dev builds fail loudly with BinaryNotBundled. 2. ssh_upload_executable completed the remote chmod+mv chain even when the local stream died partway (cat saw EOF, exited 0). The remote script now verifies the received byte count with wc -c before the mv; on mismatch it removes the tmp file and exits non-zero, leaving the previously-working binary untouched. The uploader also rejects an empty source outright (would trivially pass an -eq 0 check) and kills the ssh child if write_all fails mid-stream. 3. The probe reported a present-but-broken binary identically to "not installed". It now classifies that case (binary_present_but_broken): a 0-byte file yields a bare "CMR: " line, and a truncated ELF's exec failure is caught by a new "|| printf 'BROKEN'" fallback (previously the exit-126 made the whole probe report Unreachable). The Hosts UI now says "Install to repair - this replaces the file without touching a running daemon" instead of "isn't installed yet", and the background poller's skip reason distinguishes broken from absent. Tests: plausible_remote_binary size/type gating, upload_script pipeline shape (wc -c check, cleanup, tilde escaping), empty-source rejection in the localhost E2E, probe argv BROKEN fallbacks, and parse classification for bare-CMR / garbage / BROKEN / NOT_INSTALLED payloads. Docs: remote-hosts.md bootstrap + probe sections and a Troubleshooting entry for the corrupted-binary repair path (plain Install repairs without restarting the live daemon; "Reinstall agent" kills sessions).
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
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.
Closes #133.
Symptom
A field host ended up with a 0-byte, mode-0700
~/.local/bin/codemux-remotewhile the oldservedaemon kept running from the deleted inode — every probe failed and all devices reported "Reachable, but codemux-remote isn't installed yet", even though the correct repair (plain Install) was never offered.Root causes & fixes
1.
bundled_binary_path()accepted 0-byte placeholder binaries (src-tauri/src/ssh/bootstrap.rs)All three lookup tiers (Tauri resource dir, source-tree paths, dev sibling-exe) gated only on
.exists(). A 0-byte dev placeholder sidecar passed, and the background upgrade poller (hosts_upgrade.rs, best-effort by design) then atomically installed the empty file over a working production binary — silent corruption.→ Every tier now goes through
plausible_remote_binary: regular file, ≥MIN_PLAUSIBLE_REMOTE_BINARY_BYTES(1 MB; the real binary is ~16 MB). Empty/tiny candidates are treated as not bundled →BinaryNotBundled, so dev builds fail loudly instead of shipping placeholders to hosts.2.
ssh_upload_executable()completed the remote chain on a dead local streamIf
stdin.write_allfailed partway, the ssh child wasn't killed — remotecatsaw EOF, exited 0, andchmod && mv -fatomically installed a truncated binary. Atomic ≠ verified.→ The remote script now verifies the received byte count before the swap:
On any failure the tmp file is removed, the error surfaces as
UploadFailed, and the previously-working binary is left untouched — closing the hole for any stream-failure mode. The uploader also rejects an empty source outright (it would trivially pass an-eq 0check) and kills the ssh child onwrite_allfailure.3. Probe classification (issue's fix 3) (
src-tauri/src/ssh/probe.rs)A present-but-broken binary was reported identically to "not installed".
ProbeOutcome::Reachablegains#[serde(default)] binary_present_but_broken:CMR:line → broken.2>/dev/null || printf 'BROKEN\n'so the failure yields aCMR: BROKENline instead of making the whole probe reportUnreachablewith raw exec noise (which previously bypassed classification entirely and hid the Install button).hosts_test_connectionnow says "Reachable, but codemux-remote is present and not working (binary may be corrupted or truncated). Install to repair — this replaces the file without touching a running daemon." — sameneeds_install: true, so the existing Install button repairs in place (no frontend changes). The background poller still never auto-repairs (consent), but its skip reason now distinguishes broken from absent.Verification
cargo checkclean; full lib suite 1974 passed / 0 failed (61 inssh::).plausible_remote_binarysize+type gating,upload_scriptpipeline shape (byte-count check, cleanup, tilde-aware escaping), empty-source rejection assertions in the localhost E2E upload test, probe argvBROKEN-fallback assertions, and parse classification tests for bare-CMR:/ garbage /BROKEN/NOT_INSTALLEDpayloads.sh: truncated stream → exit 1, tmp removed, old binary untouched; exact stream → installed with exec bit; mkdir failure → clean non-zero exit.Docs
docs/features/remote-hosts.md: integrity guards in § bootstrap.rs, updated probe command + classification in § probe.rs, test-coverage counts, and a new Troubleshooting entry for the corrupted-binary case (plain Install repairs without restarting the live daemon; Reinstall agent kills live sessions — matching the issue's repair note).