From 671d510d677fbf17e2ae30790f43a910d499cdf7 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Wed, 22 Jul 2026 18:43:30 +0200 Subject: [PATCH 1/9] =?UTF-8?q?feat(cli):=20tracebloc=20prepare-host=20?= =?UTF-8?q?=E2=80=94=20one-time=20admin=20step=20wrapper=20(RFC=200001=20#?= =?UTF-8?q?1178)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `tracebloc prepare-host`: a thin, discoverable wrapper that re-runs the official installer's verified prepare-host step (curl … | bash -s -- prepare-host), exactly like `tracebloc upgrade` delegates to the installer rather than re-implementing privileged host prep in the CLI. Registered in root; copy catalog gains 11-prepare-host.golden + the strings flow into zz-all-strings. build/vet/ staticcheck/deadcode clean; catalog no-drift; gofmt clean. Pairs with the client installer prepare-host step (tracebloc/client#377). Co-Authored-By: Claude Opus 4.8 --- internal/cli/copy_catalog_test.go | 31 +++++++----- internal/cli/prepare_host.go | 48 +++++++++++++++++++ internal/cli/root.go | 1 + internal/cli/testdata/golden/00-home.golden | 25 +++++----- .../testdata/golden/11-prepare-host.golden | 33 +++++++++++++ .../cli/testdata/golden/zz-all-strings.golden | 2 + 6 files changed, 117 insertions(+), 23 deletions(-) create mode 100644 internal/cli/prepare_host.go create mode 100644 internal/cli/testdata/golden/11-prepare-host.golden diff --git a/internal/cli/copy_catalog_test.go b/internal/cli/copy_catalog_test.go index 433fabde..416d3c24 100644 --- a/internal/cli/copy_catalog_test.go +++ b/internal/cli/copy_catalog_test.go @@ -358,18 +358,27 @@ func TestCopyCatalog(t *testing.T) { []run{{"tracebloc version --help", help("version")}}, ) + // ── 11 prepare-host ────────────────────────────────────────────────────────── + prepareHostFile := doc( + "tb prepare-host — one-time admin step so a non-admin can install", + "What you see when you run `tb prepare-host` — the one-time administrator step\nthat readies a shared / HPC host so a non-admin user can then install tracebloc\nwith no root. It re-runs the installer's verified prepare-host step; the\nprivileged prep + its progress stream from the installer (not CLI copy). Only the\n--help is byte-exact below.", + nil, + []run{{"tracebloc prepare-host --help", help("prepare-host")}}, + ) + files := map[string]string{ - "00-home.golden": homeFile, - "01-data-ingest.golden": dataIngestFile, - "02-data-list.golden": dataListFile, - "03-data-delete.golden": dataDeleteFile, - "04-resources.golden": resourcesFile, - "05-doctor.golden": doctorFile, - "06-delete.golden": deleteFile, - "07-login.golden": loginFile, - "08-client.golden": clientFile, - "09-cluster.golden": clusterFile, - "10-version.golden": versionFile, + "00-home.golden": homeFile, + "01-data-ingest.golden": dataIngestFile, + "02-data-list.golden": dataListFile, + "03-data-delete.golden": dataDeleteFile, + "04-resources.golden": resourcesFile, + "05-doctor.golden": doctorFile, + "06-delete.golden": deleteFile, + "07-login.golden": loginFile, + "08-client.golden": clientFile, + "09-cluster.golden": clusterFile, + "10-version.golden": versionFile, + "11-prepare-host.golden": prepareHostFile, "zz-all-strings.golden": "every user-facing string in the source (AST-harvested — all arguments to the\n" + "Printer methods + errors.New/fmt.Errorf/fmt.Sprintf, plus the text/remedy\n" + "fields of healthLine{} and doctor.Result{} literals, both \"…\" and `…` raw\n" + diff --git a/internal/cli/prepare_host.go b/internal/cli/prepare_host.go new file mode 100644 index 00000000..4294a255 --- /dev/null +++ b/internal/cli/prepare_host.go @@ -0,0 +1,48 @@ +package cli + +import ( + "fmt" + "os" + "os/exec" + + "github.com/spf13/cobra" +) + +// 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. +const prepareHostInstallerCmd = "curl -fsSL https://tracebloc.io/i.sh | bash -s -- prepare-host" + +// newPrepareHostCmd builds `tracebloc prepare-host` — the one-time administrator +// step that readies a machine so a non-admin user can then install tracebloc +// with no root at all. +func newPrepareHostCmd() *cobra.Command { + return &cobra.Command{ + Use: "prepare-host", + Short: "Prepare this machine so a non-admin user can install tracebloc (run once, as an administrator)", + Long: `Prepares a host that a non-admin user can't install on directly. + +Run this ONCE, as an administrator, on a machine where the person who will use +tracebloc has no root or sudo — a shared server, an HPC login node. It installs +the container runtime and its prerequisites and grants that user access to it; +afterwards they install tracebloc with no administrator rights at all. + +It re-runs the official installer's prepare-host step (verified with cosign). It +does NOT create your secure environment or sign you in — it only prepares the +host, so it's safe to run on a shared machine. Safe to re-run.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + p := printerFor(cmd) + p.Newline() + p.Para("Preparing this host — re-running the installer's prepare-host step (installs the container runtime and prerequisites; needs administrator rights once).") + p.Newline() + c := exec.CommandContext(cmd.Context(), "bash", "-c", prepareHostInstallerCmd) + c.Stdin, c.Stdout, c.Stderr = os.Stdin, os.Stdout, os.Stderr + if err := c.Run(); err != nil { + return &exitError{code: exitFailure, err: fmt.Errorf("prepare-host didn't complete (%w). You can run the installer directly:\n %s", err, prepareHostInstallerCmd)} + } + return nil + }, + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 7954cc35..39765264 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -104,6 +104,7 @@ Helm, no YAML, no kubectl needed.`, // `client` and NOT `client delete --uninstall`: one machine owns one client, // so this removes tracebloc from the host and avoids colliding with `data delete`. root.AddCommand(newDeleteCmd()) + root.AddCommand(newPrepareHostCmd()) // Bare `tracebloc` (no subcommand) renders a status-aware home screen — // where you stand (signed in? environment live?) then the commands — diff --git a/internal/cli/testdata/golden/00-home.golden b/internal/cli/testdata/golden/00-home.golden index e2166ef9..8463b948 100644 --- a/internal/cli/testdata/golden/00-home.golden +++ b/internal/cli/testdata/golden/00-home.golden @@ -236,18 +236,19 @@ Usage: tracebloc [command] Available Commands: - auth Inspect tracebloc authentication state - client Provision this machine's tracebloc client - cluster Inspect the cluster the CLI is currently targeting - completion Generate the autocompletion script for the specified shell - data Manage the datasets in your secure environment - delete Offboard this machine from tracebloc (revoke, uninstall, reclaim disk) - doctor Check your secure environment is connected and ready to run training - help Help about any command - login Sign in to tracebloc in your browser (device flow) - logout Sign out (revoke the token server-side and clear it locally) - resources Show how much of this machine tracebloc may use - version Print the tracebloc CLI version, git SHA, and build date + auth Inspect tracebloc authentication state + client Provision this machine's tracebloc client + cluster Inspect the cluster the CLI is currently targeting + completion Generate the autocompletion script for the specified shell + data Manage the datasets in your secure environment + delete Offboard this machine from tracebloc (revoke, uninstall, reclaim disk) + doctor Check your secure environment is connected and ready to run training + help Help about any command + login Sign in to tracebloc in your browser (device flow) + logout Sign out (revoke the token server-side and clear it locally) + prepare-host Prepare this machine so a non-admin user can install tracebloc (run once, as an administrator) + resources Show how much of this machine tracebloc may use + version Print the tracebloc CLI version, git SHA, and build date Flags: -h, --help help for tracebloc diff --git a/internal/cli/testdata/golden/11-prepare-host.golden b/internal/cli/testdata/golden/11-prepare-host.golden new file mode 100644 index 00000000..e7062189 --- /dev/null +++ b/internal/cli/testdata/golden/11-prepare-host.golden @@ -0,0 +1,33 @@ +tb prepare-host — one-time admin step so a non-admin can install +================================================================ +What you see when you run `tb prepare-host` — the one-time administrator step +that readies a shared / HPC host so a non-admin user can then install tracebloc +with no root. It re-runs the installer's verified prepare-host step; the +privileged prep + its progress stream from the installer (not CLI copy). Only the +--help is byte-exact below. + + +------------------------------------------------------------ +--help +------------------------------------------------------------ +$ tracebloc prepare-host --help +Prepares a host that a non-admin user can't install on directly. + +Run this ONCE, as an administrator, on a machine where the person who will use +tracebloc has no root or sudo — a shared server, an HPC login node. It installs +the container runtime and its prerequisites and grants that user access to it; +afterwards they install tracebloc with no administrator rights at all. + +It re-runs the official installer's prepare-host step (verified with cosign). It +does NOT create your secure environment or sign you in — it only prepares the +host, so it's safe to run on a shared machine. Safe to re-run. + +Usage: + tracebloc prepare-host [flags] + +Flags: + -h, --help help for prepare-host + +Global Flags: + --plain disable color and decorative output (also honors $NO_COLOR) + --verbose stream detailed step-by-step progress (also via $TRACEBLOC_LOG_LEVEL=debug) diff --git a/internal/cli/testdata/golden/zz-all-strings.golden b/internal/cli/testdata/golden/zz-all-strings.golden index 009e7131..2ab3b017 100644 --- a/internal/cli/testdata/golden/zz-all-strings.golden +++ b/internal/cli/testdata/golden/zz-all-strings.golden @@ -218,6 +218,7 @@ screen. %s/%d are runtime placeholders. "Pending > %s: %v" "Pick this dataset when you set it up." "Please name the dataset." +"Preparing this host — re-running the installer's prepare-host step (installs the container runtime and prerequisites; needs administrator rights once)." "Private image pulls will ImagePullBackOff. Reinstall the chart with valid registry credentials." "Proceed with the ingest?" "Provision this client?" @@ -473,6 +474,7 @@ screen. %s/%d are runtime placeholders. "pod %s container %s restarted %d times" "port-forward allocated zero ports" "port-forward to %s/%s failed during startup: %w" +"prepare-host didn't complete (%w). You can run the installer directly:\n %s" "pvc path" "querying datasets: %w%s" "reading %q: %w" From e9a4f67f34a80480a8927f150058e5e127baedc0 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Thu, 23 Jul 2026 08:45:11 +0200 Subject: [PATCH 2/9] fix(cli): set -o pipefail in prepare-host so a curl failure is not swallowed (Bugbot #394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without pipefail, `curl … | bash -s -- prepare-host` under `bash -c` exits 0 when curl fails (bash reads empty stdin), so the command reported success while prepare-host never ran. Prepend `set -o pipefail;` so curl's non-zero propagates and c.Run() surfaces the failure. Regression guard added. Co-Authored-By: Claude Opus 4.8 --- internal/cli/prepare_host.go | 6 +++++- internal/cli/prepare_host_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 internal/cli/prepare_host_test.go diff --git a/internal/cli/prepare_host.go b/internal/cli/prepare_host.go index 4294a255..b750d2ef 100644 --- a/internal/cli/prepare_host.go +++ b/internal/cli/prepare_host.go @@ -12,7 +12,11 @@ import ( // 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. -const prepareHostInstallerCmd = "curl -fsSL https://tracebloc.io/i.sh | bash -s -- prepare-host" +// `set -o pipefail` is essential: without it, if curl fails (network/DNS/HTTP +// error) the downstream `bash -s` still gets empty stdin and exits 0, so the +// whole pipeline — and c.Run() — succeeds while prepare-host never ran (Bugbot +// #394). With pipefail, curl's non-zero propagates and we surface the failure. +const prepareHostInstallerCmd = "set -o pipefail; curl -fsSL https://tracebloc.io/i.sh | bash -s -- prepare-host" // newPrepareHostCmd builds `tracebloc prepare-host` — the one-time administrator // step that readies a machine so a non-admin user can then install tracebloc diff --git a/internal/cli/prepare_host_test.go b/internal/cli/prepare_host_test.go new file mode 100644 index 00000000..a9ff925c --- /dev/null +++ b/internal/cli/prepare_host_test.go @@ -0,0 +1,31 @@ +package cli + +import ( + "strings" + "testing" +) + +// The prepare-host command shells out to `bash -c prepareHostInstallerCmd`. If +// curl fails, a plain `curl | bash` pipeline still exits 0 (bash reads empty +// stdin), so the command would report success while nothing ran. `set -o +// pipefail` is what makes curl's failure propagate — guard it against removal +// (Bugbot #394). +func TestPrepareHostCmdUsesPipefail(t *testing.T) { + if !strings.HasPrefix(prepareHostInstallerCmd, "set -o pipefail;") { + t.Fatalf("prepareHostInstallerCmd must start with `set -o pipefail;` so a curl failure isn't swallowed; got: %q", prepareHostInstallerCmd) + } + if !strings.Contains(prepareHostInstallerCmd, "prepare-host") { + t.Fatalf("prepareHostInstallerCmd should invoke the installer's prepare-host step; got: %q", prepareHostInstallerCmd) + } +} + +func TestPrepareHostCmdMetadata(t *testing.T) { + c := newPrepareHostCmd() + if c.Use != "prepare-host" { + t.Errorf("Use = %q, want prepare-host", c.Use) + } + // NoArgs: prepare-host takes no positional arguments. + if err := c.Args(c, []string{"unexpected"}); err == nil { + t.Error("prepare-host should reject positional arguments (cobra.NoArgs)") + } +} From fb9b2aa170502f348bdc20434ff847719c1814ee Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Thu, 23 Jul 2026 10:27:55 +0200 Subject: [PATCH 3/9] prepare-host: run installer pipeline in its own process group Cancelling the context (Ctrl-C / parent shutdown) previously killed only the top-level `bash -c`, leaving the `curl` and the `bash -s` prepare-host child -- which performs privileged host prep -- running detached after the CLI had already reported failure and exited (Bugbot #394). Extract prepareHostCmd(ctx): set SysProcAttr.Setpgid so the pipeline gets its own process group, and a Cancel that group-signals (kill -PGID SIGINT) so the whole pipeline stops when the user aborts. Add a test asserting Setpgid + Cancel are wired. Co-Authored-By: Claude Opus 4.8 --- internal/cli/prepare_host.go | 25 ++++++++++++++++++++++++- internal/cli/prepare_host_test.go | 16 ++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/internal/cli/prepare_host.go b/internal/cli/prepare_host.go index b750d2ef..6491dcce 100644 --- a/internal/cli/prepare_host.go +++ b/internal/cli/prepare_host.go @@ -1,9 +1,11 @@ package cli import ( + "context" "fmt" "os" "os/exec" + "syscall" "github.com/spf13/cobra" ) @@ -18,6 +20,27 @@ import ( // #394). With pipefail, curl's non-zero propagates and we surface the failure. const prepareHostInstallerCmd = "set -o pipefail; curl -fsSL https://tracebloc.io/i.sh | bash -s -- prepare-host" +// prepareHostCmd builds the exec.Cmd that runs the installer pipeline. It puts +// the pipeline in its own process group (Setpgid) and, on context cancel +// (Ctrl-C / parent shutdown), signals the WHOLE group rather than just the +// top-level `bash` (Bugbot #394). Without this, cancelling the CLI killed only +// the immediate `bash -c`, leaving the `curl` and the `bash -s` prepare-host +// child — which does privileged host prep — running detached after the CLI had +// already reported failure and exited. Group-signalling stops the privileged +// work when the user aborts. +func prepareHostCmd(ctx context.Context) *exec.Cmd { + c := exec.CommandContext(ctx, "bash", "-c", prepareHostInstallerCmd) + c.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + c.Cancel = func() error { + if c.Process == nil { + return nil + } + // Negative PID → signal the entire process group we created above. + return syscall.Kill(-c.Process.Pid, syscall.SIGINT) + } + return c +} + // newPrepareHostCmd builds `tracebloc prepare-host` — the one-time administrator // step that readies a machine so a non-admin user can then install tracebloc // with no root at all. @@ -41,7 +64,7 @@ host, so it's safe to run on a shared machine. Safe to re-run.`, p.Newline() p.Para("Preparing this host — re-running the installer's prepare-host step (installs the container runtime and prerequisites; needs administrator rights once).") p.Newline() - c := exec.CommandContext(cmd.Context(), "bash", "-c", prepareHostInstallerCmd) + c := prepareHostCmd(cmd.Context()) c.Stdin, c.Stdout, c.Stderr = os.Stdin, os.Stdout, os.Stderr if err := c.Run(); err != nil { return &exitError{code: exitFailure, err: fmt.Errorf("prepare-host didn't complete (%w). You can run the installer directly:\n %s", err, prepareHostInstallerCmd)} diff --git a/internal/cli/prepare_host_test.go b/internal/cli/prepare_host_test.go index a9ff925c..a50d00ea 100644 --- a/internal/cli/prepare_host_test.go +++ b/internal/cli/prepare_host_test.go @@ -1,6 +1,7 @@ package cli import ( + "context" "strings" "testing" ) @@ -19,6 +20,21 @@ func TestPrepareHostCmdUsesPipefail(t *testing.T) { } } +// The installer runs as a `curl | bash -s` pipeline. If we only kill the +// top-level bash on cancel, the curl and the privileged prepare-host child keep +// running detached after the CLI has reported failure and exited. Guard that +// the command is created in its own process group with a group-killing Cancel +// (Bugbot #394). +func TestPrepareHostCmdRunsInOwnProcessGroup(t *testing.T) { + c := prepareHostCmd(context.Background()) + if c.SysProcAttr == nil || !c.SysProcAttr.Setpgid { + t.Error("prepareHostCmd must set SysProcAttr.Setpgid so the installer pipeline gets its own process group") + } + if c.Cancel == nil { + t.Error("prepareHostCmd must set Cancel to group-kill the pipeline when the context is cancelled") + } +} + func TestPrepareHostCmdMetadata(t *testing.T) { c := newPrepareHostCmd() if c.Use != "prepare-host" { From 7b599bed66c79fa65eace6a77fb48e270655b67f Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Thu, 23 Jul 2026 10:42:16 +0200 Subject: [PATCH 4/9] prepare-host: run installer from a temp file, not a pipe (keep stdin on TTY) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `curl | bash -s -- prepare-host` makes the inner bash read its *program* from the pipe, so the installer's stdin is no longer the terminal — any interactive prompt in prepare-host (e.g. which non-admin user gets runtime access) gets EOF (Bugbot #394, second finding). Switching to `bash <(curl …)` would fix stdin but reopen the FIRST #394 finding: process-substitution exit codes bypass pipefail, so a failed curl would silently run nothing. Instead download to a temp file under `set -e` (so a failed `curl -o` aborts) and run the file (stdin stays on the TTY). Best of both: fail-closed on download error AND interactive-capable. The manual-run hint shown on failure uses `bash <(curl …)` — the repo's recommended idiom — since a human re-running it keeps their own TTY. Add tests: fail-closed on download error (set -e + curl -o), and never pipe the script into bash. Co-Authored-By: Claude Opus 4.8 --- internal/cli/prepare_host.go | 46 +++++++++++++++++++++---------- internal/cli/prepare_host_test.go | 29 +++++++++++++------ 2 files changed, 53 insertions(+), 22 deletions(-) diff --git a/internal/cli/prepare_host.go b/internal/cli/prepare_host.go index 6491dcce..b4b258f7 100644 --- a/internal/cli/prepare_host.go +++ b/internal/cli/prepare_host.go @@ -14,20 +14,38 @@ import ( // 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. -// `set -o pipefail` is essential: without it, if curl fails (network/DNS/HTTP -// error) the downstream `bash -s` still gets empty stdin and exits 0, so the -// whole pipeline — and c.Run() — succeeds while prepare-host never ran (Bugbot -// #394). With pipefail, curl's non-zero propagates and we surface the failure. -const prepareHostInstallerCmd = "set -o pipefail; curl -fsSL https://tracebloc.io/i.sh | bash -s -- prepare-host" +// +// We download the installer to a temp file and run THAT, rather than +// `curl | bash -s`. Two reasons, both Bugbot #394: +// - stdin: with `curl | bash -s`, the inner bash reads its *program* from the +// pipe, so the installer's stdin is no longer the terminal. Any interactive +// prompt in prepare-host (e.g. 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 temp file is removed on exit. +const prepareHostInstallerCmd = `set -e +tmp="$(mktemp)" +trap 'rm -f "$tmp"' EXIT +curl -fsSL https://tracebloc.io/i.sh -o "$tmp" +bash "$tmp" prepare-host` -// prepareHostCmd builds the exec.Cmd that runs the installer pipeline. It puts -// the pipeline in its own process group (Setpgid) and, on context cancel -// (Ctrl-C / parent shutdown), signals the WHOLE group rather than just the -// top-level `bash` (Bugbot #394). Without this, cancelling the CLI killed only -// the immediate `bash -c`, leaving the `curl` and the `bash -s` prepare-host -// child — which does privileged host prep — running detached after the CLI had -// already reported failure and exited. Group-signalling stops the privileged -// work when the user aborts. +// prepareHostManualHint is the copy-pasteable command we show if the automated +// run fails. It uses process substitution (bash <(curl …)) — the idiom the rest +// of the repo recommends — so a human re-running it by hand keeps stdin on their +// terminal for any interactive prompt. +const prepareHostManualHint = "bash <(curl -fsSL https://tracebloc.io/i.sh) prepare-host" + +// prepareHostCmd builds the exec.Cmd that runs the installer. It puts the whole +// job in its own process group (Setpgid) and, on context cancel (Ctrl-C / parent +// shutdown), signals the WHOLE group rather than just the top-level `bash` +// (Bugbot #394). Without this, cancelling the CLI killed only the immediate +// `bash -c`, leaving the `curl` and the `bash "$tmp"` prepare-host child — which +// does privileged host prep — running detached after the CLI had already +// reported failure and exited. Group-signalling stops the privileged work when +// the user aborts. func prepareHostCmd(ctx context.Context) *exec.Cmd { c := exec.CommandContext(ctx, "bash", "-c", prepareHostInstallerCmd) c.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} @@ -67,7 +85,7 @@ host, so it's safe to run on a shared machine. Safe to re-run.`, c := prepareHostCmd(cmd.Context()) c.Stdin, c.Stdout, c.Stderr = os.Stdin, os.Stdout, os.Stderr if err := c.Run(); err != nil { - return &exitError{code: exitFailure, err: fmt.Errorf("prepare-host didn't complete (%w). You can run the installer directly:\n %s", err, prepareHostInstallerCmd)} + return &exitError{code: exitFailure, err: fmt.Errorf("prepare-host didn't complete (%w). You can run the installer directly:\n %s", err, prepareHostManualHint)} } return nil }, diff --git a/internal/cli/prepare_host_test.go b/internal/cli/prepare_host_test.go index a50d00ea..14b86fc7 100644 --- a/internal/cli/prepare_host_test.go +++ b/internal/cli/prepare_host_test.go @@ -6,20 +6,33 @@ import ( "testing" ) -// The prepare-host command shells out to `bash -c prepareHostInstallerCmd`. If -// curl fails, a plain `curl | bash` pipeline still exits 0 (bash reads empty -// stdin), so the command would report success while nothing ran. `set -o -// pipefail` is what makes curl's failure propagate — guard it against removal -// (Bugbot #394). -func TestPrepareHostCmdUsesPipefail(t *testing.T) { - if !strings.HasPrefix(prepareHostInstallerCmd, "set -o pipefail;") { - t.Fatalf("prepareHostInstallerCmd must start with `set -o pipefail;` so a curl failure isn't swallowed; got: %q", prepareHostInstallerCmd) +// A failed download must abort rather than run an empty script: with the old +// `curl | bash`, a curl failure left bash reading empty stdin and exiting 0, so +// the command reported success while prepare-host never ran. `set -e` + `curl +// -o ` makes the failure propagate — guard both against removal (Bugbot +// #394). +func TestPrepareHostCmdFailsClosedOnDownloadError(t *testing.T) { + if !strings.Contains(prepareHostInstallerCmd, "set -e") { + t.Fatalf("prepareHostInstallerCmd must `set -e` so a failed download aborts; got: %q", prepareHostInstallerCmd) + } + if !strings.Contains(prepareHostInstallerCmd, "curl") || !strings.Contains(prepareHostInstallerCmd, "-o ") { + t.Fatalf("prepareHostInstallerCmd must download the installer to a file (curl -o) so curl's exit is checked; got: %q", prepareHostInstallerCmd) } if !strings.Contains(prepareHostInstallerCmd, "prepare-host") { t.Fatalf("prepareHostInstallerCmd should invoke the installer's prepare-host step; got: %q", prepareHostInstallerCmd) } } +// The installer must NOT be fed to bash over a pipe: `curl | bash -s` makes the +// inner bash read its program from the pipe, stealing the installer's stdin so +// any interactive prompt in prepare-host gets EOF. We download and run a file +// instead, leaving stdin on the TTY (Bugbot #394). +func TestPrepareHostCmdDoesNotPipeIntoBash(t *testing.T) { + if strings.Contains(prepareHostInstallerCmd, "| bash") || strings.Contains(prepareHostInstallerCmd, "|bash") { + t.Errorf("prepareHostInstallerCmd must not pipe the script into bash (steals the installer's stdin); got: %q", prepareHostInstallerCmd) + } +} + // The installer runs as a `curl | bash -s` pipeline. If we only kill the // top-level bash on cancel, the curl and the privileged prepare-host child keep // running detached after the CLI has reported failure and exited. Guard that From 5eabf4101d7afd670e1dbcf10d12b991ef23f656 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Thu, 23 Jul 2026 11:18:30 +0200 Subject: [PATCH 5/9] =?UTF-8?q?prepare-host:=20fix=20Windows=20build=20?= =?UTF-8?q?=E2=80=94=20move=20POSIX=20process-group=20logic=20behind=20bui?= =?UTF-8?q?ld=20tags?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The process-group cancel added for Bugbot #394 used syscall.SysProcAttr.Setpgid and syscall.Kill(-pid, …), which are POSIX-only — the Windows build failed to compile (unknown field Setpgid; undefined syscall.Kill). Extract configureProcessGroup into build-tagged files: - prepare_host_unix.go (//go:build !windows): sets Setpgid + the group-killing Cancel (unchanged behavior on linux/darwin). - prepare_host_windows.go (//go:build windows): no-op — POSIX process groups don't exist there, and exec.CommandContext still kills the top-level process; prepare-host is a Linux host op regardless. Move the Setpgid assertion test into prepare_host_unix_test.go (also !windows-tagged) so `go test`/`go vet` compile on Windows too. Verified: `GOOS=windows go build/vet ./...` now pass for amd64 + arm64, and the native build/test/staticcheck stay green. Co-Authored-By: Claude Opus 4.8 --- internal/cli/prepare_host.go | 28 ++++++++++---------------- internal/cli/prepare_host_test.go | 16 --------------- internal/cli/prepare_host_unix.go | 28 ++++++++++++++++++++++++++ internal/cli/prepare_host_unix_test.go | 24 ++++++++++++++++++++++ internal/cli/prepare_host_windows.go | 14 +++++++++++++ 5 files changed, 77 insertions(+), 33 deletions(-) create mode 100644 internal/cli/prepare_host_unix.go create mode 100644 internal/cli/prepare_host_unix_test.go create mode 100644 internal/cli/prepare_host_windows.go diff --git a/internal/cli/prepare_host.go b/internal/cli/prepare_host.go index b4b258f7..11860f5b 100644 --- a/internal/cli/prepare_host.go +++ b/internal/cli/prepare_host.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "os/exec" - "syscall" "github.com/spf13/cobra" ) @@ -38,24 +37,19 @@ bash "$tmp" prepare-host` // terminal for any interactive prompt. const prepareHostManualHint = "bash <(curl -fsSL https://tracebloc.io/i.sh) prepare-host" -// prepareHostCmd builds the exec.Cmd that runs the installer. It puts the whole -// job in its own process group (Setpgid) and, on context cancel (Ctrl-C / parent -// shutdown), signals the WHOLE group rather than just the top-level `bash` -// (Bugbot #394). Without this, cancelling the CLI killed only the immediate -// `bash -c`, leaving the `curl` and the `bash "$tmp"` prepare-host child — which -// does privileged host prep — running detached after the CLI had already -// reported failure and exited. Group-signalling stops the privileged work when -// the user aborts. +// prepareHostCmd builds the exec.Cmd that runs the installer. On Unix, +// configureProcessGroup puts the whole job in its own process group and, on +// context cancel (Ctrl-C / parent shutdown), signals the WHOLE group rather than +// just the top-level `bash` (Bugbot #394) — otherwise cancelling the CLI killed +// only the immediate `bash -c`, leaving the `curl` and the `bash "$tmp"` +// prepare-host child (which does privileged host prep) running detached. The +// process-group primitives (syscall.Setpgid / syscall.Kill) are POSIX-only, so +// that logic lives in prepare_host_unix.go; on Windows configureProcessGroup is +// a no-op (exec.CommandContext still kills the top-level process on cancel, and +// prepare-host is a Linux host operation regardless). func prepareHostCmd(ctx context.Context) *exec.Cmd { c := exec.CommandContext(ctx, "bash", "-c", prepareHostInstallerCmd) - c.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - c.Cancel = func() error { - if c.Process == nil { - return nil - } - // Negative PID → signal the entire process group we created above. - return syscall.Kill(-c.Process.Pid, syscall.SIGINT) - } + configureProcessGroup(c) return c } diff --git a/internal/cli/prepare_host_test.go b/internal/cli/prepare_host_test.go index 14b86fc7..5cc90fef 100644 --- a/internal/cli/prepare_host_test.go +++ b/internal/cli/prepare_host_test.go @@ -1,7 +1,6 @@ package cli import ( - "context" "strings" "testing" ) @@ -33,21 +32,6 @@ func TestPrepareHostCmdDoesNotPipeIntoBash(t *testing.T) { } } -// The installer runs as a `curl | bash -s` pipeline. If we only kill the -// top-level bash on cancel, the curl and the privileged prepare-host child keep -// running detached after the CLI has reported failure and exited. Guard that -// the command is created in its own process group with a group-killing Cancel -// (Bugbot #394). -func TestPrepareHostCmdRunsInOwnProcessGroup(t *testing.T) { - c := prepareHostCmd(context.Background()) - if c.SysProcAttr == nil || !c.SysProcAttr.Setpgid { - t.Error("prepareHostCmd must set SysProcAttr.Setpgid so the installer pipeline gets its own process group") - } - if c.Cancel == nil { - t.Error("prepareHostCmd must set Cancel to group-kill the pipeline when the context is cancelled") - } -} - func TestPrepareHostCmdMetadata(t *testing.T) { c := newPrepareHostCmd() if c.Use != "prepare-host" { diff --git a/internal/cli/prepare_host_unix.go b/internal/cli/prepare_host_unix.go new file mode 100644 index 00000000..fb0eeab2 --- /dev/null +++ b/internal/cli/prepare_host_unix.go @@ -0,0 +1,28 @@ +//go:build !windows + +package cli + +import ( + "os/exec" + "syscall" +) + +// configureProcessGroup puts the installer pipeline in its own process group and, +// on context cancel (Ctrl-C / parent shutdown), signals the WHOLE group rather +// than just the top-level `bash` (Bugbot #394). Without this, cancelling the CLI +// killed only the immediate `bash -c`, leaving the `curl` and the `bash "$tmp"` +// prepare-host child — which performs privileged host prep — running detached +// after the CLI had already reported failure and exited. +// +// Unix-only: syscall.Setpgid / syscall.Kill (POSIX process groups) don't exist +// on Windows, so the Windows build gets the no-op in prepare_host_windows.go. +func configureProcessGroup(c *exec.Cmd) { + c.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + c.Cancel = func() error { + if c.Process == nil { + return nil + } + // Negative PID → signal the entire process group we created above. + return syscall.Kill(-c.Process.Pid, syscall.SIGINT) + } +} diff --git a/internal/cli/prepare_host_unix_test.go b/internal/cli/prepare_host_unix_test.go new file mode 100644 index 00000000..37723bdb --- /dev/null +++ b/internal/cli/prepare_host_unix_test.go @@ -0,0 +1,24 @@ +//go:build !windows + +package cli + +import ( + "context" + "testing" +) + +// On cancel we must group-signal the whole installer pipeline, not just the +// top-level bash, or the curl and the privileged prepare-host child keep running +// detached after the CLI has reported failure and exited. Guard that the command +// is created in its own process group with a group-killing Cancel (Bugbot #394). +// Unix-only: SysProcAttr.Setpgid doesn't exist on Windows (see +// prepare_host_windows.go, where configureProcessGroup is a no-op). +func TestPrepareHostCmdRunsInOwnProcessGroup(t *testing.T) { + c := prepareHostCmd(context.Background()) + if c.SysProcAttr == nil || !c.SysProcAttr.Setpgid { + t.Error("prepareHostCmd must set SysProcAttr.Setpgid so the installer pipeline gets its own process group") + } + if c.Cancel == nil { + t.Error("prepareHostCmd must set Cancel to group-kill the pipeline when the context is cancelled") + } +} diff --git a/internal/cli/prepare_host_windows.go b/internal/cli/prepare_host_windows.go new file mode 100644 index 00000000..1e9cda38 --- /dev/null +++ b/internal/cli/prepare_host_windows.go @@ -0,0 +1,14 @@ +//go:build windows + +package cli + +import "os/exec" + +// configureProcessGroup is a no-op on Windows: POSIX process groups +// (syscall.Setpgid / syscall.Kill with a negative PID) don't exist there. That's +// acceptable — prepare-host is a Linux host operation that shells out to bash, so +// the Windows path is only reachable under WSL/Git-bash, and exec.CommandContext +// already terminates the top-level process on cancel. The richer group-signalling +// that stops detached privileged children lives in prepare_host_unix.go (Bugbot +// #394). +func configureProcessGroup(c *exec.Cmd) {} From 1f64bd64cb6ef1af4888a0023eec5bc7315d6947 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Thu, 23 Jul 2026 11:38:45 +0200 Subject: [PATCH 6/9] prepare-host: run in the foreground group, quiet interrupt on cancel (Bugbot #394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The process-group approach was wrong for an interactive command and Bugbot flagged five follow-ups: - Setpgid put the installer in its own (background) process group while stdin was the TTY, so any prepare-host prompt got SIGTTIN and hung (High). - the SIGINT-only Cancel with no WaitDelay could hang Wait forever if a privileged child ignored the signal, re-opening the orphaned-work risk. - a user Ctrl-C was wrapped as "prepare-host didn't complete — retry" instead of a quiet interrupt. Drop Setpgid and the custom Cancel entirely: keep the installer in the CLI's foreground process group so prompts work and a terminal Ctrl-C signals the whole pipeline (bash + curl + child) at once. Add WaitDelay=5s so a programmatic cancel can't hang Wait. Treat ctx cancel as exitInterrupted (130), matching the other cancellable paths. This also removes all syscall usage, so the Windows build no longer needs the build-tagged split (files deleted). Co-Authored-By: Claude Opus 4.8 --- internal/cli/prepare_host.go | 37 +++++++++++++++++--------- internal/cli/prepare_host_test.go | 17 ++++++++++++ internal/cli/prepare_host_unix.go | 28 ------------------- internal/cli/prepare_host_unix_test.go | 24 ----------------- internal/cli/prepare_host_windows.go | 14 ---------- 5 files changed, 42 insertions(+), 78 deletions(-) delete mode 100644 internal/cli/prepare_host_unix.go delete mode 100644 internal/cli/prepare_host_unix_test.go delete mode 100644 internal/cli/prepare_host_windows.go diff --git a/internal/cli/prepare_host.go b/internal/cli/prepare_host.go index 11860f5b..753ca78b 100644 --- a/internal/cli/prepare_host.go +++ b/internal/cli/prepare_host.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "os/exec" + "time" "github.com/spf13/cobra" ) @@ -37,19 +38,24 @@ bash "$tmp" prepare-host` // terminal for any interactive prompt. const prepareHostManualHint = "bash <(curl -fsSL https://tracebloc.io/i.sh) prepare-host" -// prepareHostCmd builds the exec.Cmd that runs the installer. On Unix, -// configureProcessGroup puts the whole job in its own process group and, on -// context cancel (Ctrl-C / parent shutdown), signals the WHOLE group rather than -// just the top-level `bash` (Bugbot #394) — otherwise cancelling the CLI killed -// only the immediate `bash -c`, leaving the `curl` and the `bash "$tmp"` -// prepare-host child (which does privileged host prep) running detached. The -// process-group primitives (syscall.Setpgid / syscall.Kill) are POSIX-only, so -// that logic lives in prepare_host_unix.go; on Windows configureProcessGroup is -// a no-op (exec.CommandContext still kills the top-level process on cancel, and -// prepare-host is a Linux host operation regardless). +// prepareHostCmd builds the exec.Cmd that runs the installer. +// +// It deliberately does NOT put the installer in its own process group. The +// installer is interactive (prepare-host may prompt, e.g. which non-admin user +// gets runtime access), and stdin is the TTY — a child in a *background* process +// group that reads the terminal gets SIGTTIN and hangs (Bugbot #394). Staying in +// the CLI's foreground group means prompts work AND a terminal Ctrl-C delivers +// SIGINT to the whole pipeline (the `bash -c`, the `curl`, and the `bash "$tmp"` +// prepare-host child) in one go — no orphaned privileged work. +// +// WaitDelay bounds teardown on a *programmatic* cancel (parent shutdown / a +// SIGTERM to the CLI alone): CommandContext SIGKILLs the process and, after the +// delay, force-closes the I/O pipes so Wait can't block forever behind a child +// 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) - configureProcessGroup(c) + c.WaitDelay = 5 * time.Second return c } @@ -76,9 +82,16 @@ host, so it's safe to run on a shared machine. Safe to re-run.`, p.Newline() p.Para("Preparing this host — re-running the installer's prepare-host step (installs the container runtime and prerequisites; needs administrator rights once).") p.Newline() - c := prepareHostCmd(cmd.Context()) + ctx := cmd.Context() + c := prepareHostCmd(ctx) 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 + // quietly with 130 like the other cancellable paths, not a scary + // "prepare-host didn't complete — retry" (Bugbot #394). + if ctx.Err() != nil { + return &exitError{code: exitInterrupted} + } return &exitError{code: exitFailure, err: fmt.Errorf("prepare-host didn't complete (%w). You can run the installer directly:\n %s", err, prepareHostManualHint)} } return nil diff --git a/internal/cli/prepare_host_test.go b/internal/cli/prepare_host_test.go index 5cc90fef..9f5f86e8 100644 --- a/internal/cli/prepare_host_test.go +++ b/internal/cli/prepare_host_test.go @@ -1,6 +1,7 @@ package cli import ( + "context" "strings" "testing" ) @@ -32,6 +33,22 @@ func TestPrepareHostCmdDoesNotPipeIntoBash(t *testing.T) { } } +// The installer must run in the CLI's foreground process group (NOT its own): +// it's interactive and stdin is the TTY, so a backgrounded group would get +// SIGTTIN and hang on any prompt. And WaitDelay must be positive so a child that +// traps signals can't hang Wait forever after a programmatic cancel (Bugbot +// #394). SysProcAttr==nil is portable (the field is *syscall.SysProcAttr on +// every OS), so this stays a single cross-platform test. +func TestPrepareHostCmdStaysInForegroundGroup(t *testing.T) { + c := prepareHostCmd(context.Background()) + if c.SysProcAttr != nil { + t.Error("prepareHostCmd must not set SysProcAttr — a separate/background process group breaks interactive TTY prompts (SIGTTIN)") + } + if c.WaitDelay <= 0 { + t.Error("prepareHostCmd must set a positive WaitDelay so Wait can't hang forever after a cancel") + } +} + func TestPrepareHostCmdMetadata(t *testing.T) { c := newPrepareHostCmd() if c.Use != "prepare-host" { diff --git a/internal/cli/prepare_host_unix.go b/internal/cli/prepare_host_unix.go deleted file mode 100644 index fb0eeab2..00000000 --- a/internal/cli/prepare_host_unix.go +++ /dev/null @@ -1,28 +0,0 @@ -//go:build !windows - -package cli - -import ( - "os/exec" - "syscall" -) - -// configureProcessGroup puts the installer pipeline in its own process group and, -// on context cancel (Ctrl-C / parent shutdown), signals the WHOLE group rather -// than just the top-level `bash` (Bugbot #394). Without this, cancelling the CLI -// killed only the immediate `bash -c`, leaving the `curl` and the `bash "$tmp"` -// prepare-host child — which performs privileged host prep — running detached -// after the CLI had already reported failure and exited. -// -// Unix-only: syscall.Setpgid / syscall.Kill (POSIX process groups) don't exist -// on Windows, so the Windows build gets the no-op in prepare_host_windows.go. -func configureProcessGroup(c *exec.Cmd) { - c.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - c.Cancel = func() error { - if c.Process == nil { - return nil - } - // Negative PID → signal the entire process group we created above. - return syscall.Kill(-c.Process.Pid, syscall.SIGINT) - } -} diff --git a/internal/cli/prepare_host_unix_test.go b/internal/cli/prepare_host_unix_test.go deleted file mode 100644 index 37723bdb..00000000 --- a/internal/cli/prepare_host_unix_test.go +++ /dev/null @@ -1,24 +0,0 @@ -//go:build !windows - -package cli - -import ( - "context" - "testing" -) - -// On cancel we must group-signal the whole installer pipeline, not just the -// top-level bash, or the curl and the privileged prepare-host child keep running -// detached after the CLI has reported failure and exited. Guard that the command -// is created in its own process group with a group-killing Cancel (Bugbot #394). -// Unix-only: SysProcAttr.Setpgid doesn't exist on Windows (see -// prepare_host_windows.go, where configureProcessGroup is a no-op). -func TestPrepareHostCmdRunsInOwnProcessGroup(t *testing.T) { - c := prepareHostCmd(context.Background()) - if c.SysProcAttr == nil || !c.SysProcAttr.Setpgid { - t.Error("prepareHostCmd must set SysProcAttr.Setpgid so the installer pipeline gets its own process group") - } - if c.Cancel == nil { - t.Error("prepareHostCmd must set Cancel to group-kill the pipeline when the context is cancelled") - } -} diff --git a/internal/cli/prepare_host_windows.go b/internal/cli/prepare_host_windows.go deleted file mode 100644 index 1e9cda38..00000000 --- a/internal/cli/prepare_host_windows.go +++ /dev/null @@ -1,14 +0,0 @@ -//go:build windows - -package cli - -import "os/exec" - -// configureProcessGroup is a no-op on Windows: POSIX process groups -// (syscall.Setpgid / syscall.Kill with a negative PID) don't exist there. That's -// acceptable — prepare-host is a Linux host operation that shells out to bash, so -// the Windows path is only reachable under WSL/Git-bash, and exec.CommandContext -// already terminates the top-level process on cancel. The richer group-signalling -// that stops detached privileged children lives in prepare_host_unix.go (Bugbot -// #394). -func configureProcessGroup(c *exec.Cmd) {} From 447cadc1d97f3766c2cd72d86a998d2ef074e707 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Thu, 23 Jul 2026 11:52:40 +0200 Subject: [PATCH 7/9] prepare-host: reuse installCmd for the hint + robust interrupt detection (Bugbot #394) - prepareHostManualHint duplicated the bootstrap idiom owned by installCmd (doctor.go), so a URL/idiom change could leave the prepare-host fallback stale. Build it from installCmd + " prepare-host" (same value, one source). - Interrupt detection only checked ctx.Err() after c.Run(); on a terminal Ctrl-C the child dies (bash exits 130) and Run can return before NotifyContext flips ctx.Err(), so an abort was mis-reported as a failed install. Add prepareHostInterrupted(ctx, err): interrupt if ctx cancelled OR the run exited 130 (128+SIGINT). Unit-tested (cancelled ctx, exit 130, exit 1). Co-Authored-By: Claude Opus 4.8 --- internal/cli/prepare_host.go | 29 ++++++++++++++++++++++++----- internal/cli/prepare_host_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/internal/cli/prepare_host.go b/internal/cli/prepare_host.go index 753ca78b..e18fc1ae 100644 --- a/internal/cli/prepare_host.go +++ b/internal/cli/prepare_host.go @@ -2,6 +2,7 @@ package cli import ( "context" + "errors" "fmt" "os" "os/exec" @@ -33,10 +34,11 @@ curl -fsSL https://tracebloc.io/i.sh -o "$tmp" bash "$tmp" prepare-host` // prepareHostManualHint is the copy-pasteable command we show if the automated -// run fails. It uses process substitution (bash <(curl …)) — the idiom the rest -// of the repo recommends — so a human re-running it by hand keeps stdin on their -// terminal for any interactive prompt. -const prepareHostManualHint = "bash <(curl -fsSL https://tracebloc.io/i.sh) prepare-host" +// 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. +const prepareHostManualHint = installCmd + " prepare-host" // prepareHostCmd builds the exec.Cmd that runs the installer. // @@ -59,6 +61,23 @@ func prepareHostCmd(ctx context.Context) *exec.Cmd { return c } +// prepareHostInterrupted reports whether the installer run ended because the user +// aborted, so the caller can exit quietly (130) instead of framing it as a failed +// install. ctx.Err() catches a cancel the signal handler already propagated — but +// on a terminal Ctrl-C the child can die and c.Run() can return BEFORE +// NotifyContext flips ctx.Err() (a race), so also treat bash's 130 (128+SIGINT) +// exit as an interrupt (Bugbot #394). +func prepareHostInterrupted(ctx context.Context, runErr error) bool { + if ctx.Err() != nil { + return true + } + var ee *exec.ExitError + if errors.As(runErr, &ee) { + return ee.ExitCode() == exitInterrupted + } + return false +} + // newPrepareHostCmd builds `tracebloc prepare-host` — the one-time administrator // step that readies a machine so a non-admin user can then install tracebloc // with no root at all. @@ -89,7 +108,7 @@ host, so it's safe to run on a shared machine. Safe to re-run.`, // User aborted (Ctrl-C) or the parent context was cancelled: exit // quietly with 130 like the other cancellable paths, not a scary // "prepare-host didn't complete — retry" (Bugbot #394). - if ctx.Err() != nil { + if prepareHostInterrupted(ctx, err) { return &exitError{code: exitInterrupted} } return &exitError{code: exitFailure, err: fmt.Errorf("prepare-host didn't complete (%w). You can run the installer directly:\n %s", err, prepareHostManualHint)} diff --git a/internal/cli/prepare_host_test.go b/internal/cli/prepare_host_test.go index 9f5f86e8..e031c3c2 100644 --- a/internal/cli/prepare_host_test.go +++ b/internal/cli/prepare_host_test.go @@ -2,6 +2,8 @@ package cli import ( "context" + "errors" + "os/exec" "strings" "testing" ) @@ -49,6 +51,34 @@ func TestPrepareHostCmdStaysInForegroundGroup(t *testing.T) { } } +// A user abort must be detected as an interrupt even when NotifyContext hasn't +// flipped ctx.Err() yet — bash exits 130 on SIGINT and c.Run() can return first +// (Bugbot #394). A genuine failure (exit 1) must NOT be treated as an interrupt. +func TestPrepareHostInterrupted(t *testing.T) { + // Cancelled context → interrupt regardless of the run error. + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if !prepareHostInterrupted(ctx, errors.New("boom")) { + t.Error("a cancelled context must be treated as an interrupt") + } + + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash not available for the exit-code cases") + } + // Live context + bash exit 130 (128+SIGINT) → interrupt (the Ctrl-C race). + if err := exec.Command("bash", "-c", "exit 130").Run(); err == nil { + t.Fatal("expected a non-nil error from exit 130") + } else if !prepareHostInterrupted(context.Background(), err) { + t.Error("exit 130 with a live context must be treated as an interrupt") + } + // Live context + a normal failure (exit 1) → NOT an interrupt. + if err := exec.Command("bash", "-c", "exit 1").Run(); err == nil { + t.Fatal("expected a non-nil error from exit 1") + } else if prepareHostInterrupted(context.Background(), err) { + t.Error("exit 1 must NOT be treated as an interrupt") + } +} + func TestPrepareHostCmdMetadata(t *testing.T) { c := newPrepareHostCmd() if c.Use != "prepare-host" { From 93ac409ac6933629c47b64ca10accefb1cecbb9e Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Thu, 23 Jul 2026 12:40:09 +0200 Subject: [PATCH 8/9] prepare-host: accept a researcher username to grant docker-group access (Divya #377) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit prepare-host was cobra.NoArgs with no flag and no mention of TB_PREPARE_USER, so an admin had no discoverable way to name the researcher — the installer's docker-group grant (which reads TB_PREPARE_USER) was always skipped and the feature couldn't deliver Tier-0 access end-to-end. Add an optional positional arg: `tracebloc prepare-host ` (MaximumNArgs(1)). The username is validated (Linux-username shape) and passed to the installer via the TB_PREPARE_USER environment variable (not the command string, so it can't be shell-interpreted; the installer quotes it for usermod). Help text explains the arg and stresses it's the researcher, not the admin. Without it, prepare-host installs prereqs only and prints how to grant access. The client installer already grants ONLY TB_PREPARE_USER (never $SUDO_USER), so this closes the loop. Co-Authored-By: Claude Opus 4.8 --- internal/cli/prepare_host.go | 42 +++++++++++++++---- internal/cli/prepare_host_test.go | 35 +++++++++++++--- .../testdata/golden/11-prepare-host.golden | 15 +++++-- .../cli/testdata/golden/zz-all-strings.golden | 4 +- 4 files changed, 80 insertions(+), 16 deletions(-) diff --git a/internal/cli/prepare_host.go b/internal/cli/prepare_host.go index e18fc1ae..e4112f5f 100644 --- a/internal/cli/prepare_host.go +++ b/internal/cli/prepare_host.go @@ -6,11 +6,18 @@ import ( "fmt" "os" "os/exec" + "regexp" "time" "github.com/spf13/cobra" ) +// prepareHostUserRe validates the researcher username before we pass it to the +// installer as TB_PREPARE_USER. Conservative Linux-username shape: starts +// alphanumeric, then letters/digits/._- (usermod quotes it, but reject nonsense +// 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}$`) + // 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 @@ -83,27 +90,48 @@ func prepareHostInterrupted(ctx context.Context, runErr error) bool { // with no root at all. func newPrepareHostCmd() *cobra.Command { return &cobra.Command{ - Use: "prepare-host", + Use: "prepare-host [researcher-username]", Short: "Prepare this machine so a non-admin user can install tracebloc (run once, as an administrator)", Long: `Prepares a host that a non-admin user can't install on directly. Run this ONCE, as an administrator, on a machine where the person who will use tracebloc has no root or sudo — a shared server, an HPC login node. It installs -the container runtime and its prerequisites and grants that user access to it; -afterwards they install tracebloc with no administrator rights at all. +the container runtime and its prerequisites. + +Pass that person's username to also grant them container-runtime (docker-group) +access, so they can then install tracebloc at Tier 0 with no administrator +rights at all: + + sudo tracebloc prepare-host alice + +Without a username it installs only the runtime + prerequisites and tells you +how to grant a user access afterwards. NOTE: the username is the RESEARCHER who +will use tracebloc — not you, the admin running this. It re-runs the official installer's prepare-host step (verified with cosign). It does NOT create your secure environment or sign you in — it only prepares the host, so it's safe to run on a shared machine. Safe to re-run.`, - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { p := printerFor(cmd) p.Newline() - p.Para("Preparing this host — re-running the installer's prepare-host step (installs the container runtime and prerequisites; needs administrator rights once).") - p.Newline() ctx := cmd.Context() c := prepareHostCmd(ctx) c.Stdin, c.Stdout, c.Stderr = os.Stdin, os.Stdout, os.Stderr + if len(args) == 1 { + user := args[0] + if !prepareHostUserRe.MatchString(user) { + return &exitError{code: exitBadInput, err: fmt.Errorf("invalid username %q — expected a Linux username (letters, digits, '.', '_', '-')", user)} + } + // The installer reads TB_PREPARE_USER to pick who gets docker-group + // access. Pass it through the environment (not the command string), so + // it can't be shell-interpreted; the installer quotes it for usermod. + c.Env = append(os.Environ(), "TB_PREPARE_USER="+user) + p.Para(fmt.Sprintf("Preparing this host and granting %s container-runtime access — re-running the installer's prepare-host step (needs administrator rights once).", user)) + } else { + p.Para("Preparing this host — re-running the installer's prepare-host step (installs the container runtime and prerequisites; needs administrator rights once). Pass a researcher's username to also grant them access: tracebloc prepare-host ") + } + p.Newline() if err := c.Run(); err != nil { // User aborted (Ctrl-C) or the parent context was cancelled: exit // quietly with 130 like the other cancellable paths, not a scary diff --git a/internal/cli/prepare_host_test.go b/internal/cli/prepare_host_test.go index e031c3c2..41107c64 100644 --- a/internal/cli/prepare_host_test.go +++ b/internal/cli/prepare_host_test.go @@ -81,11 +81,36 @@ func TestPrepareHostInterrupted(t *testing.T) { func TestPrepareHostCmdMetadata(t *testing.T) { c := newPrepareHostCmd() - if c.Use != "prepare-host" { - t.Errorf("Use = %q, want prepare-host", c.Use) + if !strings.HasPrefix(c.Use, "prepare-host") { + t.Errorf("Use = %q, want it to start with prepare-host", c.Use) } - // NoArgs: prepare-host takes no positional arguments. - if err := c.Args(c, []string{"unexpected"}); err == nil { - t.Error("prepare-host should reject positional arguments (cobra.NoArgs)") + // MaximumNArgs(1): the optional researcher username. Zero or one arg is fine; + // two is rejected (Bugbot / Divya #377: name the researcher to grant access). + if err := c.Args(c, []string{}); err != nil { + t.Errorf("prepare-host must accept zero args: %v", err) + } + if err := c.Args(c, []string{"alice"}); err != nil { + t.Errorf("prepare-host must accept one username arg: %v", err) + } + if err := c.Args(c, []string{"alice", "bob"}); err == nil { + t.Error("prepare-host should reject more than one positional argument") + } +} + +// The researcher username is passed to the installer as TB_PREPARE_USER, so it +// must be validated: accept real Linux usernames, reject shell-metacharacter / +// empty / overlong input (Divya #377). +func TestPrepareHostUserValidation(t *testing.T) { + valid := []string{"alice", "bob123", "a.b_c-d", "R2D2", "svc_account"} + for _, u := range valid { + if !prepareHostUserRe.MatchString(u) { + t.Errorf("username %q should be valid", u) + } + } + invalid := []string{"", "-leading", ".dot", "has space", "semi;colon", "a/b", "$(whoami)", "a`b`", "toolong_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"} + for _, u := range invalid { + if prepareHostUserRe.MatchString(u) { + t.Errorf("username %q should be rejected", u) + } } } diff --git a/internal/cli/testdata/golden/11-prepare-host.golden b/internal/cli/testdata/golden/11-prepare-host.golden index e7062189..ec5fcda5 100644 --- a/internal/cli/testdata/golden/11-prepare-host.golden +++ b/internal/cli/testdata/golden/11-prepare-host.golden @@ -15,15 +15,24 @@ Prepares a host that a non-admin user can't install on directly. Run this ONCE, as an administrator, on a machine where the person who will use tracebloc has no root or sudo — a shared server, an HPC login node. It installs -the container runtime and its prerequisites and grants that user access to it; -afterwards they install tracebloc with no administrator rights at all. +the container runtime and its prerequisites. + +Pass that person's username to also grant them container-runtime (docker-group) +access, so they can then install tracebloc at Tier 0 with no administrator +rights at all: + + sudo tracebloc prepare-host alice + +Without a username it installs only the runtime + prerequisites and tells you +how to grant a user access afterwards. NOTE: the username is the RESEARCHER who +will use tracebloc — not you, the admin running this. It re-runs the official installer's prepare-host step (verified with cosign). It does NOT create your secure environment or sign you in — it only prepares the host, so it's safe to run on a shared machine. Safe to re-run. Usage: - tracebloc prepare-host [flags] + tracebloc prepare-host [researcher-username] [flags] Flags: -h, --help help for prepare-host diff --git a/internal/cli/testdata/golden/zz-all-strings.golden b/internal/cli/testdata/golden/zz-all-strings.golden index 2ab3b017..eb401097 100644 --- a/internal/cli/testdata/golden/zz-all-strings.golden +++ b/internal/cli/testdata/golden/zz-all-strings.golden @@ -218,7 +218,8 @@ screen. %s/%d are runtime placeholders. "Pending > %s: %v" "Pick this dataset when you set it up." "Please name the dataset." -"Preparing this host — re-running the installer's prepare-host step (installs the container runtime and prerequisites; needs administrator rights once)." +"Preparing this host and granting %s container-runtime access — re-running the installer's prepare-host step (needs administrator rights once)." +"Preparing this host — re-running the installer's prepare-host step (installs the container runtime and prerequisites; needs administrator rights once). Pass a researcher's username to also grant them access: tracebloc prepare-host " "Private image pulls will ImagePullBackOff. Reinstall the chart with valid registry credentials." "Proceed with the ingest?" "Provision this client?" @@ -411,6 +412,7 @@ screen. %s/%d are runtime placeholders. "interactive setup: %w" "internal: re-parsing synthesized spec: %w\n%s" "invalid table name %q: %w" +"invalid username %q — expected a Linux username (letters, digits, '.', '_', '-')" "jobs-manager" "jobs-manager %s returned HTTP %d: %s" "jobs-manager has no literal REQUESTS_PROXY_URL (chart too old, or it's set via a configMap/secret ref)" From 4d9833f831dff0f2284285e90c25b0e83d322761 Mon Sep 17 00:00:00 2001 From: shujaat hasan Date: Thu, 23 Jul 2026 17:07:41 +0200 Subject: [PATCH 9/9] fix(cli): guard prepare-host on Windows (Bugbot #394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit prepare-host shells out to bash/curl/mktemp and readies a Linux server / HPC login node (container runtime, docker group) — a Unix-only concept. On Windows it appeared in --help then failed with a cryptic missing-bash error and a Unix-only retry hint. It now stops early with a clear explanation (a no-op-with-message), mirroring upgrade's Windows handling. Adds a regression test for the OS guard. Co-Authored-By: Claude Opus 4.8 --- internal/cli/prepare_host.go | 16 ++++++++++++++++ internal/cli/prepare_host_test.go | 14 ++++++++++++++ .../cli/testdata/golden/zz-all-strings.golden | 1 + 3 files changed, 31 insertions(+) diff --git a/internal/cli/prepare_host.go b/internal/cli/prepare_host.go index 1df8b649..6f1757f5 100644 --- a/internal/cli/prepare_host.go +++ b/internal/cli/prepare_host.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "regexp" + "runtime" "strings" "time" @@ -116,6 +117,14 @@ func prepareHostInterrupted(ctx context.Context, runErr error) bool { return false } +// prepareHostUnsupportedOnOS reports whether prepare-host can't run on this OS. +// The step readies a Linux server / HPC login node (container runtime, docker +// group) — a Unix-only concept — and shells out to bash/curl/mktemp. On Windows +// that would fail with a cryptic missing-bash error and a Unix-only retry hint, +// so we stop early with a clear message instead (mirrors upgrade's Windows +// handling; Bugbot #394). +func prepareHostUnsupportedOnOS(goos string) bool { return goos == "windows" } + // newPrepareHostCmd builds `tracebloc prepare-host` — the one-time administrator // step that readies a machine so a non-admin user can then install tracebloc // with no root at all. @@ -146,6 +155,13 @@ host, so it's safe to run on a shared machine. Safe to re-run.`, RunE: func(cmd *cobra.Command, args []string) error { p := printerFor(cmd) p.Newline() + if prepareHostUnsupportedOnOS(runtime.GOOS) { + // Discoverable in --help everywhere, but a no-op-with-explanation + // here rather than a cryptic missing-bash failure (Bugbot #394). + p.Para("prepare-host readies a Linux server or HPC login node so a non-admin user can install tracebloc without root — it doesn't apply to Windows. Run it as an administrator on the Unix host the researcher will use.") + p.Newline() + return nil + } ctx := cmd.Context() c := prepareHostCmd(ctx) c.Stdin, c.Stdout, c.Stderr = os.Stdin, os.Stdout, os.Stderr diff --git a/internal/cli/prepare_host_test.go b/internal/cli/prepare_host_test.go index 717b3668..a8ee58e1 100644 --- a/internal/cli/prepare_host_test.go +++ b/internal/cli/prepare_host_test.go @@ -155,3 +155,17 @@ func TestPrepareHostManualHint_CarriesUser(t *testing.T) { t.Errorf("hint must invoke prepare-host: %q", h) } } + +// 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). +func TestPrepareHostUnsupportedOnWindows(t *testing.T) { + if !prepareHostUnsupportedOnOS("windows") { + t.Error("prepare-host must be guarded on windows") + } + for _, goos := range []string{"linux", "darwin"} { + if prepareHostUnsupportedOnOS(goos) { + t.Errorf("prepare-host must run on %s", goos) + } + } +} diff --git a/internal/cli/testdata/golden/zz-all-strings.golden b/internal/cli/testdata/golden/zz-all-strings.golden index 8c1bdc00..50841a51 100644 --- a/internal/cli/testdata/golden/zz-all-strings.golden +++ b/internal/cli/testdata/golden/zz-all-strings.golden @@ -483,6 +483,7 @@ screen. %s/%d are runtime placeholders. "port-forward allocated zero ports" "port-forward to %s/%s failed during startup: %w" "prepare-host didn't complete (%w). You can run the installer directly:\n %s" +"prepare-host readies a Linux server or HPC login node so a non-admin user can install tracebloc without root — it doesn't apply to Windows. Run it as an administrator on the Unix host the researcher will use." "pvc path" "querying datasets: %w%s" "reading %q: %w"