fix: Bugbot findings from promotion PR #397 (TLS floor + delete cache)

internal/cli/delete.go

@@ -44,6 +44,9 @@ type deleteOpts struct {
 	namespace       string
 }
 
+// deleteCmdName is the top-level offboard command's name.
+const deleteCmdName = "delete"
+
 // newDeleteCmd wires the TOP-LEVEL `tracebloc delete` — offboarding this machine
 // (RFC-0001 §7.10). It is deliberately NOT under `client` and NOT
 // `client delete --uninstall`: on the single-machine CLI the host owns exactly
@@ -64,8 +67,15 @@ type deleteOpts struct {
 func newDeleteCmd() *cobra.Command {
 	var o deleteOpts
 	cmd := &cobra.Command{
-		Use:   "delete",
-		Short: "Offboard this machine from tracebloc (revoke, uninstall, reclaim disk)",
+		Use: deleteCmdName,
+		// Suppress the post-command update nudge/cache-write: offboarding removes
+		// the CLI and (on the default path) wipes ~/.tracebloc, so nudging to
+		// upgrade a just-removed binary is nonsensical and the cache write would
+		// resurrect the just-wiped data dir. The annotation (not a name match) means
+		// `data delete` is unaffected — only this top-level command opts in
+		// (Bugbot #397, #404). See SkipUpdateNudge.
+		Annotations: map[string]string{skipUpdateNudgeAnnotation: "true"},
+		Short:       "Offboard this machine from tracebloc (revoke, uninstall, reclaim disk)",
 		Long: `Removes tracebloc from this machine: revokes the machine credential,
 uninstalls the Helm release, deletes the local cluster, reclaims the tracebloc
 container images, and clears local state — then removes the CLI itself.

internal/cli/prepare_host.go

@@ -35,8 +35,11 @@ var prepareHostUserRe = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,31}$`)
 //     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. The URL comes from installerURL (doctor.go)
-// so every installer path shares one source and can't drift (Bugbot #394/#397).
+// 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 != "" {
@@ -45,7 +48,7 @@ func installerRunScript(subcommand string) string {
 	return `set -e
 tmp="$(mktemp)"
 trap 'rm -f "$tmp"' EXIT
-curl -fsSL ` + installerURL + ` -o "$tmp"
+curl -fsSL --tlsv1.2 ` + installerURL + ` -o "$tmp"
 ` + run
 }
 

internal/cli/prepare_host_test.go

@@ -25,6 +25,22 @@ 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).
+func TestInstallerRunScriptPinsTLSFloor(t *testing.T) {
+	for _, sub := range []string{"", "prepare-host"} {
+		script := installerRunScript(sub)
+		if !strings.Contains(script, "curl") {
+			t.Fatalf("installerRunScript(%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)
+		}
+	}
+}
+
 // 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

internal/cli/update_check.go

@@ -149,12 +149,20 @@ func readUpdateCache(path string) (updateCache, bool) {
 	return c, true
 }
 
+// writeUpdateCache persists the throttle state next to the config. Best-effort
+// (every caller ignores the error) and, critically, it must NOT create the
+// tracebloc data directory: after `tracebloc delete` wipes ~/.tracebloc, the
+// post-command update check still runs, and an MkdirAll here would resurrect the
+// just-offboarded host data dir (Bugbot #397; RFC-0003 offboard hygiene). So we
+// only write when the dir already exists — its lifecycle is owned by
+// login/client-create and delete, never by a throttle cache. A missing dir is a
+// silent no-op (the throttle simply isn't persisted until the dir exists again).
 func writeUpdateCache(path string, c updateCache) error {
 	if path == "" {
 		return nil
 	}
-	if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
-		return err
+	if _, err := os.Stat(filepath.Dir(path)); err != nil {
+		return nil // dir gone (fresh machine, or just-offboarded) — don't recreate it
 	}
 	raw, err := json.Marshal(c)
 	if err != nil {

internal/cli/update_check_test.go

@@ -54,6 +54,34 @@ func TestUpdateCacheRoundTrip(t *testing.T) {
 	}
 }
 
+// writeUpdateCache must never create the config dir: after `tracebloc delete`
+// wipes ~/.tracebloc, the post-command update check still runs, and recreating
+// the dir to write the throttle cache would resurrect the just-offboarded host
+// data dir. A missing dir is a silent no-op — no file, no dir (Bugbot #397).
+func TestWriteUpdateCache_DoesNotResurrectMissingDir(t *testing.T) {
+	base := t.TempDir()
+	gone := filepath.Join(base, "wiped") // deliberately never created
+	path := filepath.Join(gone, updateCacheFile)
+
+	if err := writeUpdateCache(path, updateCache{CheckedAt: time.Now(), Latest: "0.9.9"}); err != nil {
+		t.Fatalf("writeUpdateCache to a missing dir must be a silent no-op, got err: %v", err)
+	}
+	if _, err := os.Stat(gone); !os.IsNotExist(err) {
+		t.Errorf("writeUpdateCache must not create the missing dir %s (err=%v)", gone, err)
+	}
+	if _, err := os.Stat(path); !os.IsNotExist(err) {
+		t.Errorf("writeUpdateCache must not write a cache file into a missing dir: %v", err)
+	}
+
+	// Sanity: with the dir present it still writes (round-trips).
+	if err := writeUpdateCache(filepath.Join(base, updateCacheFile), updateCache{CheckedAt: time.Now(), Latest: "1.0.0"}); err != nil {
+		t.Fatalf("writeUpdateCache into an existing dir must succeed: %v", err)
+	}
+	if _, ok := readUpdateCache(filepath.Join(base, updateCacheFile)); !ok {
+		t.Error("cache should be present after writing into an existing dir")
+	}
+}
+
 func TestFetchLatestRelease(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write([]byte(`{"tag_name":"v0.9.7","name":"ignored"}`))

internal/cli/upgrade.go

@@ -60,24 +60,38 @@ func upgradePlanFor(goos string) upgradePlan {
 	}
 }
 
-// isUpgradeCommand reports whether the executed command is `tracebloc upgrade`,
-// so main can skip the post-command update nudge (the still-running process
-// carries its old compile-time version after swapping its own binary, and would
-// otherwise nudge about the very release it just installed).
-func isUpgradeCommand(cmd *cobra.Command) bool {
-	return cmd != nil && cmd.Name() == upgradeCmdName
-}
+// skipUpdateNudgeAnnotation marks a command after which main must NOT run the
+// post-command update nudge (nor the cache write it triggers). Commands
+// self-declare it in their cobra Annotations (see newUpgradeCmd, newDeleteCmd),
+// so SkipUpdateNudge needs no central name list and — critically — can't be
+// fooled by a same-named subcommand: the top-level offboard `delete` carries the
+// annotation while `data delete` does not, even though both leaves are named
+// "delete" (Bugbot #404).
+const skipUpdateNudgeAnnotation = "tracebloc.io/skip-update-nudge"
 
-// SkipUpdateNudge reports whether the update nudge should be suppressed for the
-// command that just ran. Exported for main, which owns the nudge call.
-func SkipUpdateNudge(cmd *cobra.Command) bool { return isUpgradeCommand(cmd) }
+// SkipUpdateNudge reports whether the post-command update nudge (and the cache
+// write it triggers) must be suppressed for the command that just ran. Exported
+// for main, which owns the nudge call. A command opts in via
+// skipUpdateNudgeAnnotation; today that's:
+//   - upgrade: the running process swapped its own binary and now carries the
+//     stale compile-time version — it would nudge about the release it just
+//     installed.
+//   - delete: offboarding removed the CLI and (on the default path) wiped
+//     ~/.tracebloc, so the nudge is nonsensical and its cache write would
+//     resurrect the just-offboarded host data dir (Bugbot #397).
+func SkipUpdateNudge(cmd *cobra.Command) bool {
+	return cmd != nil && cmd.Annotations[skipUpdateNudgeAnnotation] == "true"
+}
 
 // newUpgradeCmd implements `tracebloc upgrade` — the apply step the update
 // nudge (update_check.go) and the 426 "too old" error both point at.
 func newUpgradeCmd() *cobra.Command {
 	return &cobra.Command{
-		Use:   upgradeCmdName,
-		Short: "Update tracebloc to the latest release",
+		Use: upgradeCmdName,
+		// Suppress the post-command update nudge/cache-write: this process just
+		// swapped its own binary and is stale by design (see SkipUpdateNudge).
+		Annotations: map[string]string{skipUpdateNudgeAnnotation: "true"},
+		Short:       "Update tracebloc to the latest release",
 		Long: `Updates tracebloc to the latest release by re-running the official
 installer. It verifies signatures (cosign) and replaces the CLI; on Linux/macOS
 it also upgrades your secure environment's services to match, so the CLI and the

internal/cli/upgrade_test.go

@@ -92,13 +92,24 @@ func TestUpgradePlanFor_PerOS(t *testing.T) {
 
 // TestSkipUpdateNudge: the nudge must be suppressed right after `tracebloc
 // upgrade` (the running process is stale-by-design once it swaps its own binary)
+// AND after `tracebloc delete` (offboarding removed the CLI and wiped
+// ~/.tracebloc — the nudge's cache write would resurrect the dir; Bugbot #397),
 // but fire for any other command.
 func TestSkipUpdateNudge(t *testing.T) {
 	if !SkipUpdateNudge(newUpgradeCmd()) {
 		t.Error("upgrade command must skip the update nudge")
 	}
-	if SkipUpdateNudge(newDeleteCmd()) {
-		t.Error("non-upgrade command must not skip the nudge")
+	if !SkipUpdateNudge(newDeleteCmd()) {
+		t.Error("delete command must skip the update nudge (its cache write would resurrect the wiped ~/.tracebloc)")
+	}
+	// `data delete` shares the leaf name "delete" but does NOT offboard/wipe — it
+	// must still get the nudge. Guards the name-collision fix (Bugbot #404): the
+	// skip is driven by an annotation, not by cmd.Name().
+	if SkipUpdateNudge(newDataDeleteCmd()) {
+		t.Error("`data delete` must NOT skip the nudge — it doesn't wipe ~/.tracebloc or remove the CLI")
+	}
+	if SkipUpdateNudge(newDoctorCmd(false)) {
+		t.Error("an ordinary command (doctor) must not skip the nudge")
 	}
 	if SkipUpdateNudge(nil) {
 		t.Error("nil command must not skip the nudge")