fix(installer): harden the bootstrap idiom to fail closed (cli#396)

internal/cli/doctor.go

@@ -17,20 +17,10 @@ import (
 	"github.com/tracebloc/cli/internal/cluster"
 	"github.com/tracebloc/cli/internal/config"
 	"github.com/tracebloc/cli/internal/doctor"
+	"github.com/tracebloc/cli/internal/installer"
 	"github.com/tracebloc/cli/internal/ui"
 )
 
-// installerURL is the single source of truth for the installer script URL.
-// Everything that downloads or points at the installer (installCmd here,
-// prepareHostInstallerCmd in prepare_host.go) derives from this so a URL change
-// updates every path at once.
-const installerURL = "https://tracebloc.io/i.sh"
-
-// installCmd is the one-line installer we point people at when there's no
-// secure environment on this machine, or a component needs reinstalling. Kept in
-// one place so every remedy says the same thing.
-const installCmd = "bash <(curl -fsSL " + installerURL + ")"
-
 // doctorRunFn is a test seam over doctor.Run (the cluster-side probe). Tests
 // inject a fixed []doctor.Result so the roll-up + render can be exercised with a
 // controlled mix without standing up a fake cluster.
@@ -133,7 +123,7 @@ func runClusterDoctor(
 			return &exitError{code: exitChecksFailed, err: nil}
 		case errors.As(werr, &ue):
 			p.Newline()
-			p.Errorf("This CLI is out of date — update it: %s", installCmd)
+			p.Errorf("This CLI is out of date — update it: %s", installer.Cmd)
 			return &exitError{code: exitChecksFailed, err: nil}
 		case errors.As(werr, &ae):
 			tok = tokenServerErr // tracebloc answered, just not with 200
@@ -172,7 +162,7 @@ func runClusterDoctor(
 		p.Newline()
 		noteSessionProblem(p, tok)
 		p.Errorf("No secure environment on this machine yet.")
-		p.Hintf("     Set one up: %s", installCmd)
+		p.Hintf("     Set one up: %s", installer.Cmd)
 		return &exitError{code: earlyExitCode(tok), err: nil}
 	}
 	cs, err := newClientsetFn(resolved)
@@ -195,7 +185,7 @@ func runClusterDoctor(
 		p.Newline()
 		noteSessionProblem(p, tok)
 		p.Errorf("No secure environment on this machine yet.")
-		p.Hintf("     Set one up: %s", installCmd)
+		p.Hintf("     Set one up: %s", installer.Cmd)
 		renderDetailsIfVerbose(p, resolved, results)
 		return &exitError{code: earlyExitCode(tok), err: nil}
 	}
@@ -372,7 +362,7 @@ func summarizeDoctor(results []doctor.Result, tok tokenState) (connected, ready
 	case by["Pod health"].Status == doctor.StatusFail:
 		ready = healthLine{doctor.StatusFail,
 			"Not ready — part of your secure environment isn't running.",
-			fmt.Sprintf("Reinstall with `%s`, or email support@tracebloc.io with `%s doctor --diagnose`.", installCmd, launcher())}
+			fmt.Sprintf("Reinstall with `%s`, or email support@tracebloc.io with `%s doctor --diagnose`.", installer.Cmd, launcher())}
 	case by["Pod health"].Status == doctor.StatusWarn && strings.HasPrefix(by["Pod health"].Detail, "could not list pods"):
 		// checkPods returns StatusWarn for TWO different situations: pods stuck
 		// Pending (below) AND a failure to list pods at all (e.g. RBAC, doctor.go

internal/cli/prepare_host.go

@@ -12,6 +12,8 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+
+	"github.com/tracebloc/cli/internal/installer"
 )
 
 // prepareHostUserRe validates the researcher username before we pass it to the
@@ -20,58 +22,25 @@ import (
 // early with a clear error rather than a confusing failure deep in the installer).
 var prepareHostUserRe = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,31}$`)
 
-// installerRunScript builds the bash program that downloads the cosign-verified
-// installer to a temp file and runs THAT — optionally with a subcommand (e.g.
-// "prepare-host"). Shared by `tracebloc upgrade` (no subcommand, full install)
-// and `tracebloc prepare-host`, so both stay on one download-then-execute idiom.
-//
-// We run a downloaded FILE rather than `curl … | bash`. Two reasons, both Bugbot
-// (#394, #397):
-//   - stdin: with `curl | bash`, the inner bash reads its *program* from the
-//     pipe, so the installer's stdin is no longer the terminal. Any interactive
-//     prompt (sign-in, or which non-admin user gets runtime access) would get
-//     EOF. Running a downloaded file leaves stdin on the TTY.
-//   - fail-closed: `set -e` + `curl -o` makes a failed download (network/DNS/HTTP
-//     error) abort with a non-zero status instead of silently running nothing.
-//     (`curl | bash` swallowed this — bash read empty stdin and exited 0.)
-//
-// The download uses `--tlsv1.2` — the TLS 1.2 floor scripts/install.sh enforces
-// on every security-sensitive fetch — so this privileged installer download can
-// never negotiate a weaker protocol (Bugbot #397). The temp file is removed on
-// exit. The URL comes from installerURL (doctor.go) so every installer path
-// shares one source and can't drift (Bugbot #394/#397).
-func installerRunScript(subcommand string) string {
-	run := `bash "$tmp"`
-	if subcommand != "" {
-		run += " " + subcommand
-	}
-	return `set -e
-tmp="$(mktemp)"
-trap 'rm -f "$tmp"' EXIT
-curl -fsSL --tlsv1.2 ` + installerURL + ` -o "$tmp"
-` + run
-}
-
 // prepareHostInstallerCmd runs the official installer's admin-only prepare-host
 // step. Like `tracebloc upgrade`, this deliberately delegates to the verified
 // installer (cosign-checked) instead of re-implementing any privileged host prep
 // in the CLI — the privileged surface stays in one audited place. See
-// installerRunScript for why we download-then-execute rather than pipe.
-var prepareHostInstallerCmd = installerRunScript("prepare-host")
+// installer.Script for why we download-then-execute rather than pipe.
+var prepareHostInstallerCmd = installer.Script("prepare-host", "")
 
 // prepareHostManualHint is the copy-pasteable command we show if the automated
-// run fails. Built from installCmd (doctor.go) — the single shared bootstrap
-// idiom — so a URL/idiom change updates every hint at once (Bugbot #394); we
-// only append the prepare-host subcommand. installCmd uses process substitution
-// (bash <(curl …)), which keeps stdin on the terminal for interactive prompts.
-// When a researcher username was given we prefix TB_PREPARE_USER=<user> so a
-// copy-pasted retry still grants access — otherwise the manual fallback would
-// silently do less than the original request (Bugbot #394).
+// run fails. It's built from installer.Script — the same single bootstrap idiom
+// we just executed — so the hint is byte-identical to the command that failed,
+// and a URL/idiom change updates every hint at once (Bugbot #394, cli#396).
+// When a researcher username was given we carry TB_PREPARE_USER=<user> into the
+// installer so a copy-pasted retry still grants access — otherwise the manual
+// fallback would silently do less than the original request (Bugbot #394).
 func prepareHostManualHint(user string) string {
-	if user != "" {
-		return "TB_PREPARE_USER=" + user + " " + installCmd + " prepare-host"
+	if user == "" {
+		return prepareHostInstallerCmd
 	}
-	return installCmd + " prepare-host"
+	return installer.Script("prepare-host", "TB_PREPARE_USER="+user)
 }
 
 // prepareHostEnv is the child's environment: the parent's, but with any ambient
@@ -110,7 +79,7 @@ func prepareHostEnv(user string) []string {
 // that traps signals. We rely on the default SIGKILL rather than a custom
 // SIGINT-only Cancel (which a privileged child could ignore, hanging Wait).
 func prepareHostCmd(ctx context.Context) *exec.Cmd {
-	c := exec.CommandContext(ctx, "bash", "-c", prepareHostInstallerCmd) // #nosec G204 -- argv is compile-time constant: literal "bash" -c installerRunScript("prepare-host"), built only from the installerURL const; no runtime input.
+	c := exec.CommandContext(ctx, "bash", "-c", prepareHostInstallerCmd) // #nosec G204 -- argv is compile-time constant: literal "bash" -c installer.Script("prepare-host", ""), built only from the installer.URL const; no runtime input.
 	c.WaitDelay = 5 * time.Second
 	return c
 }

internal/cli/prepare_host_test.go

@@ -6,6 +6,8 @@ import (
 	"os/exec"
 	"strings"
 	"testing"
+
+	"github.com/tracebloc/cli/internal/installer"
 )
 
 // A failed download must abort rather than run an empty script: with the old
@@ -28,15 +30,15 @@ func TestPrepareHostCmdFailsClosedOnDownloadError(t *testing.T) {
 // Every curl in the shared installer script must pin the TLS 1.2 floor
 // (--tlsv1.2), matching scripts/install.sh, so this privileged download can never
 // negotiate a weaker protocol. Guards both the upgrade (no subcommand) and
-// prepare-host paths since they share installerRunScript (Bugbot #397).
+// prepare-host paths since they share installer.Script (Bugbot #397).
 func TestInstallerRunScriptPinsTLSFloor(t *testing.T) {
 	for _, sub := range []string{"", "prepare-host"} {
-		script := installerRunScript(sub)
+		script := installer.Script(sub, "")
 		if !strings.Contains(script, "curl") {
-			t.Fatalf("installerRunScript(%q) must curl the installer; got: %q", sub, script)
+			t.Fatalf("installer.Script(%q) must curl the installer; got: %q", sub, script)
 		}
 		if !strings.Contains(script, "--tlsv1.2") {
-			t.Errorf("installerRunScript(%q) must pin --tlsv1.2 on the download (matches install.sh); got: %q", sub, script)
+			t.Errorf("installer.Script(%q) must pin --tlsv1.2 on the download (matches install.sh); got: %q", sub, script)
 		}
 	}
 }
@@ -172,6 +174,15 @@ func TestPrepareHostManualHint_CarriesUser(t *testing.T) {
 	}
 }
 
+// The no-username manual hint must be the EXACT command we just tried, so a user
+// pasting it reproduces the automated run rather than a lookalike that could
+// drift from it (cli#396).
+func TestPrepareHostManualHint_MatchesTheCommandWeRan(t *testing.T) {
+	if got := prepareHostManualHint(""); got != prepareHostInstallerCmd {
+		t.Errorf("manual hint = %q, want the command we executed %q", got, prepareHostInstallerCmd)
+	}
+}
+
 // prepare-host shells out to bash/curl and readies a Unix host, so it must be
 // guarded on Windows (a no-op-with-explanation, not a cryptic missing-bash
 // failure) — mirrors upgrade's Windows handling (Bugbot #394).

internal/cli/upgrade.go

@@ -7,6 +7,8 @@ import (
 	"runtime"
 
 	"github.com/spf13/cobra"
+
+	"github.com/tracebloc/cli/internal/installer"
 )
 
 // upgradeCmdName is the command that re-runs the installer; the update nudge
@@ -18,12 +20,12 @@ const upgradeCmdName = "upgrade"
 // installer ourselves: it re-downloads + cosign-verifies the release, replaces
 // the CLI, and upgrades the secure environment's services to match — so we never
 // re-implement (and risk diverging from) the installer's signature verification.
-// We download-then-execute the installer (installerRunScript, shared with
-// prepare-host) rather than `curl … | bash`: piping makes the inner bash read
-// its program from the pipe, stealing the installer's stdin so its interactive
-// prompts (sign-in, etc.) can't read the TTY. The URL is derived from
-// installerURL (doctor.go) so it can't drift from the other installer paths
-// (Bugbot #397).
+// We download-then-execute the installer (installer.Cmd, the shared bootstrap
+// idiom) rather than `curl … | bash`: piping makes the inner bash read its
+// program from the pipe, stealing the installer's stdin so its interactive
+// prompts (sign-in, etc.) can't read the TTY. Both the URL and the idiom come
+// from internal/installer so they can't drift from the other installer paths
+// (Bugbot #397, cli#396).
 //
 // Windows is different: we do NOT self-exec there. A running .exe is locked, so
 // install.ps1's Move-Item can't overwrite the very binary we're running, and
@@ -47,16 +49,17 @@ func upgradePlanFor(goos string) upgradePlan {
 	if goos == "windows" {
 		return upgradePlan{exec: false, manual: upgradeInstallerCmdWindows}
 	}
-	// Download-then-execute the verified installer (installerRunScript, shared
-	// with prepare-host): its `set -e`+`curl -o` fails closed on a bad download,
-	// and running a file (not a pipe) keeps the installer's stdin on the TTY. The
-	// manual hint reuses installCmd (doctor.go), the shared bootstrap idiom, so
-	// the URL has a single source.
+	// Download-then-execute the verified installer (installer.Cmd, shared with
+	// prepare-host and every printed remedy): its `set -e`+`curl -o` fails closed
+	// on a bad download, and running a file (not a pipe) keeps the installer's
+	// stdin on the TTY. exec and manual are deliberately the SAME string — if the
+	// run fails, the command we hand the user is byte-identical to the one that
+	// just failed.
 	return upgradePlan{
 		exec:   true,
 		name:   "bash",
-		args:   []string{"-c", installerRunScript("")},
-		manual: installCmd,
+		args:   []string{"-c", installer.Cmd},
+		manual: installer.Cmd,
 	}
 }
 
@@ -120,7 +123,7 @@ Safe to run anytime; safe to re-run.`,
 			// Stream the installer straight to the user's terminal, and keep
 			// stdin wired so its interactive prompts (sign-in, etc.) still work.
 			ctx := cmd.Context()
-			c := exec.CommandContext(ctx, plan.name, plan.args...) // #nosec G204 -- upgradePlanFor(runtime.GOOS) yields compile-time constants: "bash" -c installerRunScript(""); only the GOOS branch varies, no user input.
+			c := exec.CommandContext(ctx, plan.name, plan.args...) // #nosec G204 -- upgradePlanFor(runtime.GOOS) yields compile-time constants: "bash" -c installer.Cmd; only the GOOS branch varies, no user input.
 			c.Stdin, c.Stdout, c.Stderr = os.Stdin, os.Stdout, os.Stderr
 			if err := c.Run(); err != nil {
 				// User aborted (Ctrl-C) or the parent context was cancelled: exit

internal/cli/upgrade_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"strings"
 	"testing"
+
+	"github.com/tracebloc/cli/internal/installer"
 )
 
 // TestUpgradeCmd_Metadata pins the command's shape without running it (RunE
@@ -50,8 +52,9 @@ func TestUpgradeCmd_HelpMentionsVerified(t *testing.T) {
 // TestUpgradePlanFor_PerOS: Windows must NOT self-exec (a running .exe is locked
 // and install.ps1 is CLI-only) — it only prints the manual command. Unix runs
 // the verified installer via the shared download-then-execute script, never
-// `curl | bash` (which would steal the installer's stdin), and reuses installCmd
-// for the manual hint so the URL has one source (Bugbot #397).
+// `curl | bash` (which would steal the installer's stdin), and reuses
+// installer.Cmd for the manual hint so the URL and idiom have one source
+// (Bugbot #397, cli#396).
 func TestUpgradePlanFor_PerOS(t *testing.T) {
 	win := upgradePlanFor("windows")
 	if win.exec {
@@ -83,9 +86,11 @@ func TestUpgradePlanFor_PerOS(t *testing.T) {
 		if !strings.Contains(joined, "i.sh") {
 			t.Errorf("%s upgrade must run i.sh: %q", goos, joined)
 		}
-		// Manual hint reuses installCmd (single URL source), not a re-hardcoded URL.
-		if p.manual != installCmd {
-			t.Errorf("%s manual hint = %q, want installCmd %q", goos, p.manual, installCmd)
+		// Manual hint reuses installer.Cmd (single source for URL *and* idiom), not
+		// a re-hardcoded command — and it's the same string we exec, so the hint we
+		// print after a failed run is exactly what failed (cli#396).
+		if p.manual != installer.Cmd {
+			t.Errorf("%s manual hint = %q, want installer.Cmd %q", goos, p.manual, installer.Cmd)
 		}
 	}
 }

internal/cluster/discover.go

@@ -12,6 +12,8 @@ import (
 	"k8s.io/client-go/kubernetes"
 
 	"gopkg.in/yaml.v3"
+
+	"github.com/tracebloc/cli/internal/installer"
 )
 
 // ErrNoParentRelease is the sentinel for DiscoverParentRelease's "the namespace
@@ -128,9 +130,9 @@ func DiscoverParentRelease(ctx context.Context, cs kubernetes.Interface, namespa
 			"%w in namespace %q. "+
 				"If your client runs in another namespace, pass --namespace; "+
 				"if this cluster has no tracebloc client yet, run the installer: "+
-				"bash <(curl -fsSL https://tracebloc.io/i.sh). "+
+				"%s. "+
 				"Diagnose with `tracebloc doctor`.",
-			ErrNoParentRelease, namespace,
+			ErrNoParentRelease, namespace, installer.Cmd,
 		)
 	case 1:
 		// happy path

internal/cluster/discover_test.go

@@ -12,6 +12,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
 	k8stesting "k8s.io/client-go/testing"
+
+	"github.com/tracebloc/cli/internal/installer"
 )
 
 // jobsManagerDeployment builds the minimal Deployment the chart
@@ -186,7 +188,7 @@ func TestDiscoverParentRelease_NoReleaseFound(t *testing.T) {
 	// The error message has to be customer-actionable. Pin the
 	// key remediation phrase so a future refactor that loses it
 	// (or worse, replaces it with a stack trace) fails this test.
-	for _, want := range []string{"no tracebloc client found", "--namespace", "https://tracebloc.io/i.sh", "tracebloc doctor"} {
+	for _, want := range []string{"no tracebloc client found", "--namespace", installer.URL, "tracebloc doctor"} {
 		if !strings.Contains(err.Error(), want) {
 			t.Errorf("expected error to mention %q, got: %s", want, err)
 		}