release-train: develop -> staging - #498
Merged
Merged
Conversation
…497) * fix(installer): harden the bootstrap idiom to fail closed (cli#396) The one-liner we print (and exec) for the installer was `bash <(curl -fsSL https://tracebloc.io/i.sh)`. Process substitution doesn't propagate curl's exit code: on a network/DNS/HTTP failure bash reads an empty script from the substituted fd and exits 0. So every copy-paste recovery hint derived from it — doctor's reinstall / "set one up" remedies, the cluster-discovery error, and prepareHostManualHint — could report success while nothing ran, or run a partial download. Move the URL and the idiom into one internal/installer package. The new shape is download-to-temp-file + `set -e`, but on a single line wrapped in a subshell: (set -e; tmp="$(mktemp)"; trap 'rm -f "$tmp"' EXIT; curl -fsSL --tlsv1.2 URL -o "$tmp"; bash "$tmp") - fail-closed: `set -e` + `curl -o` aborts with curl's real status; the full download completes before anything runs, so no partial execution. - single line: a multi-line block pastes unreliably (PowerShell can run a multi-line paste bottom-up, executing the installer before the download). One line pastes the same everywhere. - paste-safe: the subshell scopes `set -e`/`trap` so pasting into an interactive shell can't arm errexit on it (which would close the terminal on the curl failure) or leave a stray EXIT trap. - interactive stdin preserved: runs a downloaded file, not a pipe or process substitution, so the installer's prompts still read the TTY. Every consumer now derives from installer.Cmd/Script, so the printed hint is byte-identical to the command we just tried, and a URL/idiom change lands everywhere at once. Adds an executable test that proves fail-closed (dead endpoint, offline) and no errexit/trap leak. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(installer): separate the stdin and fail-open rationales (cli#396) Asad's review: the doc lumped `bash <(curl …)` in with the stdin-stealing argument, but process substitution hands bash a /dev/fd/N *filename*, so its stdin stays the TTY — that's why it was the original choice. The load-bearing reason to reject it is fail-open (it exits 0 on a curl failure), not stdin. Split the two rationales so the #397 justification is precise. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: shujaat hasan <shujaathasan@shujaats-MacBook-Pro.local> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 522113c. Configure here.
tracebloc-release-train
Bot
added
gate-nudge
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
2 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.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Changes the privileged installer download/bootstrap path and every user-facing install/upgrade hint; behavior is intentionally tightened (fail-closed downloads, new command shape) with tests, but a mistake would affect onboarding and self-update flows broadly.
Overview
Centralizes how the CLI runs and advertises the official installer: new
internal/installerownsURL,Cmd, andScript(subcommand, env)so doctor remedies,upgrade,prepare-host, and cluster “no client” errors all share one bootstrap string.Replaces the old
bash <(curl …)/ duplicateddoctor.goconstants with a one-line subshell that downloadsi.shto a temp file (set -e,curl -o,--tlsv1.2), runsbash "$tmp", and scopesset -e/trap so pasting the hint does not arm errexit on the user’s shell. Upgrade and prepare-host manual fallbacks are now byte-identical to what the CLI actually executes (including optionalTB_PREPARE_USERfor prepare-host).Adds structural and bash integration tests guarding single-line paste safety, no pipe/process substitution, subshell wrapping, and non-zero exit on failed downloads.
Reviewed by Cursor Bugbot for commit 522113c. Bugbot is set up for automated code reviews on this repo. Configure here.